SyntaxValueProvider: avoid performance issue with syntax list containing many items #83483
Conversation
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-extensions-logging Issue Details
|
src/libraries/Common/src/Roslyn/SyntaxValueProvider_ForAttributeWithSimpleName.cs
Outdated
Show resolved
Hide resolved
|
@cston is there is benchmark numbers show the improvement? |
tarekgh
added
area-System.Runtime.CompilerServices
and removed
area-Extensions-Logging
labels
|
Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices Issue Details
|
From a couple of runs of the added test Those runs were without the fix in dotnet/roslyn#66876 however. In .NET 8 builds that include dotnet/roslyn#66876, there shouldn't be a performance issue iterating a |
| foreach (var child in node.ChildNodesAndTokens().Reverse()) | ||
| var childNodesAndTokens = node.ChildNodesAndTokens(); | ||
|
|
||
| // Avoid performance issue (in .NET 7 and earlier) iterating the child list in reverse |
There was a problem hiding this comment.
How does this relate to ".NET 7"? Is this actually about the version of Roslyn that's being used?
There was a problem hiding this comment.
Yes, this comment is about the version of Roslyn that's used.
Perhaps this should be: "Avoid performance issue in earlier implementations of ChildSyntaxList iterating the child list in reverse ..."
There was a problem hiding this comment.
I see. You could just be explicit and include a link to the Roslyn PR that fixed it. Essentially we're saying if you're running with an earlier Roslyn than that, you could have perf issues. I'm just questioning the original wording because you will likely end up getting that fix as part of .NET 7 SDK updates and as part of VS updates, right?
|
/backport to release/7.0 |
|
/backport to release/6.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4483155231 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/4483155981 |
See dotnet/roslyn#66475