Small allocation improvement to GreenNodeExtensions.WithAnnotationsGreen#12199
Conversation
…eenNode The WithAnnotationsGreen method shows up as about 0.7% of allocations in the RoslynCodeAnalysisService process during the C# completions scenario in the razor cohosting completion test. These changes should get rid of about 0.2% of that (The top two blue highlighted lines in the profile image).
| } | ||
|
|
||
| if (newAnnotations.Count == 0) | ||
| using var newAnnotations = new PooledArrayBuilder<SyntaxAnnotation>(annotations.Length); |
There was a problem hiding this comment.
Is it possible to use a pooled hash set here instead?
There was a problem hiding this comment.
I was going to ask the same question, but looking around it seems like the number of annotations is unlikely to ever be more than one. Unless I've misunderstood their use.
There was a problem hiding this comment.
Definitely possible, but I figured these collections are usually small enough that an array might be the better choice, even with the Contains usage.
There was a problem hiding this comment.
I also suspect an array is fine. There aren't many annotations in Razoz. And FWIW, Roslyn also uses an array here. https://source.dot.net/#Microsoft.CodeAnalysis/Syntax/[GreenNodeExtensions.cs](https://source.dot.net/#Microsoft.CodeAnalysis/Syntax/GreenNodeExtensions.cs,4562ff870c047a32),4562ff870c047a32
davidwengier
left a comment
davidwengier
left a comment
There was a problem hiding this comment.
From a quick search of the repo (and lack of understanding of the compiler), it seems like annotations are only used in half a dozen places, and could be replaced with strongly typed properties on each specific node syntax type. Worth logging a follow up?
TL;DR Annotations on Razor nodes could go the way of trivia.
ToddGrun
changed the title
@DustinCampbell -- Is this on your list of items to purge? |
Nope, and I think that's probably premature without further analysis. |
5 participants
The WithAnnotationsGreen method shows up as about 0.7% of allocations in the RoslynCodeAnalysisService process during the C# completions scenario in the razor cohosting completion test.
These changes should get rid of about 0.2% of that (The top two blue highlighted lines in the profile image).
