-
Notifications
You must be signed in to change notification settings - Fork 869
F# VS editor: reduce background CPU load by gating expensive analyzers to the active document only #20117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
F# VS editor: reduce background CPU load by gating expensive analyzers to the active document only #20117
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. | ||
|
|
||
| namespace Microsoft.VisualStudio.FSharp.Editor | ||
|
|
||
| open System | ||
| open Microsoft.CodeAnalysis | ||
| open Microsoft.VisualStudio | ||
| open Microsoft.VisualStudio.Shell | ||
| open Microsoft.VisualStudio.Shell.Interop | ||
|
|
||
| /// Helpers for determining whether a Roslyn Document corresponds to the document | ||
| /// currently active (focused) in the Visual Studio shell. | ||
| /// | ||
| /// Background expensive analyzers (UnusedOpens, UnusedDeclarations, SimplifyName, | ||
| /// InlayHints) should run only for the active document, mirroring how C# restricts | ||
| /// "remove unnecessary usings" and similar live analyzers. | ||
| /// | ||
| /// Roslyn's BackgroundAnalysisScope lets a host choose "open documents" or | ||
| /// "entire solution" but has no built-in "active document only" tier, so we | ||
| /// determine the truly active document ourselves via the VS shell. | ||
| [<RequireQualifiedAccess>] | ||
| module internal ActiveDocumentDetection = | ||
|
|
||
| /// Returns the document moniker (full file path) of the currently focused | ||
| /// editor window, or ValueNone if it cannot be determined. | ||
| let tryGetActiveDocumentMoniker (serviceProvider: IServiceProvider) : string voption = | ||
| match serviceProvider.GetService(typeof<SVsShellMonitorSelection>) with | ||
| | :? IVsMonitorSelection as monitorSelection -> | ||
| let mutable frameObj = null | ||
|
|
||
| if | ||
| ErrorHandler.Succeeded( | ||
| monitorSelection.GetCurrentElementValue(uint32 VSConstants.VSSELELEMID.SEID_DocumentFrame, &frameObj) | ||
| ) | ||
| then | ||
| match frameObj with | ||
| | :? IVsWindowFrame as frame -> | ||
| let mutable monikerObj = null | ||
|
|
||
| if | ||
| ErrorHandler.Succeeded(frame.GetProperty(int32 __VSFPROPID.VSFPROPID_pszMkDocument, &monikerObj)) | ||
| then | ||
| match monikerObj with | ||
| | :? string as moniker -> ValueSome moniker | ||
| | _ -> ValueNone | ||
| else | ||
| ValueNone | ||
| | _ -> ValueNone | ||
| else | ||
| ValueNone | ||
| | _ -> ValueNone | ||
|
|
||
| /// Returns true when the given document is the currently active editor document. | ||
| /// | ||
| /// Falls back to true (= do not suppress analysis) when the active document | ||
| /// cannot be determined, so analysis is never silently lost. | ||
| let isActiveDocument (serviceProvider: IServiceProvider) (document: Document) : bool = | ||
| match document.FilePath with | ||
| | null -> true | ||
| | filePath -> | ||
| match tryGetActiveDocumentMoniker serviceProvider with | ||
| | ValueNone -> true // couldn't determine the active document, don't suppress analysis | ||
| | ValueSome activeMoniker -> String.Equals(activeMoniker, filePath, StringComparison.OrdinalIgnoreCase) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Results now depend on focus, but no provider invalidates results when focus moves between visible panes. Invalidate both documents on selection changes, or remove result-level gating. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,9 @@ type private PerDocumentSavedData = | |
| } | ||
|
|
||
| [<Export(typeof<IFSharpSimplifyNameDiagnosticAnalyzer>)>] | ||
| type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () = | ||
| type internal SimplifyNameDiagnosticAnalyzer | ||
| [<ImportingConstructor>] | ||
| ([<Import("Microsoft.VisualStudio.Shell.SVsServiceProvider")>] serviceProvider: IServiceProvider) = | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| static let userOpName = "SimplifyNameDiagnosticAnalyzer" | ||
| static let cache = new MemoryCache("FSharp.Editor." + userOpName) | ||
|
|
@@ -37,6 +39,7 @@ type internal SimplifyNameDiagnosticAnalyzer [<ImportingConstructor>] () = | |
|
|
||
| asyncMaybe { | ||
| do! Option.guard document.Project.IsFSharpCodeFixesSimplifyNameEnabled | ||
| do! Option.guard (ActiveDocumentDetection.isActiveDocument serviceProvider document) | ||
| do Trace.TraceInformation("{0:n3} (start) SimplifyName", DateTime.Now.TimeOfDay.TotalSeconds) | ||
| let! textVersion = document.GetTextVersionAsync(cancellationToken) | ||
| let textVersionHash = textVersion.GetHashCode() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roslyn runs these callbacks on worker threads. This code calls UI-bound shell services without switching threads, so exceptions escape the fail-open path. Track selection on the UI thread and publish a thread-safe snapshot.