Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: use ValueListBuilder for PrivatePrintLeadingTrivia #1532

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TimothyMakkison
Copy link
Contributor

@TimothyMakkison TimothyMakkison commented Mar 9, 2025

Added ValueListBuilder to PrivatePrintLeadingTrivia saving 0.55 MB

  • Added ValueListBuilder.Pop - this is an unmodified copy of microsofts Pop
  • Added my own version of Insert(int index, T item)
  • Added ValueListBuilder.ToList this is because CompilationUnit needs the resulting IList<Doc> contents to be an actual List<Doc> instead of an array.
    • I could modify CompilationUnit.Print to not need a mutable list WDYT.

This isn't a massive saving I can close this if you like. I suspect this is more impactful for projects with a lot of comments and summaries though.

@belav
Copy link
Owner

belav commented Mar 16, 2025

I'm wondering if keeping it is worth it to try to keep Token consistently using ValueListBuilder.

Besides this, multiline raw strings also make use of List<Doc>, I'm not sure how much they would benefit from it.

In theory I think CompilationUnit could set things to Doc.Null instead of removing them. I think it is pretty rare that it actually ends that code block, so it may be worth it to avoid the ToList call. And some day if I can find the time I did want to try to redo all of this comment logic, which would probably also get rid of the need for ToList.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants