Skip to content

Commit eb89850

Browse files
authored
Move remaining legacy workspace event listeners over to new APIs. (#78333)
About 4.1% of main thread CPU costs during solution load are in workspace eventing. This moves nearly all those costs to background threads. The majority of the workspace changed handler costs are associated with ProjectCodeModelFactoy. If there were any questions about the event handler needing the main thread, this PR attempts to be safe, and continue to use the main thread. Future PRs can judiciously look at moving event handlers to work on background threads. Test insertion PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/631924 See PR for before/after images of CPU usage characteristics.
1 parent 748b0e6 commit eb89850

File tree

23 files changed

+120
-62
lines changed

23 files changed

+120
-62
lines changed

src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
88
using System.Composition;
9+
using System.Diagnostics;
910
using System.Linq;
1011
using System.Threading;
1112
using System.Threading.Tasks;
@@ -30,14 +31,22 @@ internal sealed class PdbMatchingSourceTextProvider() : IEventListener, IPdbMatc
3031
private bool _isActive;
3132
private int _baselineSolutionVersion;
3233
private readonly Dictionary<string, (DocumentState state, int solutionVersion)> _documentsWithChangedLoaderByPath = [];
34+
private WorkspaceEventRegistration? _workspaceChangedDisposer;
3335

3436
public void StartListening(Workspace workspace)
35-
=> workspace.WorkspaceChanged += WorkspaceChanged;
37+
{
38+
Debug.Assert(_workspaceChangedDisposer == null);
39+
40+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(WorkspaceChanged);
41+
}
3642

3743
public void StopListening(Workspace workspace)
38-
=> workspace.WorkspaceChanged -= WorkspaceChanged;
44+
{
45+
_workspaceChangedDisposer?.Dispose();
46+
_workspaceChangedDisposer = null;
47+
}
3948

40-
private void WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
49+
private void WorkspaceChanged(WorkspaceChangeEventArgs e)
4150
{
4251
if (!_isActive)
4352
{

src/EditorFeatures/Core/EditorConfigSettings/Aggregator/SettingsAggregator.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public SettingsAggregator(
3232
IAsynchronousOperationListener listener)
3333
{
3434
_workspace = workspace;
35-
_workspace.WorkspaceChanged += UpdateProviders;
35+
_ = workspace.RegisterWorkspaceChangedHandler(UpdateProviders);
3636

3737
var currentSolution = _workspace.CurrentSolution.SolutionState;
3838
UpdateProviders(currentSolution);
@@ -48,7 +48,7 @@ public SettingsAggregator(
4848
threadingContext.DisposalToken);
4949
}
5050

51-
private void UpdateProviders(object? sender, WorkspaceChangeEventArgs e)
51+
private void UpdateProviders(WorkspaceChangeEventArgs e)
5252
{
5353
switch (e.Kind)
5454
{

src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ internal sealed partial class InlineRenameSession : IInlineRenameSession, IFeatu
5151
private readonly ITextView _triggerView;
5252
private readonly IDisposable _inlineRenameSessionDurationLogBlock;
5353
private readonly IThreadingContext _threadingContext;
54+
private readonly WorkspaceEventRegistration _workspaceChangedDisposer;
55+
5456
public readonly InlineRenameService RenameService;
5557

5658
private bool _dismissed;
@@ -161,7 +163,9 @@ public InlineRenameSession(
161163
_inlineRenameSessionDurationLogBlock = Logger.LogBlock(FunctionId.Rename_InlineSession, CancellationToken.None);
162164

163165
Workspace = workspace;
164-
Workspace.WorkspaceChanged += OnWorkspaceChanged;
166+
167+
// Requires the main thread due to OnWorkspaceChanged calling the Cancel method
168+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
165169

166170
_textBufferFactoryService = textBufferFactoryService;
167171
_textBufferCloneService = textBufferCloneService;
@@ -383,7 +387,7 @@ private void VerifyNotDismissed()
383387
}
384388
}
385389

386-
private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs args)
390+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs args)
387391
{
388392
if (args.Kind != WorkspaceChangeKind.DocumentChanged)
389393
{
@@ -701,7 +705,8 @@ private void DismissUIAndRollbackEditsAndEndRenameSession_MustBeCalledOnUIThread
701705

702706
void DismissUIAndRollbackEdits()
703707
{
704-
Workspace.WorkspaceChanged -= OnWorkspaceChanged;
708+
_workspaceChangedDisposer.Dispose();
709+
705710
_textBufferAssociatedViewService.SubjectBuffersConnected -= OnSubjectBuffersConnected;
706711

707712
// Reenable completion now that the inline rename session is done

src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.ParseOptionChangedEventSource.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Diagnostics;
56
using Microsoft.CodeAnalysis.Shared.Extensions;
67
using Microsoft.CodeAnalysis.Text;
78
using Microsoft.VisualStudio.Text;
@@ -13,13 +14,21 @@ internal partial class TaggerEventSources
1314
{
1415
private sealed class ParseOptionChangedEventSource(ITextBuffer subjectBuffer) : AbstractWorkspaceTrackingTaggerEventSource(subjectBuffer)
1516
{
17+
private WorkspaceEventRegistration? _workspaceChangedDisposer;
18+
1619
protected override void ConnectToWorkspace(Workspace workspace)
17-
=> workspace.WorkspaceChanged += OnWorkspaceChanged;
20+
{
21+
Debug.Assert(_workspaceChangedDisposer == null);
22+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
23+
}
1824

1925
protected override void DisconnectFromWorkspace(Workspace workspace)
20-
=> workspace.WorkspaceChanged -= OnWorkspaceChanged;
26+
{
27+
_workspaceChangedDisposer?.Dispose();
28+
_workspaceChangedDisposer = null;
29+
}
2130

22-
private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
31+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)
2332
{
2433
if (e.Kind == WorkspaceChangeKind.ProjectChanged)
2534
{

src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.WorkspaceChangedEventSource.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ internal partial class TaggerEventSources
1515
private sealed class WorkspaceChangedEventSource : AbstractWorkspaceTrackingTaggerEventSource
1616
{
1717
private readonly AsyncBatchingWorkQueue _asyncDelay;
18+
private WorkspaceEventRegistration? _workspaceChangedDisposer;
1819

1920
public WorkspaceChangedEventSource(
2021
ITextBuffer subjectBuffer,
@@ -36,17 +37,19 @@ public WorkspaceChangedEventSource(
3637

3738
protected override void ConnectToWorkspace(Workspace workspace)
3839
{
39-
workspace.WorkspaceChanged += OnWorkspaceChanged;
40+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
4041
this.RaiseChanged();
4142
}
4243

4344
protected override void DisconnectFromWorkspace(Workspace workspace)
4445
{
45-
workspace.WorkspaceChanged -= OnWorkspaceChanged;
46+
_workspaceChangedDisposer?.Dispose();
47+
_workspaceChangedDisposer = null;
48+
4649
this.RaiseChanged();
4750
}
4851

49-
private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs eventArgs)
52+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs eventArgs)
5053
=> _asyncDelay.AddWork();
5154
}
5255
}

src/EditorFeatures/Core/SolutionEvents/HostLegacySolutionEventsWorkspaceEventListener.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ internal sealed partial class HostLegacySolutionEventsWorkspaceEventListener : I
3232
private readonly IThreadingContext _threadingContext;
3333
private readonly AsyncBatchingWorkQueue<WorkspaceChangeEventArgs> _eventQueue;
3434

35+
private WorkspaceEventRegistration? _workspaceChangedDisposer;
36+
3537
[ImportingConstructor]
3638
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
3739
public HostLegacySolutionEventsWorkspaceEventListener(
@@ -53,16 +55,19 @@ public void StartListening(Workspace workspace)
5355
// We only support this option to disable crawling in internal speedometer and ddrit perf runs to lower noise.
5456
// It is not exposed to the user.
5557
if (_globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler))
56-
workspace.WorkspaceChanged += OnWorkspaceChanged;
58+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
5759
}
5860

5961
public void StopListening(Workspace workspace)
6062
{
6163
if (_globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler))
62-
workspace.WorkspaceChanged -= OnWorkspaceChanged;
64+
{
65+
_workspaceChangedDisposer?.Dispose();
66+
_workspaceChangedDisposer = null;
67+
}
6368
}
6469

65-
private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
70+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)
6671
{
6772
// Legacy workspace events exist solely to let unit testing continue to work using their own fork of solution
6873
// crawler. As such, they only need events for the project types they care about. Specifically, that is only

src/Features/Core/Portable/Diagnostics/CodeAnalysisDiagnosticAnalyzerService.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
using System.Threading.Tasks;
1212
using Microsoft.CodeAnalysis.Host;
1313
using Microsoft.CodeAnalysis.Host.Mef;
14-
using Microsoft.CodeAnalysis.Shared.Utilities;
15-
using Microsoft.CodeAnalysis.Threading;
1614
using Roslyn.Utilities;
1715

1816
namespace Microsoft.CodeAnalysis.Diagnostics;
@@ -52,10 +50,12 @@ public CodeAnalysisDiagnosticAnalyzerService(
5250
_workspace = workspace;
5351
_diagnosticAnalyzerService = _workspace.Services.GetRequiredService<IDiagnosticAnalyzerService>();
5452

55-
_workspace.WorkspaceChanged += OnWorkspaceChanged;
53+
// Main thread as OnWorkspaceChanged's call to IDiagnosticAnalyzerService.RequestDiagnosticRefresh isn't clear on
54+
// threading requirements
55+
_ = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged, WorkspaceEventOptions.RequiresMainThreadOptions);
5656
}
5757

58-
private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
58+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)
5959
{
6060
switch (e.Kind)
6161
{

src/Features/Core/Portable/Workspace/CompileTimeSolutionProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
6565

6666
public CompileTimeSolutionProvider(Workspace workspace)
6767
{
68-
workspace.WorkspaceChanged += (s, e) =>
68+
_ = workspace.RegisterWorkspaceChangedHandler((e) =>
6969
{
7070
if (e.Kind is WorkspaceChangeKind.SolutionCleared or WorkspaceChangeKind.SolutionRemoved)
7171
{
@@ -79,7 +79,7 @@ public CompileTimeSolutionProvider(Workspace workspace)
7979
_lastCompileTimeSolution = null;
8080
}
8181
}
82-
};
82+
});
8383
}
8484

8585
private static bool IsRazorAnalyzerConfig(TextDocumentState documentState)

src/VisualStudio/Core/Def/DesignerAttribute/VisualStudioDesignerAttributeService.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ internal sealed class VisualStudioDesignerAttributeService :
6060
// deliver them to VS in batches to prevent flooding the UI thread.
6161
private readonly AsyncBatchingWorkQueue<DesignerAttributeData> _projectSystemNotificationQueue;
6262

63+
private WorkspaceEventRegistration? _workspaceChangedDisposer;
64+
6365
[ImportingConstructor]
6466
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
6567
public VisualStudioDesignerAttributeService(
@@ -92,7 +94,7 @@ void IEventListener.StartListening(Workspace workspace)
9294
if (workspace != _workspace)
9395
return;
9496

95-
_workspace.WorkspaceChanged += OnWorkspaceChanged;
97+
_workspaceChangedDisposer = workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
9698
_workQueue.AddWork(cancelExistingWork: true);
9799
}
98100

@@ -101,10 +103,11 @@ void IEventListener.StopListening(Workspace workspace)
101103
if (workspace != _workspace)
102104
return;
103105

104-
_workspace.WorkspaceChanged -= OnWorkspaceChanged;
106+
_workspaceChangedDisposer?.Dispose();
107+
_workspaceChangedDisposer = null;
105108
}
106109

107-
private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
110+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)
108111
{
109112
_workQueue.AddWork(cancelExistingWork: true);
110113
}

src/VisualStudio/Core/Def/Library/ObjectBrowser/AbstractObjectBrowserLibraryManager.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ internal abstract partial class AbstractObjectBrowserLibraryManager : AbstractLi
4141
private ObjectListItem _activeListItem;
4242
private AbstractListItemFactory _listItemFactory;
4343
private readonly object _classMemberGate = new();
44+
private WorkspaceEventRegistration _workspaceChangedDisposer;
4445

4546
protected AbstractObjectBrowserLibraryManager(
4647
string languageName,
@@ -53,7 +54,7 @@ protected AbstractObjectBrowserLibraryManager(
5354
_languageName = languageName;
5455

5556
Workspace = workspace;
56-
Workspace.WorkspaceChanged += OnWorkspaceChanged;
57+
_workspaceChangedDisposer = Workspace.RegisterWorkspaceChangedHandler(OnWorkspaceChanged);
5758

5859
_libraryService = new Lazy<ILibraryService>(() => Workspace.Services.GetLanguageServices(_languageName).GetService<ILibraryService>());
5960
}
@@ -73,9 +74,12 @@ private AbstractListItemFactory GetListItemFactory()
7374
}
7475

7576
public void Dispose()
76-
=> this.Workspace.WorkspaceChanged -= OnWorkspaceChanged;
77+
{
78+
_workspaceChangedDisposer?.Dispose();
79+
_workspaceChangedDisposer = null;
80+
}
7781

78-
private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
82+
private void OnWorkspaceChanged(WorkspaceChangeEventArgs e)
7983
{
8084
switch (e.Kind)
8185
{

0 commit comments

Comments
 (0)