Cache the getnode/gettoken classifiers retrieived from the extension manager. #73712
Conversation
…manager. These showed up as ~6% of allocations in a find all references trace I'm looking at.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -24,8 +25,17 @@ internal abstract class AbstractClassificationService(ISyntaxClassificationServi | |||
| { | |||
| private readonly ISyntaxClassificationService _syntaxClassificationService = syntaxClassificationService; | |||
|
|
|||
| private static Func<SyntaxNode, ImmutableArray<Classifiers.ISyntaxClassifier>>? s_getNodeClassifiers; | |||
| private static Func<SyntaxToken, ImmutableArray<Classifiers.ISyntaxClassifier>>? s_getTokenClassifiers; | |||
| private static ExtensionClassifierInfo? s_cachedExtensionClassifierInfo; | |||
There was a problem hiding this comment.
alternatively. each classification service is lang specific. so don't cache these statically. cache as instance members.
There was a problem hiding this comment.
ooh, I like that better
There was a problem hiding this comment.
Bummer, RemoteSemanticClassificationService accesses this method statically.
There was a problem hiding this comment.
This seems like a common enough code path, that it should also hit the optimization, and I don't want to cache these both statically and on the instances, so I think I'd prefer to keep it as is unless you see a nice way around this.
There was a problem hiding this comment.
you can have RemoteSemanticClassificationService fetch the real service, not access it statically.
There was a problem hiding this comment.
I think the general idea works (I don't think RemoteSemanticClassificationService can access AbstractClassificationService, but I can just have the static method do the cast check.
These showed up as ~6% of allocations in a find all references trace I'm looking at and it's trivial to cache these.
*** Previously ***
With my changes, these allocations are completely gone from the profile.