Skip to content

Commit 83fba2a

Browse files
Avoid acquiring the workspace lock on the UI thread if unneeded
If the project system tells us about a file that's open and we are told about it on a background thread, we connect the file to the workspace on that background thread but then schedule work to the UI thread to ensure we correctly connect it to the right context (linked files, shared projects, multitargeting, etc.) That code on the UI thread then acquires the workspace lock, and we're seeing cases in the wild where that lock acquisition is taking longer than we'd like. Although there are deeper fixes we can do there, there's one trivial fix we can make: don't schedule the work to the UI thread unless the file is in at least two projects. Otherwise, we don't actually need to figure out the context in the first place and it's just wasting time. An entirely unscientific analysis of a handful of traces from users hitting this, there weren't any cases where the file was actually in more than one project, so this should have a nice improvement overall. Closes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2471561
1 parent cdcc3c4 commit 83fba2a

File tree

1 file changed

+30
-12
lines changed

1 file changed

+30
-12
lines changed

src/VisualStudio/Core/Def/ProjectSystem/VisualStudioWorkspaceImpl.OpenFileTracker.cs

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,16 @@ public sealed partial class OpenFileTracker : IOpenTextBufferEventListener
4949
/// </summary>
5050
private readonly MultiDictionary<string, IReferenceCountedDisposable<ICacheEntry<IVsHierarchy, HierarchyEventSink>>> _watchedHierarchiesForDocumentMoniker = [];
5151

52+
#endregion
53+
5254
/// <summary>
5355
/// Boolean flag to indicate if any <see cref="TextDocument"/> has been opened in the workspace.
5456
/// </summary>
55-
private bool _anyDocumentOpened;
56-
57-
#endregion
57+
/// <remarks>
58+
/// This is written on the UI thread once it's been enabled, since the enablement can only happen there. It might be read on other threads in deciding to
59+
/// do the work on the UI thread, but that's not a problem since <see cref="EnsureSuggestedActionsSourceProviderEnabled"/> is idempotent.
60+
/// </remarks>
61+
private bool _suggestedActionSourceProviderEnabled;
5862

5963
private OpenFileTracker(VisualStudioWorkspaceImpl workspace, ProjectSystemProjectFactory projectSystemProjectFactory, IComponentModel componentModel)
6064
{
@@ -109,9 +113,9 @@ private void EnsureSuggestedActionsSourceProviderEnabled()
109113
{
110114
_workspace._threadingContext.ThrowIfNotOnUIThread();
111115

112-
if (!_anyDocumentOpened)
116+
if (!_suggestedActionSourceProviderEnabled)
113117
{
114-
_anyDocumentOpened = true;
118+
_suggestedActionSourceProviderEnabled = true;
115119

116120
// First document opened in the workspace.
117121
// We enable quick actions from SuggestedActionsSourceProvider via an editor option.
@@ -369,22 +373,36 @@ public Task CheckForAddedFileBeingOpenMaybeAsync(bool useAsync, ImmutableArray<s
369373
// and if it was actually open, we'll schedule an update asynchronously.
370374
if (TryOpeningDocumentsForFilePathCore(w, newFileName, textBuffer, hierarchy: null))
371375
{
372-
// The files are now tied to the buffer, but let's schedule work to correctly update the context.
373-
var token = _asynchronousOperationListener.BeginAsyncOperation(nameof(CheckForAddedFileBeingOpenMaybeAsync));
374-
UpdateContextAfterOpenAsync(newFileName).CompletesAsyncOperation(token);
376+
// We've opened the document, but we only have to go to the UI thread if either:
377+
//
378+
// 1. We have multiple documents with the same file path, so we need to ensure the context is actually correct.
379+
// 2. We haven't enabled the suggested actions source provider, which needs to be done once.
380+
var needsContextUpdate = w.CurrentSolution.GetDocumentIdsWithFilePath(newFileName).Length >= 2;
381+
if (needsContextUpdate || !_suggestedActionSourceProviderEnabled)
382+
{
383+
var token = _asynchronousOperationListener.BeginAsyncOperation(nameof(UpdateContextAfterOpenAndEnableSuggestedActionsSourceProviderAsync));
384+
385+
// Update the context on the UI thread if necessary, and enable the suggested actions source provider no matter what.
386+
UpdateContextAfterOpenAndEnableSuggestedActionsSourceProviderAsync(needsContextUpdate ? newFileName : null).CompletesAsyncOperation(token);
387+
}
375388
}
376389
}
377390
}
378391
}
379392
}).AsTask();
380393
}
381394

382-
private async Task UpdateContextAfterOpenAsync(string filePath)
395+
/// <param name="filePath">The file path to update the context on, or null if we're just calling this to ensure suggested actions are enabled.</param>
396+
private async Task UpdateContextAfterOpenAndEnableSuggestedActionsSourceProviderAsync(string? filePath)
383397
{
384398
await _workspace._threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
385-
var hierarchy = _openTextBufferProvider.GetDocumentHierarchy(filePath);
386-
if (hierarchy != null)
387-
RefreshContextForMoniker(filePath, hierarchy);
399+
400+
if (filePath != null)
401+
{
402+
var hierarchy = _openTextBufferProvider.GetDocumentHierarchy(filePath);
403+
if (hierarchy != null)
404+
RefreshContextForMoniker(filePath, hierarchy);
405+
}
388406

389407
EnsureSuggestedActionsSourceProviderEnabled();
390408
}

0 commit comments

Comments
 (0)