[Repo Assist] perf: remove global mutable FSharpDisplayContext in DocumentationFormatter#1529
Draft
github-actions[bot] wants to merge 2 commits intomainfrom
Conversation
…atter Eliminates the module-level mutable lastDisplayContext from DocumentationFormatter, fixing a memory retention issue where FSharpDisplayContext (and the FCS references it holds) was kept alive indefinitely between documentation requests. - Remove mutable lastDisplayContext field entirely - Add explicit displayContext parameter to getTooltipDetailsFromSymbol - Remove the now-redundant lastDisplayContext assignment in getTooltipDetailsFromSymbolUse (which already used symbol.DisplayContext directly for all its formatting) - Update ParseAndCheckResults caller to pass FSharpDisplayContext.Empty (matches previous behaviour when no prior hover call had been made) Closes #692 Co-authored-by: Copilot <[email protected]>
27 tasks
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.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Closes #692
Root Cause
DocumentationFormatterheld a module-levelmutable lastDisplayContext: FSharpDisplayContext = FSharpDisplayContext.Empty. This was set ingetTooltipDetailsFromSymbolUseon every hover request but never cleared. BecauseFSharpDisplayContextholds internal references to FCS type-checking state, it was pinning memory across requests indefinitely — visible as unusually high memory attributed toFSharpDisplayContextin heap snapshots.Fix
mutable lastDisplayContextfield entirely.displayContext: FSharpDisplayContextparameter togetTooltipDetailsFromSymbol(the only function that actually read the mutable —getTooltipDetailsFromSymbolUsealready usedsymbol.DisplayContextdirectly throughout its body, making the assignment vestigial).ParseAndCheckResults.TryGetFormattedDocumentationForSymbolto passFSharpDisplayContext.Empty. This matches the pre-existing behaviour for cold starts (before any hover call set the mutable), and is appropriate because this code path searchesAssemblySymbolentities resolved from the project's assembly references — not from a live type-check — so no richer context is available there.Trade-offs
getTooltipDetailsFromSymbolsignature is a breaking API change for any downstream consumers ofDocumentationFormatter(outside this repo). However,DocumentationFormatteris an internal module (no.fsisignature file), so this should only affect code that references the module directly.FSharpDisplayContext.Emptyformats generic parameters with default/anonymous names, which is the same behaviour as before for the cold-start case. Signatures formatted for documentation lookup (thefsharp/documentationForSymbolendpoint) were already using whatever context happened to be cached, which is not necessarily related to the current symbol — soEmptyis no worse.Test Status
✅
dotnet build src/FsAutoComplete.Core/FsAutoComplete.Core.fsproj -f net8.0— succeeded (0 warnings, 0 errors)✅
dotnet build src/FsAutoComplete/FsAutoComplete.fsproj -f net8.0— succeeded (0 warnings, 0 errors)✅
dotnet fantomas— no reformatting needed