diff --git a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs index 9a98110210a1f..e9131a51d00e2 100644 --- a/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs +++ b/src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs @@ -58,8 +58,13 @@ public SolutionChecksumUpdater( _shutdownToken = shutdownToken; + // A time span of Short is chosen here to ensure that the batching favors fewer but larger batches. + // During solution load a large number of WorkspaceChange events might be raised over a few seconds, + // and in performance tests a 50ms delay was found to be causing a lot of extra memory churn synchronizing + // things OOP. Short didn't cause a similar issue; it's possible this will need to be fine tuned for something in + // the middle. _synchronizeWorkspaceQueue = new AsyncBatchingWorkQueue( - DelayTimeSpan.NearImmediate, + DelayTimeSpan.Short, SynchronizePrimaryWorkspaceAsync, listener, shutdownToken); diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs index ee02ce7ef6cf7..ce3dc9480a18e 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DocumentActiveContextChangedEventSource.cs @@ -13,9 +13,8 @@ private sealed class DocumentActiveContextChangedEventSource(ITextBuffer subject { private WorkspaceEventRegistration? _documentActiveContextChangedDisposer; - // Require main thread on the callback as RaiseChanged implementors may have main thread dependencies. protected override void ConnectToWorkspace(Workspace workspace) - => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged, WorkspaceEventOptions.RequiresMainThreadOptions); + => _documentActiveContextChangedDisposer = workspace.RegisterDocumentActiveContextChangedHandler(OnDocumentActiveContextChanged); protected override void DisconnectFromWorkspace(Workspace workspace) => _documentActiveContextChangedDisposer?.Dispose(); diff --git a/src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs b/src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs index 1a8e1f613febd..5bc6e17c726c7 100644 --- a/src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs +++ b/src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs @@ -41,7 +41,7 @@ internal interface ITaggerEventSource /// /// An event has happened on the thing the tagger is attached to. The tagger should - /// recompute tags. + /// recompute tags. May be raised on any thread. /// event EventHandler Changed; } diff --git a/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs b/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs index 62ccb84a89d9c..9fbca617af846 100644 --- a/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs +++ b/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs @@ -49,9 +49,7 @@ public CodeAnalysisDiagnosticAnalyzerService( _workspace = workspace; _diagnosticAnalyzerService = _workspace.Services.GetRequiredService(); - // Main thread as OnWorkspaceChanged's call to IDiagnosticAnalyzerService.RequestDiagnosticRefresh isn't clear on - // threading requirements - _ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); + _ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); } private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) diff --git a/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs b/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs index 77381fe91a7d8..52e6bb235e339 100644 --- a/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs +++ b/src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs @@ -22,6 +22,9 @@ internal interface IDiagnosticAnalyzerService : IWorkspaceService /// /// Re-analyze all projects and documents. This will cause an LSP diagnostic refresh request to be sent. /// + /// + /// This implementation must be safe to call on any thread. + /// void RequestDiagnosticRefresh(); /// diff --git a/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs b/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs index 1b7a230ddb6b6..35f3406c1bbf1 100644 --- a/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs +++ b/src/LanguageServer/Protocol/Workspaces/LspWorkspaceRegistrationService.cs @@ -39,9 +39,7 @@ public virtual void Register(Workspace? workspace) m["WorkspacePartialSemanticsEnabled"] = workspace.PartialSemanticsEnabled; }, workspace)); - // Forward workspace change events for all registered LSP workspaces. Requires main thread as it - // fires LspSolutionChanged which hasn't been guaranteed to be thread safe. - var workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnLspWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); + var workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnLspWorkspaceChanged); lock (_gate) { @@ -94,9 +92,7 @@ public void Dispose() } /// - /// Indicates whether the LSP solution has changed in a non-tracked document context. - /// - /// IMPORTANT: Implementations of this event handler should do as little synchronous work as possible since this will block. + /// Indicates whether the LSP solution has changed in a non-tracked document context. May be raised on any thread. /// public EventHandler? LspSolutionChanged; } diff --git a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs index 3814232331dc5..1c4c8e6166489 100644 --- a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs +++ b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs @@ -50,9 +50,6 @@ public StackTraceExplorerViewModel(IThreadingContext threadingContext, Workspace _threadingContext = threadingContext; _workspace = workspace; - // Main thread dependency as Workspace_WorkspaceChanged modifies an ObservableCollection - _ = workspace.RegisterWorkspaceChangedHandler(Workspace_WorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); - _classificationTypeMap = classificationTypeMap; _formatMap = formatMap; @@ -103,15 +100,6 @@ private void CallstackLines_CollectionChanged(object sender, System.Collections. NotifyPropertyChanged(nameof(IsInstructionTextVisible)); } - private void Workspace_WorkspaceChanged(WorkspaceChangeEventArgs e) - { - if (e.Kind == WorkspaceChangeKind.SolutionChanged) - { - Selection = null; - Frames.Clear(); - } - } - private FrameViewModel GetViewModel(ParsedFrame frame) => frame switch { diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs index 0525b9c7cc116..f5d797ba1dfb4 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs @@ -19,6 +19,7 @@ internal sealed partial class CpsDiagnosticItemSource : BaseDiagnosticAndGenerat { private readonly IVsHierarchyItem _item; private readonly string _projectDirectoryPath; + private readonly string? _analyzerFilePath; private WorkspaceEventRegistration? _workspaceChangedDisposer; @@ -37,6 +38,8 @@ public CpsDiagnosticItemSource( _item = item; _projectDirectoryPath = Path.GetDirectoryName(projectPath); + _analyzerFilePath = CpsUtilities.ExtractAnalyzerFilePath(_projectDirectoryPath, _item.CanonicalName); + this.AnalyzerReference = TryGetAnalyzerReference(Workspace.CurrentSolution); if (this.AnalyzerReference == null) { @@ -47,9 +50,7 @@ public CpsDiagnosticItemSource( // then connect to it. if (workspace.CurrentSolution.ContainsProject(projectId)) { - // Main thread dependency as OnWorkspaceChangedLookForAnalyzer accesses the IVsHierarchy - // and fires the PropertyChanged event - _workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChangedLookForAnalyzer, WorkspaceEventOptions.RequiresMainThreadOptions); + _workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChangedLookForAnalyzer); item.PropertyChanged += IVsHierarchyItem_PropertyChanged; // Now that we've subscribed, check once more in case we missed the event @@ -118,14 +119,11 @@ private void OnWorkspaceChangedLookForAnalyzer(WorkspaceChangeEventArgs e) return null; } - var canonicalName = _item.CanonicalName; - var analyzerFilePath = CpsUtilities.ExtractAnalyzerFilePath(_projectDirectoryPath, canonicalName); - - if (string.IsNullOrEmpty(analyzerFilePath)) + if (string.IsNullOrEmpty(_analyzerFilePath)) { return null; } - return project.AnalyzerReferences.FirstOrDefault(r => string.Equals(r.FullPath, analyzerFilePath, StringComparison.OrdinalIgnoreCase)); + return project.AnalyzerReferences.FirstOrDefault(r => string.Equals(r.FullPath, _analyzerFilePath, StringComparison.OrdinalIgnoreCase)); } }