ChildSyntaxList.ItemInternal optimization #73650
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
ToddGrun
changed the title
|
@jaredpar -- Who would be appropriate compiler reviewers? |
ToddGrun
changed the title
|
For context, the "line walk" simulates how 'syntactic classification' works, where the editor calls into us to classify lines in view (including as the view is scrolled). So we get N calls to classify certain subspans of the document, and each of those N calls needs to walk the tree to find the tokens intersecting that subspan. Improvements to the tree-walk make a big difference. Here, it's about 66% better (from 1,962.582ms to 629.993ms). This is because to walk the tree, we're constantly hitting nodes and doing .ChildNodesAndTokens on it. Speeding that up from N^2 to linear for each node we hit makes a significant difference. We were looking at this because we were seeing expenses in Roslyn in classification/scrolling higher than what we wanted. This alone drops our current syntactic classification time by about 50%. |
|
Note; while classification was the core scenario we cared about (as it is such a hot spot), this shows up everywhere, since we're constantly walking trees. Everything benefits form this :) |
src/Compilers/Core/Portable/Syntax/ChildSyntaxList.Enumerator.cs
Outdated
Show resolved
Hide resolved
|
With my tests, i get this speedup on the walking algorithm that classification uses: before: After So basically 50% faster. Also, the classification walk is much better than .DescendantTokens wrt to memory: That's 20x less memory. And nothing in gen0//gen1. |
|
Refresh my memory, it was pretty invasive to try and get the DescendantTokens code to have an allocation profile similar to the manual walk done in WalkClassification, right? In reply to: 2126281448 |
I think it wouldn't be too hard to do. we'd want to potentially invest in some dedicated tests to ensure the same behavior as before (esp. around empty length tokens). |
|
@dotnet/roslyn-compiler -- ptal, need 2nd review |
1 similar comment
|
@dotnet/roslyn-compiler -- ptal, need 2nd review |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@dotnet-policy-service rerun |
ChildSyntaxList.ItemInternal has shown up in many profiles that I've looked at. I finally dug in a bit to it, and think I've identified a potential optimization opportunity.
This optimization takes advantage that the first loop was previously always iterating through all the slots until it had found index items. However, as this method is commonly called from inside the ChildSyntaxList.Enumerator, it can use knowledge from previous calls to start that first loop at a more appropriate location.
Running the benchmark.net tests in the PR yield the following output. Essentially, the tree walk is ~10% faster while the line walk is much closer to 50% faster (depending on the file size)
*** without changes ***
Run1:
Run2:
*** with changes ***
Run 1:
Run2: