Analyzer feature telemetry

[arkalyanms]

This tracks improving analyzer feature telemetry and below was the original analysis by ManishV

• Even though the IDE host requests for analyzer diagnostics one analyzer at a time here: https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs#L371-L376, the DocumentAnalysisExecutor requests for all analyzer diagnostics at once: https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor.cs#L267-L276, and caches them, and answers all per-analyzer diagnostic queries from this cache. Analyzers are always executed concurrently in OOP: https://github.com/dotnet/roslyn/blob/main/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs#L263-L266.
• The only way to get reliable perf numbers per-analyzer is to get them from the core analyzer driver in compiler layer that makes callbacks into the analyzer. We already pass true for the boolean flag logPerformanceExecutionTime in CompilationWithAnalyzersOptions used on the OOP side: https://github.com/dotnet/roslyn/blob/main/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs#L276-L286. If the client calling into OOP for diagnostic requests passes in LogPerformanceInfo = true to the remote host here: https://github.com/dotnet/roslyn/blob/main/src/Features/Core/Portable/Diagnostics/DiagnosticArguments.cs#L23-L28, we log analyzer performance telemetry here: https://github.com/dotnet/roslyn/blob/86e06cace23209576e17cb362b2501593c76cbd3/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs#L116-L121. This can be done by the IDE client by passing in logPerformanceInfo: true to DocumentAnalysisExecutor ctor, for the lightbulb path we currently pass in false: https://github.com/dotnet/roslyn/blob/main/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs#L300. If we change the above line to pass in true, we will start logging analyzer perf numbers in OOP from lightbulb path
• All the above seems good but there is a catch. We use the same CompilationWithAnalyzers instance on the OOP side for all document diagnostic requests: https://github.com/dotnet/roslyn/blob/86e06cace23209576e17cb362b2501593c76cbd3/src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs#L25-L35. So basically, requests to get diagnostics for a single line span in the document from lightbulb and requests to get diagnostics for the entire file from background analysis would use the same underlying CompilationWithAnalyzers instance, obviously for performance reasons. This will skew the analyzer execution performance numbers as if both the requests from Ctrl + Dot and background analysis for whole file come in simultaneously, the analyzer driver will add up execution times from both calls. So, if the span based diagnostics request comes in first and finishes before background analysis request for the whole file, we will get correct data for analyzer execution times exclusively from lightbulb code path. Otherwise, the numbers will depend on whether or not background analysis has executed/is executing for the active file. Hopefully the numbers are still useful enough as relative times for analyzer execution should highlight outlier analyzers, just that we won't be sure if the perf numbers came from lightbulb path or background analysis path or both

[arkalyanms]

@mavasani Parking this on you for now to solicit discussion on further analyzer telemetry improvements. This is not an immediate call to action.

[CyrusNajmabadi]

Note: a core issue we have here (and which we likely need compiler team to help address) is that the nature of analyzers is fine for batch scenarios but not great for IDE/lightbulb work. Specifically, analyzers are hooked into while the compiler is running analysis on a file. Then, diagnostics are produced which then have to be fed into appropriate fixes to fix.

This invariably means that "Asking for a lightbulb" ends up performing a ton of work, much/most of which is not in any way useful for the actual core experience that the user is driving.

The original design here for the IDE (Back when these were called "smart tags") was that the experience was driven in teh reverse direction. Based on where the user was querying, the analyzers/fixers would determine quickly if they were even potentially involved, and would not do anything if they were not. This meant not having to do all the analysis and run everything, just to determine if they might end up mattering for the location the user was at.

I have a strong suspicion we need something similar here. The design of analyzers/fixers needs to be updated to be conscientious that "batch compiling mode" is just one mode they run in. But "targetted-location, latency-sensitive mode" is also something they should support. In other words, we should be able to flow in precisely what location we're querying about, and sidestep all unnecessary computation/analyzers/fixers that could not be involved in that request.