Skip to content

Semantic classification cache for opened documents is never populated (written to the unopened-documents cache) #20445

Description

@xperiandri

Summary

In FSharpClassificationService.AddSemanticClassificationsAsync, the open-document branch reads from openedDocumentsSemanticClassificationCache but writes to unopenedDocumentsSemanticClassificationCache. As a result openedDocumentsSemanticClassificationCache is written nowhere, its lookup always misses, and every semantic classification request for an open document goes through a full GetFSharpParseAndCheckResultsAsync + GetSemanticClassification.

vsintegration/src/FSharp.Editor/Classification/ClassificationService.fs:

Line Branch Operation Cache
146 declaration unopenedDocumentsSemanticClassificationCache (TTL 5.0)
149 declaration openedDocumentsSemanticClassificationCache (TTL 2.0)
227 not isOpenDocument read unopened…
258 not isOpenDocument write unopened…
262 open document read opened…
301 open document write unopened…

Suggested fix

                             if classificationData.Length > 0 then
                                 let classificationDataLookup = itemToSemanticClassificationLookup classificationData
-                                do! unopenedDocumentsSemanticClassificationCache.SetAsync(document, classificationDataLookup)
+                                do! openedDocumentsSemanticClassificationCache.SetAsync(document, classificationDataLookup)

Origin

Introduced in #15954 ("More VS cleanup", merged 2023-10-10), which split a single semantic classification cache into two. In that diff all four lines are additions: the read in the open-document branch was pointed at the new opened… cache, while the write was kept as a copy of the sibling branch and still names unopened….

git blame currently attributes line 301 to #18653 ("Move LSP development to the main branch"), but that is a red herring — that commit only wrapped the method in the shouldProduceClassification gate and re-indented the block. The version of the file at its parent commit already has the same wrong cache name.

Impact

Functionally the classification is still correct — it is a pure cache miss, so the effect is repeated full re-checking rather than wrong colors. It becomes visible when the recomputation stops fitting between requests: each new classification request cancels the previous one, and the semantic pass ends with

|> CancellableTask.ifCanceledReturn ()

which completes successfully with an empty result, so the editor drops the classification it had. Observed as semantic colors appearing for a moment and then disappearing.

How it was observed

Debugging FSharp.Editor in an experimental hive against a large F# solution (VS 18.10, ~40 F# projects):

  • breakpoint at line 299 (if classificationData.Length > 0 then) hits with classificationData.Length = 57 — the checker does produce data, and colors do appear;
  • the Debug output window for the session contained ~1600 OperationCanceledException / TaskCanceledException, a large share of them from FSharp.Editor.dll, and no other exceptions besides FCS-internal control-flow ones (UndefinedName, CannotRefute, IndeterminateType);
  • with the cache never hit, every request re-enters the full check path.

Secondary observation (not part of the fix above)

ifCanceledReturn () at line 305 makes a cancelled semantic pass indistinguishable from "there are no classifications here". Preserving previously reported classifications on cancellation would be a separate, more invasive change, and is filed here only for reference.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions