Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move to a semaphoreslim in the workspace. #51215

Closed
wants to merge 2 commits into from
Closed

Move to a semaphoreslim in the workspace. #51215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb502c2
Switch to a semaphoreslim
CyrusNajmabadi Feb 13, 2021
05a3950
Adding async helpers.
CyrusNajmabadi Feb 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public VisualStudioProject CreateAndAddToWorkspace(string projectSystemName, str
w.OnProjectAdded(projectInfo);
}
_visualStudioWorkspaceImpl.RefreshProjectExistsUIContextForLanguage(language);
_visualStudioWorkspaceImpl.RefreshProjectExistsUIContextForLanguage_NoLock(language);
});

return project;
Expand Down
133 changes: 79 additions & 54 deletions src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal abstract partial class VisualStudioWorkspaceImpl : VisualStudioWorkspac

private readonly ITextBufferCloneService _textBufferCloneService;

private readonly object _gate = new();
private readonly SemaphoreSlim _gate = new(initialCount: 1);

/// <summary>
/// A <see cref="ForegroundThreadAffinitizedObject"/> to make assertions that stuff is on the right thread.
Expand Down Expand Up @@ -199,7 +199,7 @@ public async Task InitializeUIAffinitizedServicesAsync(IAsyncServiceProvider asy
var openFileTracker = await OpenFileTracker.CreateAsync(this, asyncServiceProvider).ConfigureAwait(true);

// Update our fields first, so any asynchronous work that needs to use these is able to see the service.
lock (_gate)
using (_gate.DisposableWait())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not DisposableWaitAsync?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was initially making just a straight translation. but we can def update tehse calls to be DisposableWaitAsync as well .

{
_openFileTracker = openFileTracker;
}
Expand All @@ -215,7 +215,7 @@ public void ProcessQueuedWorkOnUIThread()

internal void AddProjectToInternalMaps(VisualStudioProject project, IVsHierarchy? hierarchy, Guid guid, string projectSystemName)
{
lock (_gate)
using (_gate.DisposableWait())
{
_projectToHierarchyMap = _projectToHierarchyMap.Add(project.Id, hierarchy);
_projectToGuidMap = _projectToGuidMap.Add(project.Id, guid);
Expand All @@ -225,23 +225,23 @@ internal void AddProjectToInternalMaps(VisualStudioProject project, IVsHierarchy

internal void AddProjectRuleSetFileToInternalMaps(VisualStudioProject project, Func<string?> ruleSetFilePathFunc)
{
lock (_gate)
using (_gate.DisposableWait())
{
_projectToRuleSetFilePath.Add(project.Id, ruleSetFilePathFunc);
}
}

internal void AddDocumentToDocumentsNotFromFiles(DocumentId documentId)
{
lock (_gate)
using (_gate.DisposableWait())
{
_documentsNotFromFiles = _documentsNotFromFiles.Add(documentId);
}
}

internal void RemoveDocumentToDocumentsNotFromFiles(DocumentId documentId)
{
lock (_gate)
using (_gate.DisposableWait())
{
_documentsNotFromFiles = _documentsNotFromFiles.Remove(documentId);
}
Expand Down Expand Up @@ -276,7 +276,7 @@ internal VisualStudioProjectTracker ProjectTracker

internal VisualStudioProject? GetProjectWithHierarchyAndName(IVsHierarchy hierarchy, string projectName)
{
lock (_gate)
using (_gate.DisposableWait())
{
if (_projectSystemNameToProjectsMap.TryGetValue(projectName, out var projects))
{
Expand All @@ -300,7 +300,7 @@ internal VisualStudioProjectTracker ProjectTracker
// Solution Explorer in the SolutionExplorerShim, where if we just could more directly get to the rule set file it'd simplify this.
internal override string? TryGetRuleSetPathForProject(ProjectId projectId)
{
lock (_gate)
using (_gate.DisposableWait())
{
if (_projectToRuleSetFilePath.TryGetValue(projectId, out var ruleSetPathFunc))
{
Expand Down Expand Up @@ -405,7 +405,7 @@ internal bool IsCPSProject(ProjectId projectId)

internal bool IsPrimaryProject(ProjectId projectId)
{
lock (_gate)
using (_gate.DisposableWait())
{
foreach (var (_, projects) in this._projectSystemNameToProjectsMap)
{
Expand Down Expand Up @@ -1317,7 +1317,7 @@ internal override Guid GetProjectGuid(ProjectId projectId)

internal string? TryGetDependencyNodeTargetIdentifier(ProjectId projectId)
{
lock (_gate)
using (_gate.DisposableWait())
{
return _projectToDependencyNodeTargetIdentifier.GetValueOrDefault(projectId, defaultValue: null);
}
Expand All @@ -1330,7 +1330,7 @@ internal override void SetDocumentContext(DocumentId documentId)
// Note: this method does not actually call into any workspace code here to change the workspace's context. The assumption is updating the running document table or
// IVsHierarchies will raise the appropriate events which we are subscribed to.

lock (_gate)
using (_gate.DisposableWait())
{
var hierarchy = GetHierarchy(documentId.ProjectId);
if (hierarchy == null)
Expand Down Expand Up @@ -1517,19 +1517,28 @@ internal override bool CanAddProjectReference(ProjectId referencingProject, Proj
/// </summary>
public void ApplyChangeToWorkspace(Action<Workspace> action)
{
lock (_gate)
using (_gate.DisposableWait())
{
action(this);
}
}

/// <inheritdoc cref="ApplyChangeToWorkspace(Action{Workspace})"/>
public async Task ApplyChangeToWorkspaceAsync(Func<Workspace, Task> action)
{
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
await action(this).ConfigureAwait(false);
}
}

/// <summary>
/// Applies a solution transformation to the workspace and triggers workspace changed event for specified <paramref name="projectId"/>.
/// The transformation shall only update the project of the solution with the specified <paramref name="projectId"/>.
/// </summary>
public void ApplyChangeToWorkspace(ProjectId projectId, Func<CodeAnalysis.Solution, CodeAnalysis.Solution> solutionTransformation)
{
lock (_gate)
using (_gate.DisposableWait())
{
SetCurrentSolution(solutionTransformation, WorkspaceChangeKind.ProjectChanged, projectId);
}
Expand All @@ -1542,43 +1551,60 @@ public void ApplyChangeToWorkspace(ProjectId projectId, Func<CodeAnalysis.Soluti
/// method could be moved down to the core Workspace layer and then could use the synchronization lock there.</remarks>
public void ApplyBatchChangeToWorkspace(Func<CodeAnalysis.Solution, SolutionChangeAccumulator> mutation)
{
lock (_gate)
using (_gate.DisposableWait())
{
var oldSolution = this.CurrentSolution;
var solutionChangeAccumulator = mutation(oldSolution);

if (!solutionChangeAccumulator.HasChange)
{
return;
}
ApplyBatchChangeToWorkspaceWorker(oldSolution, solutionChangeAccumulator);
}
}

foreach (var documentId in solutionChangeAccumulator.DocumentIdsRemoved)
{
this.ClearDocumentData(documentId);
}
/// <inheritdoc cref="ApplyBatchChangeToWorkspace"/>
public async Task ApplyBatchChangeToWorkspaceAsync(Func<CodeAnalysis.Solution, Task<SolutionChangeAccumulator>> mutation)
{
using (await _gate.DisposableWaitAsync().ConfigureAwait(false))
{
var oldSolution = this.CurrentSolution;
var solutionChangeAccumulator = await mutation(oldSolution).ConfigureAwait(false);

SetCurrentSolution(solutionChangeAccumulator.Solution);
RaiseWorkspaceChangedEventAsync(
solutionChangeAccumulator.WorkspaceChangeKind,
oldSolution,
solutionChangeAccumulator.Solution,
solutionChangeAccumulator.WorkspaceChangeProjectId,
solutionChangeAccumulator.WorkspaceChangeDocumentId);
ApplyBatchChangeToWorkspaceWorker(oldSolution, solutionChangeAccumulator);
}
}

private void ApplyBatchChangeToWorkspaceWorker(CodeAnalysis.Solution oldSolution, SolutionChangeAccumulator solutionChangeAccumulator)
{
if (!solutionChangeAccumulator.HasChange)
{
return;
}

foreach (var documentId in solutionChangeAccumulator.DocumentIdsRemoved)
{
this.ClearDocumentData(documentId);
}

SetCurrentSolution(solutionChangeAccumulator.Solution);
RaiseWorkspaceChangedEventAsync(
solutionChangeAccumulator.WorkspaceChangeKind,
oldSolution,
solutionChangeAccumulator.Solution,
solutionChangeAccumulator.WorkspaceChangeProjectId,
solutionChangeAccumulator.WorkspaceChangeDocumentId);
}

private readonly Dictionary<ProjectId, ProjectReferenceInformation> _projectReferenceInfoMap = new();

private ProjectReferenceInformation GetReferenceInfo_NoLock(ProjectId projectId)
{
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

return _projectReferenceInfoMap.GetOrAdd(projectId, _ => new ProjectReferenceInformation());
}

protected internal override void OnProjectRemoved(ProjectId projectId)
{
lock (_gate)
using (_gate.DisposableWait())
{
var languageName = CurrentSolution.GetRequiredProject(projectId).Language;

Expand Down Expand Up @@ -1618,7 +1644,7 @@ protected internal override void OnProjectRemoved(ProjectId projectId)
_threadingContext.RunWithShutdownBlockAsync(async cancellationToken =>
{
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
RefreshProjectExistsUIContextForLanguage(languageName);
RefreshProjectExistsUIContextForLanguage_NoLock(languageName);
});
}
}
Expand All @@ -1639,7 +1665,7 @@ private sealed class ProjectReferenceInformation

public void AddProjectOutputPath(ProjectId projectId, string outputPath)
{
lock (_gate)
using (_gate.DisposableWait())
{
var projectReferenceInformation = GetReferenceInfo_NoLock(projectId);

Expand Down Expand Up @@ -1686,7 +1712,7 @@ public void AddProjectOutputPath(ProjectId projectId, string outputPath)
Constraint = "Avoid calling " + nameof(CodeAnalysis.Solution.GetProject) + " to avoid realizing all projects.")]
private void ConvertMetadataReferencesToProjectReferences_NoLock(ProjectId projectId, string outputPath)
{
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

var modifiedSolution = this.CurrentSolution;
using var _ = PooledHashSet<ProjectId>.GetInstance(out var projectIdsChanged);
Expand Down Expand Up @@ -1726,7 +1752,7 @@ private void ConvertMetadataReferencesToProjectReferences_NoLock(ProjectId proje
Constraint = "Avoid calling " + nameof(CodeAnalysis.Solution.GetProject) + " to avoid realizing all projects.")]
private bool CanConvertMetadataReferenceToProjectReference_NoLock(ProjectId projectIdWithMetadataReference, ProjectId referencedProjectId)
{
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

// PERF: call GetProjectState instead of GetProject, otherwise creating a new project might force all
// Project instances to get created.
Expand Down Expand Up @@ -1770,7 +1796,7 @@ private bool CanConvertMetadataReferenceToProjectReference_NoLock(ProjectId proj
Constraint = "Update ConvertedProjectReferences in place to avoid duplicate list allocations.")]
private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId projectId, string outputPath)
{
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

var modifiedSolution = this.CurrentSolution;
using var _ = PooledHashSet<ProjectId>.GetInstance(out var projectIdsChanged);
Expand Down Expand Up @@ -1816,7 +1842,7 @@ private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId proje
{
// Any conversion to or from project references must be done under the global workspace lock,
// since that needs to be coordinated with updating all projects simultaneously.
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

if (_projectsByOutputPath.TryGetValue(path, out var ids) && ids.Distinct().Count() == 1)
{
Expand Down Expand Up @@ -1848,7 +1874,7 @@ private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId proje
{
// Any conversion to or from project references must be done under the global workspace lock,
// since that needs to be coordinated with updating all projects simultaneously.
Debug.Assert(Monitor.IsEntered(_gate));
Debug.Assert(_gate.CurrentCount == 0);

var projectReferenceInformation = GetReferenceInfo_NoLock(referencingProject);
foreach (var convertedProject in projectReferenceInformation.ConvertedProjectReferences)
Expand Down Expand Up @@ -1892,7 +1918,7 @@ private void SetSolutionAndRaiseWorkspaceChanged_NoLock(CodeAnalysis.Solution mo

public void RemoveProjectOutputPath(ProjectId projectId, string outputPath)
{
lock (_gate)
using (_gate.DisposableWait())
{
var projectReferenceInformation = GetReferenceInfo_NoLock(projectId);
if (!projectReferenceInformation.OutputPaths.Contains(outputPath))
Expand Down Expand Up @@ -1934,7 +1960,7 @@ public void RemoveProjectOutputPath(ProjectId projectId, string outputPath)

private void RefreshMetadataReferencesForFile(object sender, string fullFilePath)
{
lock (_gate)
using (_gate.DisposableWait())
{
var newSolution = CurrentSolution;
using var _ = PooledHashSet<ProjectId>.GetInstance(out var changedProjectIds);
Expand Down Expand Up @@ -1983,37 +2009,36 @@ internal void EnsureDocumentOptionProvidersInitialized()

internal void SetMaxLanguageVersion(ProjectId projectId, string? maxLanguageVersion)
{
lock (_gate)
using (_gate.DisposableWait())
{
_projectToMaxSupportedLangVersionMap[projectId] = maxLanguageVersion;
}
}

internal void SetDependencyNodeTargetIdentifier(ProjectId projectId, string targetIdentifier)
{
lock (_gate)
using (_gate.DisposableWait())
{
_projectToDependencyNodeTargetIdentifier[projectId] = targetIdentifier;
}
}

internal void RefreshProjectExistsUIContextForLanguage(string language)
internal void RefreshProjectExistsUIContextForLanguage_NoLock(string language)
{
// We must assert the call is on the foreground as setting UIContext.IsActive would otherwise do a COM RPC.
_foregroundObject.AssertIsForeground();

lock (_gate)
{
var uiContext =
_languageToProjectExistsUIContext.GetOrAdd(
language,
l => Services.GetLanguageServices(l).GetService<IProjectExistsUIContextProviderLanguageService>()?.GetUIContext());
// This can only be called when the caller holds the gate.
Contract.ThrowIfFalse(_gate.CurrentCount == 0);
var uiContext =
_languageToProjectExistsUIContext.GetOrAdd(
language,
l => Services.GetLanguageServices(l).GetService<IProjectExistsUIContextProviderLanguageService>()?.GetUIContext());

// UIContexts can be "zombied" if UIContexts aren't supported because we're in a command line build or in other scenarios.
if (uiContext != null && !uiContext.IsZombie)
{
uiContext.IsActive = CurrentSolution.Projects.Any(p => p.Language == language);
}
// UIContexts can be "zombied" if UIContexts aren't supported because we're in a command line build or in other scenarios.
if (uiContext != null && !uiContext.IsZombie)
{
uiContext.IsActive = CurrentSolution.Projects.Any(p => p.Language == language);
}
}
}
Expand Down