diff --git a/src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs b/src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs index d67a5cfdf7dea..6bde567e818f0 100644 --- a/src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs +++ b/src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs @@ -30,14 +30,20 @@ internal sealed class PdbMatchingSourceTextProvider() : IEventListener, IPdbMatc private bool _isActive; private int _baselineSolutionVersion; private readonly Dictionary _documentsWithChangedLoaderByPath = []; + private WorkspaceEventRegistration? _workspaceChangedDisposer; public void StartListening(Workspace workspace) - => workspace.WorkspaceChanged += WorkspaceChanged; + { + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(WorkspaceChanged); + } public void StopListening(Workspace workspace) - => workspace.WorkspaceChanged -= WorkspaceChanged; + { + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + } - private void WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private void WorkspaceChanged(WorkspaceChangeEventArgs e) { if (!_isActive) { diff --git a/src/EditorFeatures/Core/EditorConfigSettings/Aggregator/SettingsAggregator.cs b/src/EditorFeatures/Core/EditorConfigSettings/Aggregator/SettingsAggregator.cs index b5050c4bc7d3c..a3e0f455df219 100644 --- a/src/EditorFeatures/Core/EditorConfigSettings/Aggregator/SettingsAggregator.cs +++ b/src/EditorFeatures/Core/EditorConfigSettings/Aggregator/SettingsAggregator.cs @@ -32,7 +32,7 @@ public SettingsAggregator( IAsynchronousOperationListener listener) { _workspace = workspace; - _workspace.WorkspaceChanged += UpdateProviders; + _ = workspace.RegisterWorkspaceChangedHandler(UpdateProviders); var currentSolution = _workspace.CurrentSolution.SolutionState; UpdateProviders(currentSolution); @@ -48,7 +48,7 @@ public SettingsAggregator( threadingContext.DisposalToken); } - private void UpdateProviders(object? sender, WorkspaceChangeEventArgs e) + private void UpdateProviders(WorkspaceChangeEventArgs e) { switch (e.Kind) { diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index a43f23d3a545a..401a0da48909a 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -57,6 +57,7 @@ internal sealed partial class InlineRenameSession : IInlineRenameSession, IFeatu private bool _isApplyingEdit; private string _replacementText; private readonly Dictionary _openTextBuffers = []; + private WorkspaceEventRegistration _workspaceChangedDisposer; /// /// The original where rename was triggered @@ -161,7 +162,9 @@ public InlineRenameSession( _inlineRenameSessionDurationLogBlock = Logger.LogBlock(FunctionId.Rename_InlineSession, CancellationToken.None); Workspace = workspace; - Workspace.WorkspaceChanged += OnWorkspaceChanged; + + // Requires the main thread due to OnWorkspaceChanged calling the Cancel method + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); _textBufferFactoryService = textBufferFactoryService; _textBufferCloneService = textBufferCloneService; @@ -383,7 +386,7 @@ private void VerifyNotDismissed() } } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs args) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs args) { if (args.Kind != WorkspaceChangeKind.DocumentChanged) { @@ -701,7 +704,9 @@ private void DismissUIAndRollbackEditsAndEndRenameSession_MustBeCalledOnUIThread void DismissUIAndRollbackEdits() { - Workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + _textBufferAssociatedViewService.SubjectBuffersConnected -= OnSubjectBuffersConnected; // Reenable completion now that the inline rename session is done diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.ParseOptionChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.ParseOptionChangedEventSource.cs index 447f5e99f7872..d41bc08a614ef 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.ParseOptionChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.ParseOptionChangedEventSource.cs @@ -13,13 +13,20 @@ internal partial class TaggerEventSources { private sealed class ParseOptionChangedEventSource(ITextBuffer subjectBuffer) : AbstractWorkspaceTrackingTaggerEventSource(subjectBuffer) { + private WorkspaceEventRegistration? _workspaceChangedDisposer; + protected override void ConnectToWorkspace(Workspace workspace) - => workspace.WorkspaceChanged += OnWorkspaceChanged; + { + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); + } protected override void DisconnectFromWorkspace(Workspace workspace) - => workspace.WorkspaceChanged -= OnWorkspaceChanged; + { + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { if (e.Kind == WorkspaceChangeKind.ProjectChanged) { diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs index 920db2ca74baf..51a1735aab535 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs @@ -15,6 +15,7 @@ internal partial class TaggerEventSources private sealed class WorkspaceChangedEventSource : AbstractWorkspaceTrackingTaggerEventSource { private readonly AsyncBatchingWorkQueue _asyncDelay; + private WorkspaceEventRegistration? _workspaceChangedDisposer; public WorkspaceChangedEventSource( ITextBuffer subjectBuffer, @@ -36,17 +37,19 @@ public WorkspaceChangedEventSource( protected override void ConnectToWorkspace(Workspace workspace) { - workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); this.RaiseChanged(); } protected override void DisconnectFromWorkspace(Workspace workspace) { - workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + this.RaiseChanged(); } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs eventArgs) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs eventArgs) => _asyncDelay.AddWork(); } } diff --git a/src/EditorFeatures/Core/SolutionEvents/HostLegacySolutionEventsWorkspaceEventListener.cs b/src/EditorFeatures/Core/SolutionEvents/HostLegacySolutionEventsWorkspaceEventListener.cs index 5fe501dff9197..2118302a39f9b 100644 --- a/src/EditorFeatures/Core/SolutionEvents/HostLegacySolutionEventsWorkspaceEventListener.cs +++ b/src/EditorFeatures/Core/SolutionEvents/HostLegacySolutionEventsWorkspaceEventListener.cs @@ -32,6 +32,8 @@ internal sealed partial class HostLegacySolutionEventsWorkspaceEventListener : I private readonly IThreadingContext _threadingContext; private readonly AsyncBatchingWorkQueue _eventQueue; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public HostLegacySolutionEventsWorkspaceEventListener( @@ -53,16 +55,19 @@ public void StartListening(Workspace workspace) // We only support this option to disable crawling in internal speedometer and ddrit perf runs to lower noise. // It is not exposed to the user. if (_globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler)) - workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); } public void StopListening(Workspace workspace) { if (_globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler)) - workspace.WorkspaceChanged -= OnWorkspaceChanged; + { + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + } } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { // Legacy workspace events exist solely to let unit testing continue to work using their own fork of solution // crawler. As such, they only need events for the project types they care about. Specifically, that is only diff --git a/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs b/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs index 2ae365daf1c0e..ec7687e533d2d 100644 --- a/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs +++ b/src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs @@ -11,8 +11,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.Shared.Utilities; -using Microsoft.CodeAnalysis.Threading; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Diagnostics; @@ -56,10 +54,12 @@ public CodeAnalysisDiagnosticAnalyzerService( _diagnosticAnalyzerService = diagnosticAnalyzerService; _workspace = workspace; - _workspace.WorkspaceChanged += OnWorkspaceChanged; + // Main thread as OnWorkspaceChanged's call to IDiagnosticAnalyzerService.RequestDiagnosticRefresh isn't clear on + // threading requirements + _ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); } - private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { switch (e.Kind) { diff --git a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs index 2ef0e6ba59d47..f665a3de9e192 100644 --- a/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs +++ b/src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs @@ -65,7 +65,7 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices) public CompileTimeSolutionProvider(Workspace workspace) { - workspace.WorkspaceChanged += (s, e) => + _ = workspace.RegisterWorkspaceChangedHandler((e) => { if (e.Kind is WorkspaceChangeKind.SolutionCleared or WorkspaceChangeKind.SolutionRemoved) { @@ -79,7 +79,7 @@ public CompileTimeSolutionProvider(Workspace workspace) _lastCompileTimeSolution = null; } } - }; + }); } private static bool IsRazorAnalyzerConfig(TextDocumentState documentState) diff --git a/src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesignerAttributeService.cs b/src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesignerAttributeService.cs index 699bfc38552a4..909e1e424d18c 100644 --- a/src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesignerAttributeService.cs +++ b/src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesignerAttributeService.cs @@ -60,6 +60,8 @@ internal sealed class VisualStudioDesignerAttributeService : // deliver them to VS in batches to prevent flooding the UI thread. private readonly AsyncBatchingWorkQueue _projectSystemNotificationQueue; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] public VisualStudioDesignerAttributeService( @@ -92,7 +94,7 @@ void IEventListener.StartListening(Workspace workspace) if (workspace != _workspace) return; - _workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); _workQueue.AddWork(cancelExistingWork: true); } @@ -101,10 +103,11 @@ void IEventListener.StopListening(Workspace workspace) if (workspace != _workspace) return; - _workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { _workQueue.AddWork(cancelExistingWork: true); } diff --git a/src/VisualStudio/Core/Def/Library/ObjectBrowser/AbstractObjectBrowserLibraryManager.cs b/src/VisualStudio/Core/Def/Library/ObjectBrowser/AbstractObjectBrowserLibraryManager.cs index aba4e28fe4c90..5d543edc697b4 100644 --- a/src/VisualStudio/Core/Def/Library/ObjectBrowser/AbstractObjectBrowserLibraryManager.cs +++ b/src/VisualStudio/Core/Def/Library/ObjectBrowser/AbstractObjectBrowserLibraryManager.cs @@ -41,6 +41,7 @@ internal abstract partial class AbstractObjectBrowserLibraryManager : AbstractLi private ObjectListItem _activeListItem; private AbstractListItemFactory _listItemFactory; private readonly object _classMemberGate = new(); + private WorkspaceEventRegistration _workspaceChangedDisposer; protected AbstractObjectBrowserLibraryManager( string languageName, @@ -53,7 +54,7 @@ protected AbstractObjectBrowserLibraryManager( _languageName = languageName; Workspace = workspace; - Workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); _libraryService = new Lazy(() => Workspace.Services.GetLanguageServices(_languageName).GetService()); } @@ -73,9 +74,12 @@ private AbstractListItemFactory GetListItemFactory() } public void Dispose() - => this.Workspace.WorkspaceChanged -= OnWorkspaceChanged; + { + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { switch (e.Kind) { diff --git a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs index 540578ef2107c..fafe720d01eba 100644 --- a/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs +++ b/src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs @@ -232,7 +232,7 @@ protected override async Task EnableServiceAsync(CancellationToken cancellationT var packageSourceProvider = await GetPackageSourceProviderAsync().ConfigureAwait(false); // Start listening to additional events workspace changes. - Workspace.WorkspaceChanged += OnWorkspaceChanged; + _ = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); packageSourceProvider.SourcesChanged += OnSourceProviderSourcesChanged; // Kick off an initial set of work that will analyze the entire solution. @@ -421,7 +421,7 @@ await UpdateStatusBarAsync( } } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { // ThisCanBeCalledOnAnyThread(); diff --git a/src/VisualStudio/Core/Def/Progression/GraphQueryManager.cs b/src/VisualStudio/Core/Def/Progression/GraphQueryManager.cs index 71b051a6fbdda..e5c81b32345f2 100644 --- a/src/VisualStudio/Core/Def/Progression/GraphQueryManager.cs +++ b/src/VisualStudio/Core/Def/Progression/GraphQueryManager.cs @@ -50,7 +50,7 @@ internal GraphQueryManager( // indicating a change happened. And when UpdateExistingQueriesAsync fires, it will just see that there are // no live queries and immediately return. So it's just simple to do things this way instead of trying to // have state management where we try to decide if we should listen or not. - _workspace.WorkspaceChanged += (_, _) => _updateQueue.AddWork(); + _ = _workspace.RegisterWorkspaceChangedHandler((_) => _updateQueue.AddWork()); } public async Task AddQueriesAsync(IGraphContext context, ImmutableArray graphQueries, CancellationToken disposalToken) diff --git a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs index a3b29acfbc8e7..4856cebda2f50 100644 --- a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs +++ b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerViewModel.cs @@ -5,8 +5,8 @@ using System.Collections.ObjectModel; using System.Linq; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.StackTraceExplorer; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; +using Microsoft.CodeAnalysis.StackTraceExplorer; using Microsoft.VisualStudio.LanguageServices.Utilities; using Microsoft.VisualStudio.Text.Classification; @@ -53,7 +53,9 @@ public StackTraceExplorerViewModel(IThreadingContext threadingContext, Workspace _threadingContext = threadingContext; _workspace = workspace; - workspace.WorkspaceChanged += Workspace_WorkspaceChanged; + // Main thread dependency as Workspace_WorkspaceChanged modifies an ObservableCollection + _ = workspace.RegisterWorkspaceChangedHandler(Workspace_WorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions); + _classificationTypeMap = classificationTypeMap; _formatMap = formatMap; @@ -104,7 +106,7 @@ private void CallstackLines_CollectionChanged(object sender, System.Collections. NotifyPropertyChanged(nameof(IsInstructionTextVisible)); } - private void Workspace_WorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void Workspace_WorkspaceChanged(WorkspaceChangeEventArgs e) { if (e.Kind == WorkspaceChangeKind.SolutionChanged) { diff --git a/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingToolWindow.cs b/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingToolWindow.cs index 204e66d98aff2..104e7f2f922bf 100644 --- a/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingToolWindow.cs +++ b/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingToolWindow.cs @@ -3,11 +3,11 @@ // See the LICENSE file in the project root for more information. using System; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; -using Microsoft.VisualStudio.Shell; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; -using System.Diagnostics.CodeAnalysis; +using Microsoft.VisualStudio.Shell; namespace Microsoft.VisualStudio.LanguageServices.ValueTracking; @@ -65,10 +65,10 @@ public void Initialize(ValueTrackingTreeViewModel viewModel, Workspace workspace _workspace = workspace; _threadingContext = threadingContext; - _workspace.WorkspaceChanged += OnWorkspaceChanged; + _ = _workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { Contract.ThrowIfFalse(Initialized); diff --git a/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs b/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs index 2f9c4cde491f2..d58b016f39a64 100644 --- a/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs +++ b/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs @@ -262,6 +262,7 @@ private sealed class OpenSourceGeneratedFile : IDisposable private VisualStudioInfoBar.InfoBarMessage? _currentInfoBarMessage; private InfoBarInfo? _infoToShow = null; + private WorkspaceEventRegistration? _workspaceChangedDisposer; public OpenSourceGeneratedFile(SourceGeneratedFileManager fileManager, ITextBuffer textBuffer, SourceGeneratedDocumentIdentity documentIdentity) { @@ -284,7 +285,7 @@ public OpenSourceGeneratedFile(SourceGeneratedFileManager fileManager, ITextBuff readOnlyRegionEdit.Apply(); } - this.Workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = this.Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); _batchingWorkQueue = new AsyncBatchingWorkQueue( TimeSpan.FromSeconds(1), @@ -311,7 +312,8 @@ public void Dispose() { _fileManager._threadingContext.ThrowIfNotOnUIThread(); - this.Workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; // Disconnect the buffer from the workspace before making it eligible for edits DisconnectFromWorkspaceIfOpen(); @@ -422,7 +424,7 @@ public async ValueTask RefreshFileAsync(CancellationToken cancellationToken) await EnsureWindowFrameInfoBarUpdatedAsync(cancellationToken).ConfigureAwait(true); } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { var projectId = _documentIdentity.DocumentId.ProjectId; diff --git a/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs b/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs index 689670476e0a2..d622976e1481a 100644 --- a/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs +++ b/src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.ComponentModel.Composition; using System.Diagnostics; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -15,6 +16,7 @@ using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageService; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Threading; using Microsoft.VisualStudio.Shell; @@ -33,7 +35,7 @@ internal sealed class ProjectCodeModelFactory : IProjectCodeModelFactory private readonly IServiceProvider _serviceProvider; private readonly IThreadingContext _threadingContext; - private readonly AsyncBatchingWorkQueue _documentsToFireEventsFor; + private readonly AsyncBatchingWorkQueue _workspaceChangeEventsToFireEventsFor; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] @@ -52,75 +54,76 @@ public ProjectCodeModelFactory( // Queue up notifications we hear about docs changing. that way we don't have to fire events multiple times // for the same documents. Once enough time has passed, take the documents that were changed and run // through them, firing their latest events. - _documentsToFireEventsFor = new AsyncBatchingWorkQueue( + _workspaceChangeEventsToFireEventsFor = new AsyncBatchingWorkQueue( DelayTimeSpan.Idle, - ProcessNextDocumentBatchAsync, - // We only care about unique doc-ids, so pass in this comparer to collapse streams of changes for a - // single document down to one notification. - EqualityComparer.Default, + ProcessNextWorkspaceChangeEventBatchAsync, Listener, threadingContext.DisposalToken); - _visualStudioWorkspace.WorkspaceChanged += OnWorkspaceChanged; + _ = _visualStudioWorkspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); } internal IAsynchronousOperationListener Listener { get; } - private async ValueTask ProcessNextDocumentBatchAsync( - ImmutableSegmentedList documentIds, CancellationToken cancellationToken) + private async ValueTask ProcessNextWorkspaceChangeEventBatchAsync( + ImmutableSegmentedList workspaceChangeEvents, CancellationToken cancellationToken) { - // This logic preserves the previous behavior we had with IForegroundNotificationService. - // Specifically, we don't run on the UI thread for more than 15ms at a time. And once we - // have, we wait 50ms before continuing. These constants are just what we defined from - // legacy, and otherwise have no special meaning. - const int MaxTimeSlice = 15; - var delayBetweenProcessing = DelayTimeSpan.NearImmediate; - Debug.Assert(!_threadingContext.JoinableTaskContext.IsOnMainThread, "The following context switch is not expected to cause runtime overhead."); await TaskScheduler.Default; - // Ensure MEF services used by the code model are initially obtained on a background thread. - // This code avoids allocations where possible. - // https://github.com/dotnet/roslyn/issues/54159 - string? previousLanguage = null; - foreach (var projectState in _visualStudioWorkspace.CurrentSolution.SolutionState.SortedProjectStates) - { - if (projectState.Language == previousLanguage) - { - // Avoid duplicate calls if the language did not change - continue; - } + // Calculate the full set of changes over this set of events while on a background thread. + using var _1 = PooledHashSet.GetInstance(out var documentIds); + AddChangedDocuments(workspaceChangeEvents, documentIds); - previousLanguage = projectState.Language; - projectState.LanguageServices.GetService(); - projectState.LanguageServices.GetService(); - projectState.LanguageServices.GetService(); - } + if (documentIds.Count == 0) + return; + + // get the file path information while on a background thread + using var _2 = ArrayBuilder<(ProjectCodeModel projectCodeModel, string filename)>.GetInstance(out var projectCodeModelAndFileNames); + AddProjectCodeModelAndFileNames(documentIds, projectCodeModelAndFileNames); + + // Ensure MEF services used by the code model are initially obtained on a background thread. + EnsureServicesCreated(); await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - var stopwatch = SharedStopwatch.StartNew(); - foreach (var documentId in documentIds) + // Fire off the code model events while on the main thread + await FireEventsForChangedDocumentsAsync(projectCodeModelAndFileNames, cancellationToken).ConfigureAwait(false); + } + + private static void AddChangedDocuments(ImmutableSegmentedList workspaceChangeEvents, PooledHashSet documentIds) + { + if (workspaceChangeEvents.All(static e => e.Kind is WorkspaceChangeKind.DocumentRemoved or WorkspaceChangeKind.DocumentChanged)) { - FireEventsForDocument(documentId); + // Fast path when we know we affected a document that could have had code model elements in it. No + // need to do a solution diff in this case. + foreach (var e in workspaceChangeEvents) + documentIds.Add(e.DocumentId!); - // Keep firing events for this doc, as long as we haven't exceeded the max amount - // of waiting time, and there's no user input that should take precedence. - if (stopwatch.Elapsed.TotalMilliseconds > MaxTimeSlice || IsInputPending()) - { - await this.Listener.Delay(delayBetweenProcessing, cancellationToken).ConfigureAwait(true); - stopwatch = SharedStopwatch.StartNew(); - } + return; } - return; + // Contains an event that could indicate a doc change/removal. Have to actually analyze the change to + // determine what we should do here. + var oldSolution = workspaceChangeEvents[0].OldSolution; + var newSolution = workspaceChangeEvents[^1].NewSolution; + + var changes = oldSolution.GetChanges(newSolution); - void FireEventsForDocument(DocumentId documentId) + foreach (var project in changes.GetRemovedProjects()) + documentIds.AddRange(project.DocumentIds); + + foreach (var projectChange in changes.GetProjectChanges()) { - // If we've been asked to shutdown, don't bother reporting any more events. - if (_threadingContext.DisposalToken.IsCancellationRequested) - return; + documentIds.AddRange(projectChange.GetRemovedDocuments()); + documentIds.AddRange(projectChange.GetChangedDocuments()); + } + } + private void AddProjectCodeModelAndFileNames(HashSet documentIds, ArrayBuilder<(ProjectCodeModel, string)> projectCodeModelAndFileNames) + { + foreach (var documentId in documentIds) + { var projectCodeModel = this.TryGetProjectCodeModel(documentId.ProjectId); if (projectCodeModel == null) return; @@ -129,12 +132,52 @@ void FireEventsForDocument(DocumentId documentId) if (filename == null) return; - if (!projectCodeModel.TryGetCachedFileCodeModel(filename, out var fileCodeModelHandle)) + projectCodeModelAndFileNames.Add((projectCodeModel, filename)); + } + } + + private async Task FireEventsForChangedDocumentsAsync( + ArrayBuilder<(ProjectCodeModel projectCodeModel, string filename)> projectCodeModelAndFileNames, + CancellationToken cancellationToken) + { + // This logic preserves the previous behavior we had with IForegroundNotificationService. + // Specifically, we don't run on the UI thread for more than 15ms at a time. And we don't + // check the input queue more than once per ms. Once we have run for more than 15 ms, + // we wait 50ms before continuing. These constants are just what we defined from + // legacy, and otherwise have no special meaning. + const int MaxTimeSlice = 15; + double nextInputCheckElapsedMs = 1; + + var stopwatch = SharedStopwatch.StartNew(); + foreach (var (projectCodeModel, filename) in projectCodeModelAndFileNames) + { + // If we've been asked to shutdown, don't bother reporting any more events. + if (_threadingContext.DisposalToken.IsCancellationRequested) return; - var codeModel = fileCodeModelHandle.Object; - codeModel.FireEvents(); - return; + FireEventsForDocument(projectCodeModel, filename); + + // Keep firing events for this doc, as long as we haven't exceeded MaxTimeSlice ms or input isn't pending. + // We'll validate against those constraints at most once every 1 ms, to avoid spam checking the + // input queue, as this has shown up in performance profiles. + var elapsedMs = stopwatch.Elapsed.TotalMilliseconds; + if (elapsedMs > nextInputCheckElapsedMs) + { + if (elapsedMs > MaxTimeSlice || IsInputPending()) + { + await this.Listener.Delay(DelayTimeSpan.NearImmediate, cancellationToken).ConfigureAwait(true); + stopwatch = SharedStopwatch.StartNew(); + elapsedMs = 0; + } + + nextInputCheckElapsedMs = elapsedMs + 1; + } + } + + static void FireEventsForDocument(ProjectCodeModel projectCodeModel, string filename) + { + if (projectCodeModel.TryGetCachedFileCodeModel(filename, out var fileCodeModelHandle)) + fileCodeModelHandle.Object.FireEvents(); } // Returns true if any keyboard or mouse button input is pending on the message queue. @@ -153,7 +196,25 @@ static bool IsInputPending() } } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void EnsureServicesCreated() + { + // Ensure MEF services used by the code model are initially obtained on a background thread. + // This code avoids allocations where possible. + // https://github.com/dotnet/roslyn/issues/54159 + using var _ = PooledHashSet.GetInstance(out var previousLanguages); + foreach (var projectState in _visualStudioWorkspace.CurrentSolution.SolutionState.SortedProjectStates) + { + // Avoid duplicate calls if the language has been seen + if (previousLanguages.Add(projectState.Language)) + { + projectState.LanguageServices.GetService(); + projectState.LanguageServices.GetService(); + projectState.LanguageServices.GetService(); + } + } + } + + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { // Events that can't change existing code model items. Can just ignore them. switch (e.Kind) @@ -170,27 +231,9 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) case WorkspaceChangeKind.AnalyzerConfigDocumentReloaded: case WorkspaceChangeKind.AnalyzerConfigDocumentChanged: return; - case WorkspaceChangeKind.DocumentRemoved: - case WorkspaceChangeKind.DocumentChanged: - // Fast path when we know we affected a document that could have had code model elements in it. No - // need to do a solution diff in this case. - _documentsToFireEventsFor.AddWork(e.DocumentId!); - return; } - // Other type of event that could indicate a doc change/removal. Have to actually analyze the change to - // determine what we should do here. - - var changes = e.OldSolution.GetChanges(e.NewSolution); - - foreach (var project in changes.GetRemovedProjects()) - _documentsToFireEventsFor.AddWork(project.DocumentIds); - - foreach (var projectChange in changes.GetProjectChanges()) - { - _documentsToFireEventsFor.AddWork(projectChange.GetRemovedDocuments()); - _documentsToFireEventsFor.AddWork(projectChange.GetChangedDocuments()); - } + _workspaceChangeEventsToFireEventsFor.AddWork(e); } public IProjectCodeModel CreateProjectCodeModel(ProjectId id, ICodeModelInstanceFactory codeModelInstanceFactory) diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/AnalyzerItem/AnalyzerItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/AnalyzerItem/AnalyzerItemSource.cs index 53641c4e9701c..0b97c8eb01928 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/AnalyzerItem/AnalyzerItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/AnalyzerItem/AnalyzerItemSource.cs @@ -37,6 +37,8 @@ internal sealed class AnalyzerItemSource : IAttachedCollectionSource private Workspace Workspace => _analyzersFolder.Workspace; private ProjectId ProjectId => _analyzersFolder.ProjectId; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + public AnalyzerItemSource( AnalyzersFolderItem analyzersFolder, IAnalyzersCommandHandler commandHandler, @@ -51,7 +53,7 @@ public AnalyzerItemSource( listenerProvider.GetListener(FeatureAttribute.SourceGenerators), _cancellationTokenSource.Token); - this.Workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = this.Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); // Kick off the initial work to determine the starting set of items. _workQueue.AddWork(); @@ -64,7 +66,7 @@ public AnalyzerItemSource( public IEnumerable Items => _items; - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { switch (e.Kind) { @@ -93,7 +95,8 @@ private async ValueTask ProcessQueueAsync(CancellationToken cancellationToken) var project = this.Workspace.CurrentSolution.GetProject(this.ProjectId); if (project is null) { - this.Workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; _cancellationTokenSource.Cancel(); diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticAndGeneratorItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticAndGeneratorItemSource.cs index 2197f03fa4d48..f1238dbe9ec0b 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticAndGeneratorItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticAndGeneratorItemSource.cs @@ -37,6 +37,8 @@ internal abstract partial class BaseDiagnosticAndGeneratorItemSource : IAttached protected ProjectId ProjectId { get; } protected IAnalyzersCommandHandler CommandHandler { get; } + private WorkspaceEventRegistration? _workspaceChangedDisposer; + /// /// The analyzer reference that has been found. Once it's been assigned a non-null value, it'll never be assigned /// again. @@ -77,7 +79,7 @@ protected AnalyzerReference? AnalyzerReference // Listen for changes that would affect the set of analyzers/generators in this reference, and kick off work // to now get the items for this source. - Workspace.WorkspaceChanged += OnWorkspaceChanged; + _workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); _workQueue.AddWork(); } } @@ -101,7 +103,8 @@ private async ValueTask ProcessQueueAsync(CancellationToken cancellationToken) var project = this.Workspace.CurrentSolution.GetProject(this.ProjectId); if (project is null || !project.AnalyzerReferences.Contains(analyzerReference)) { - this.Workspace.WorkspaceChanged -= OnWorkspaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; _cancellationTokenSource.Cancel(); @@ -204,7 +207,7 @@ async Task> GetIdentitiesAsync() } } - private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { switch (e.Kind) { diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs index 2d7c2aa84cc51..b1c1cf5648f92 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/CpsDiagnosticItemSource.cs @@ -20,6 +20,8 @@ internal sealed partial class CpsDiagnosticItemSource : BaseDiagnosticAndGenerat private readonly IVsHierarchyItem _item; private readonly string _projectDirectoryPath; + private WorkspaceEventRegistration? _workspaceChangedDisposer; + public event PropertyChangedEventHandler? PropertyChanged; public CpsDiagnosticItemSource( @@ -46,7 +48,9 @@ public CpsDiagnosticItemSource( // then connect to it. if (workspace.CurrentSolution.ContainsProject(projectId)) { - Workspace.WorkspaceChanged += OnWorkspaceChangedLookForAnalyzer; + // Main thread dependency as OnWorkspaceChangedLookForAnalyzer accesses the IVsHierarchy + // and fires the PropertyChanged event + _workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChangedLookForAnalyzer, WorkspaceEventOptions.RequiresMainThreadOptions); item.PropertyChanged += IVsHierarchyItem_PropertyChanged; // Now that we've subscribed, check once more in case we missed the event @@ -63,7 +67,9 @@ public CpsDiagnosticItemSource( private void UnsubscribeFromEvents() { - Workspace.WorkspaceChanged -= OnWorkspaceChangedLookForAnalyzer; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; + _item.PropertyChanged -= IVsHierarchyItem_PropertyChanged; } @@ -80,7 +86,7 @@ private void IVsHierarchyItem_PropertyChanged(object sender, PropertyChangedEven public override object SourceItem => _item; - private void OnWorkspaceChangedLookForAnalyzer(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChangedLookForAnalyzer(WorkspaceChangeEventArgs e) { // If the project has gone away in this change, it's not coming back, so we can stop looking at this point if (!e.NewSolution.ContainsProject(ProjectId)) diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/SourceGeneratedFileItems/SourceGeneratedFileItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/SourceGeneratedFileItems/SourceGeneratedFileItemSource.cs index 216cf417590c7..9ec77585af733 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/SourceGeneratedFileItems/SourceGeneratedFileItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/SourceGeneratedFileItems/SourceGeneratedFileItemSource.cs @@ -43,6 +43,7 @@ internal sealed class SourceGeneratedFileItemSource( private readonly CancellationSeries _cancellationSeries = new(); private ResettableDelay? _resettableDelay; + private WorkspaceEventRegistration? _workspaceChangedDisposer; public object SourceItem => _parentGeneratorItem; @@ -167,14 +168,13 @@ public void BeforeExpand() // in AfterCollapse, and _then_ subscribed here again. cancellationToken.ThrowIfCancellationRequested(); - _workspace.WorkspaceChanged += OnWorkpaceChanged; + _workspaceChangedDisposer = _workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged); if (_workspace.CurrentSolution != solution) { // The workspace changed while we were doing our initial population, so // refresh it. We'll just call our OnWorkspaceChanged event handler // so this looks like any other change. - OnWorkpaceChanged(this, - new WorkspaceChangeEventArgs(WorkspaceChangeKind.SolutionChanged, solution, _workspace.CurrentSolution)); + OnWorkspaceChanged(new WorkspaceChangeEventArgs(WorkspaceChangeKind.SolutionChanged, solution, _workspace.CurrentSolution)); } } }, @@ -192,12 +192,13 @@ private void StopUpdating() lock (_gate) { _cancellationSeries.CreateNext(new CancellationToken(canceled: true)); - _workspace.WorkspaceChanged -= OnWorkpaceChanged; + _workspaceChangedDisposer?.Dispose(); + _workspaceChangedDisposer = null; _resettableDelay = null; } } - private void OnWorkpaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(WorkspaceChangeEventArgs e) { if (!e.NewSolution.ContainsProject(_parentGeneratorItem.ProjectId)) { @@ -216,7 +217,7 @@ private void OnWorkpaceChanged(object sender, WorkspaceChangeEventArgs e) { // Time to start the work all over again. We'll ensure any previous work is cancelled var cancellationToken = _cancellationSeries.CreateNext(); - var asyncToken = _asyncListener.BeginAsyncOperation(nameof(SourceGeneratedFileItemSource) + "." + nameof(OnWorkpaceChanged)); + var asyncToken = _asyncListener.BeginAsyncOperation(nameof(SourceGeneratedFileItemSource) + "." + nameof(OnWorkspaceChanged)); // We're going to go with a really long delay: once the user expands this we will keep it updated, but it's fairly // unlikely to change in a lot of cases if a generator only produces a stable set of names. diff --git a/src/Workspaces/Core/Portable/SemanticModelReuse/SemanticModelWorkspaceServiceFactory.SemanticModelWorkspaceService.cs b/src/Workspaces/Core/Portable/SemanticModelReuse/SemanticModelWorkspaceServiceFactory.SemanticModelWorkspaceService.cs index 5da26249d620b..8be7d9d051cdb 100644 --- a/src/Workspaces/Core/Portable/SemanticModelReuse/SemanticModelWorkspaceServiceFactory.SemanticModelWorkspaceService.cs +++ b/src/Workspaces/Core/Portable/SemanticModelReuse/SemanticModelWorkspaceServiceFactory.SemanticModelWorkspaceService.cs @@ -61,7 +61,7 @@ private sealed class SemanticModelReuseWorkspaceService : ISemanticModelReuseWor public SemanticModelReuseWorkspaceService(Workspace workspace) { _workspace = workspace; - _workspace.WorkspaceChanged += (_, e) => + _workspace.RegisterWorkspaceChangedHandler((e) => { // if our map points at documents not in the current solution, then we want to clear things out. // this way we don't hold onto semantic models past, say, the c#/vb solutions closing. @@ -78,7 +78,7 @@ public SemanticModelReuseWorkspaceService(Workspace workspace) return; } } - }; + }); } public async ValueTask ReuseExistingSpeculativeModelAsync(Document document, SyntaxNode node, CancellationToken cancellationToken)