Fix #491: cache C# syntax trees to eliminate repeated full-document parses - #492
Merged
Merged
Conversation
…arses Add a shared, thread-safe ICSharpSyntaxTreeCache and wire it into ScenarioTestTargetResolver and CSharpAttributeLiteralResolver, eliminating the redundant CSharpSyntaxTree.ParseText(File.ReadAllText(...)) on every resolveTestTargets/rename call. On the VeryLargeFeature stress corpus this took reqnroll/resolveTestTargets from ~153ms to ~3.6ms per call. That fix exposed a second bottleneck: RunTestCodeLensService.GetTargetsAsync resolves an entire feature file's worth of scenarios, but was invoked independently by the tagger and by every visible Scenario line's own out-of-process CodeLens data point. On a large file, N+1 concurrent full- document walks competed for the LSP server, slow enough that individual callers hit VS's own CodeLens timeout and the lens got stuck on "Loading data...". Add RunTestCodeLensResultCache to de-duplicate concurrent callers for the same file into one shared computation, with per-caller cancellation (AsyncLazy) so one caller giving up never aborts the shared work for others. Also: - Change the Run CodeLens title to "Run Scenario"/"Run Scenarios" (singular vs. plural for Scenario Outlines), threading a new Detail field through GherkinDocumentSymbolService/GherkinNavigationBarSymbolService and a new IsScenarioOutline flag through RunTestTargetEntry. - Add logging across the previously-uninstrumented OOP-to-in-process CodeLens callback boundary (RunTestCodeLensCallbackListener, RunTestCodeLensDataPointProvider, RunTestCodeLensDataPoint), which is what surfaced the timeout root cause above. - Fix LspInterceptingPipe so owned-RPC request/response traffic is run through the send/receive interceptors, closing a blind spot in LspInspectorLogger for that traffic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 What's changed?
ICSharpSyntaxTreeCache, a shared, thread-safe, MRU-capped cache for parsed C# syntax trees, and wired it intoScenarioTestTargetResolverandCSharpAttributeLiteralResolver. Both previously calledCSharpSyntaxTree.ParseText(File.ReadAllText(...))on every single invocation, re-parsing the same generated.feature.cs/step-definition files from scratch on everyreqnroll/resolveTestTargetsor rename request.RunTestCodeLensResultCacheto de-duplicate concurrent calls toRunTestCodeLensService.GetTargetsAsyncfor the same file. That method resolves every scenario in a.featurefile, but was being invoked independently by the tagger and by every visible Scenario line's own out-of-process CodeLens data point — on a large file that meant N+1 concurrent full-document walks, slow enough that individual callers hit VS's own CodeLens timeout and the lens got stuck on "Loading data...". Cancellation is deliberately decoupled per caller (viaAsyncLazy<T>) so one caller giving up never aborts the shared computation for the others.Detailfield throughGherkinDocumentSymbolService/GherkinNavigationBarSymbolService(Scenario vs. Scenario Outline can't be told apart fromKindalone) and a newIsScenarioOutlineflag throughRunTestTargetEntry.RunTestCodeLensCallbackListener,RunTestCodeLensDataPointProvider,RunTestCodeLensDataPoint) — this is what surfaced the VS-timeout root cause behind the result-cache fix above.LspInterceptingPipeso owned-RPC request/response traffic (used for the extension's own shutdown handshake, etc.) is run through the send/receive interceptors, closing a blind spot inLspInspectorLoggerwhere that traffic was previously invisible.docs/LSP-IDE-Support-Architecture.mdanddocs/Test-Runner-Integration-Design.mdto reflect the new cache and the corrected cost/fix note.⚡️ What's your motivation?
Fixes #491. On the
VeryLargeFeaturestress-corpus solution (~1,300+ scenarios), a single.featuretab pegged a CPU core and Run Scenario CodeLens never resolved. Root-caused via live VS Experimental-instance testing plus runtime log analysis:reqnroll/resolveTestTargetscall re-parsed the whole generated.feature.csfile from disk with no cache — confirmed via before/after log measurements taking it from ~153ms to ~3.6ms per call.OperationCanceledExceptionand leaving the lens stuck.Verified fixed via two full live VS Experimental-instance test runs against the same corpus: inlay hints in a few seconds, Run Scenario CodeLens resolving in ~30s (a single shared walk instead of N concurrent ones), label correctly pluralized, and the Run popup/test execution working end to end. No
OperationCanceledExceptionin either run's logs.🏷️ What kind of change is this?
🧩 Area(s) touched
src/LSP)src/VisualStudio)♻️ Anything particular you want feedback on?
RunTestCodeLensResultCache's cancellation model: each caller's own token only governs how long that caller waits (backed byAsyncLazy<T>'s documented contract), never the underlying shared computation — a 60s independent timeout bounds the computation itself instead. Flagging this in case there's a simpler pattern preferred for this kind of shared/de-duplicated async work elsewhere in the codebase.📋 Checklist:
docs/docs/AsBuilt-Reconciliation-Reminder.md).🤖 Generated with Claude Code