Share one whole-file classification between requests for a document version - #20452
Open
xperiandri wants to merge 3 commits into
Open
Share one whole-file classification between requests for a document version#20452xperiandri wants to merge 3 commits into
xperiandri wants to merge 3 commits into
Conversation
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
xperiandri
force-pushed
the
classification-shared-requests
branch
from
September 4, 2026 17:48
e6a1910 to
ea8a3e7
Compare
github-actions
Bot
added
the
AI-Tooling-Check-Scanned-Clean
Every entry point in WorkspaceExtensions signalled "the project has no options yet" by raising an OperationCanceledException, which the callers then had to tell apart from a real cancellation. They cannot, so they treat both as "cancelled" and return an empty result. For a service Roslyn asks repeatedly while a solution loads, that is a wrong answer, not a missing one. Add `Try` siblings that return ValueNone instead, and keep the raising members as thin wrappers so the call sites that have not moved over still get the same exception with the same message. The four-tuple those members hand out becomes a named record, which is also what ProjectCache stores, so a cache hit hands back the instance it holds rather than rebuilding a tuple. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Splitting the semantic classification cache in two (dotnet#15954) left the open-document branch reading the opened-documents cache and writing the unopened one, so the opened cache was never populated and every request for an open file re-ran the checker. What it wrote was also only the requested span, keyed by document and text version, so a second request for another span of the same version (scrolling, a split view, Roslyn asking around the viewport) would have hit that entry and sliced nothing out of it. Cache the classification of the whole file instead, and slice it per request, the way the unopened-documents branch already does. A check that cannot complete - the project is loading or reloading, or the check was superseded - must not answer with no classifications either: Roslyn replaces the tags of a span with whatever comes back, so an empty answer strips the colours the user is looking at, while a cancellation carrying Roslyn's own token leaves them alone. Keep the last classification per document and re-emit it for those requests, but only while the text it was computed from is still the current one, as its spans would otherwise land on the wrong characters. `ifCanceledThen` replaces `ifCanceledReturn ()` for the same reason: the checker relabels its internal cancellations with the caller's token, and those must produce the last known colours rather than none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The classification of an open document covers the whole file, so what it costs is proportional to the file, not to the viewport: on an 8000-line file a single pass takes about a second and a half. Two things made that pass run far more often than the text changed. The entry lived in a MemoryCache with a two-second sliding expiration, so the first scroll after any pause missed it and walked the file again for a text version that had already been classified. The entry now lives in the per-document store that the last-known-good fallback already used, keyed by text version and kept for the life of the document, and the separate opened-documents cache goes away. Requests that overlapped - Roslyn's taggers above and below the viewport, a split view - each walked the file on their own. Requests for the same version now join one in-flight computation; it starts with the first of them and is cancelled only when the last one leaves, so a superseded request never cancels the work its neighbours are still waiting for, and no request outlives its own token. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
xperiandri
force-pushed
the
classification-shared-requests
branch
from
September 4, 2026 22:11
ea8a3e7 to
5c70d1e
Compare
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
1 participant
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.
Description
Since #20450 the semantic classification of an open document covers the whole file, so its cost is proportional to the file rather than to the viewport — on an 8000-line file one pass takes about a second and a half in Release. Two things made that pass run far more often than the text changed.
The entry expired after two seconds. It lived in a
MemoryCachewith a sliding expiration, so the first scroll after any pause missed it and walked the file again for a text version that had already been classified. The entry now lives in the per-document store that the last-known-good fallback already used, keyed by text version and kept for the life of the document; the separate opened-documentsDocumentCachegoes away.Overlapping requests each walked the file. Roslyn's taggers above and below the viewport, or a split view, ask for the same version at once. Requests for one version now join a single in-flight computation (
InFlightClassification): it starts with the first of them and is cancelled only when the last one leaves, so a superseded request never cancels the work its neighbours still wait for, and no request outlives its own token.Both are covered by tests: one for sharing, one for the cancel-on-last-leave rule. The classification service tests that asserted on the removed cache now assert on the per-document store.
Depends on #20450 — it is the head of this branch. Only the last commit, "Share one whole-file classification between requests for a version", belongs to this pull request; please review it after #20450 merges.
No benchmark is attached: the change removes repeated executions of an existing pass rather than altering the pass itself, and the per-pass cost quoted above was measured with the tree's own FCS on a generated 8002-line file (69,330 classification items, 1483 ms per whole-file pass in Release).
Checklist