Skip to content

Commit

Permalink
Merge pull request #34564 from jasonmalinowski/raise-precise-workspac…
Browse files Browse the repository at this point in the history 
…e-changes

Be smarter for how we raise WorkspaceChange events when completing batches
  • Loading branch information
@jasonmalinowski
jasonmalinowski authored Mar 29, 2019
2 parents b628534 + 46d2fa0 commit dbfb4e7
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 33 deletions.
106 changes: 106 additions & 0 deletions src/VisualStudio/Core/Def/Implementation/ProjectSystem/SolutionChangeAccumulator.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using Microsoft.CodeAnalysis;

namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
{
/// <summary>
/// A little helper type to hold onto the <see cref="Solution"/> being updated in a batch, which also
/// keeps track of the right <see cref="CodeAnalysis.WorkspaceChangeKind"/> to raise when we are done.
/// </summary>
internal class SolutionChangeAccumulator
{
/// <summary>
/// The kind that encompasses all the changes we've made. It's null if no changes have been made,
/// and <see cref="WorkspaceChangeKind.ProjectChanged"/> or
/// <see cref="WorkspaceChangeKind.SolutionChanged"/> if we can't give a more precise type.
/// </summary>
private WorkspaceChangeKind? _workspaceChangeKind;

public SolutionChangeAccumulator(Solution startingSolution)
{
Solution = startingSolution;
}

public Solution Solution { get; private set; }

public bool HasChange => _workspaceChangeKind.HasValue;
public WorkspaceChangeKind WorkspaceChangeKind => _workspaceChangeKind.Value;

public ProjectId WorkspaceChangeProjectId { get; private set; }
public DocumentId WorkspaceChangeDocumentId { get; private set; }

public void UpdateSolutionForDocumentAction(Solution newSolution, WorkspaceChangeKind changeKind, IEnumerable<DocumentId> documentIds)
{
// If the newSolution is the same as the current solution, there's nothing to actually do
if (Solution == newSolution)
{
return;
}

Solution = newSolution;

foreach (var documentId in documentIds)
{
// If we don't previously have change, this is our new change
if (!_workspaceChangeKind.HasValue)
{
_workspaceChangeKind = changeKind;
WorkspaceChangeProjectId = documentId.ProjectId;
WorkspaceChangeDocumentId = documentId;
}
else
{
// We do have a new change. At this point, the change is spanning multiple documents or projects we
// will coalesce accordingly
if (documentId.ProjectId == WorkspaceChangeProjectId)
{
// It's the same project, at least, so project change it is
_workspaceChangeKind = WorkspaceChangeKind.ProjectChanged;
WorkspaceChangeDocumentId = null;
}
else
{
// Multiple projects have changed, so it's a generic solution change. At this point
// we can bail from the loop, because this is already our most general case.
_workspaceChangeKind = WorkspaceChangeKind.SolutionChanged;
WorkspaceChangeProjectId = null;
WorkspaceChangeDocumentId = null;
break;
}
}
}
}

/// <summary>
/// Should be called to update the solution if there isn't a specific document change kind that should be
/// given to <see cref="UpdateSolutionForDocumentAction"/>
/// </summary>
public void UpdateSolutionForProjectAction(ProjectId projectId, Solution newSolution)
{
// If the newSolution is the same as the current solution, there's nothing to actually do
if (Solution == newSolution)
{
return;
}

Solution = newSolution;

// Since we're changing a project, we definitely have no DocumentId anymore
WorkspaceChangeDocumentId = null;

if (!_workspaceChangeKind.HasValue || WorkspaceChangeProjectId == projectId)
{
// We can count this as a generic project change
_workspaceChangeKind = WorkspaceChangeKind.ProjectChanged;
WorkspaceChangeProjectId = projectId;
}
else
{
_workspaceChangeKind = WorkspaceChangeKind.SolutionChanged;
WorkspaceChangeProjectId = null;
}
}
}
}
88 changes: 62 additions & 26 deletions src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,23 +350,27 @@ private void OnBatchScopeDisposed()
var documentsToOpen = new List<(DocumentId documentId, SourceTextContainer textContainer)>();
var additionalDocumentsToOpen = new List<(DocumentId documentId, SourceTextContainer textContainer)>();

_workspace.ApplyBatchChangeToProject(Id, solution =>
_workspace.ApplyBatchChangeToWorkspace(solution =>
{
solution = _sourceFiles.UpdateSolutionForBatch(
solution,
var solutionChanges = new SolutionChangeAccumulator(startingSolution: solution);

_sourceFiles.UpdateSolutionForBatch(
solutionChanges,
documentFileNamesAdded,
documentsToOpen,
(s, documents) => solution.AddDocuments(documents),
WorkspaceChangeKind.DocumentAdded,
(s, id) =>
{
// Clear any document-specific data now (like open file trackers, etc.). If we called OnRemoveDocument directly this is
// called, but since we're doing this in one large batch we need to do it now.
_workspace.ClearDocumentData(id);
return s.RemoveDocument(id);
});
},
WorkspaceChangeKind.DocumentRemoved);

solution = _additionalFiles.UpdateSolutionForBatch(
solution,
_additionalFiles.UpdateSolutionForBatch(
solutionChanges,
documentFileNamesAdded,
additionalDocumentsToOpen,
(s, documents) =>
Expand All @@ -378,13 +382,15 @@ private void OnBatchScopeDisposed()

return s;
},
WorkspaceChangeKind.AdditionalDocumentAdded,
(s, id) =>
{
// Clear any document-specific data now (like open file trackers, etc.). If we called OnRemoveDocument directly this is
// called, but since we're doing this in one large batch we need to do it now.
_workspace.ClearDocumentData(id);
return s.RemoveAdditionalDocument(id);
});
},
WorkspaceChangeKind.AdditionalDocumentRemoved);

// Metadata reference adding...
if (_metadataReferencesAddedInBatch.Count > 0)
Expand All @@ -407,8 +413,10 @@ private void OnBatchScopeDisposed()
}
}

solution = solution.AddProjectReferences(Id, projectReferencesCreated)
.AddMetadataReferences(Id, metadataReferencesCreated);
solutionChanges.UpdateSolutionForProjectAction(
Id,
solutionChanges.Solution.AddProjectReferences(Id, projectReferencesCreated)
.AddMetadataReferences(Id, metadataReferencesCreated));

ClearAndZeroCapacity(_metadataReferencesAddedInBatch);
}
Expand All @@ -420,7 +428,9 @@ private void OnBatchScopeDisposed()

if (projectReference != null)
{
solution = solution.RemoveProjectReference(Id, projectReference);
solutionChanges.UpdateSolutionForProjectAction(
Id,
solutionChanges.Solution.RemoveProjectReference(Id, projectReference));
}
else
{
Expand All @@ -430,45 +440,57 @@ private void OnBatchScopeDisposed()

_workspace.FileWatchedReferenceFactory.StopWatchingReference(metadataReference);

solution = solution.RemoveMetadataReference(Id, metadataReference);
solutionChanges.UpdateSolutionForProjectAction(
Id,
newSolution: solutionChanges.Solution.RemoveMetadataReference(Id, metadataReference));
}
}

ClearAndZeroCapacity(_metadataReferencesRemovedInBatch);

// Project reference adding...
solution = solution.AddProjectReferences(Id, _projectReferencesAddedInBatch);
solutionChanges.UpdateSolutionForProjectAction(
Id,
newSolution: solutionChanges.Solution.AddProjectReferences(Id, _projectReferencesAddedInBatch));
ClearAndZeroCapacity(_projectReferencesAddedInBatch);

// Project reference removing...
foreach (var projectReference in _projectReferencesRemovedInBatch)
{
solution = solution.RemoveProjectReference(Id, projectReference);
solutionChanges.UpdateSolutionForProjectAction(
Id,
newSolution: solutionChanges.Solution.RemoveProjectReference(Id, projectReference));
}

ClearAndZeroCapacity(_projectReferencesRemovedInBatch);

// Analyzer reference adding...
solution = solution.AddAnalyzerReferences(Id, _analyzersAddedInBatch.Select(a => a.GetReference()));
solutionChanges.UpdateSolutionForProjectAction(
Id,
newSolution: solutionChanges.Solution.AddAnalyzerReferences(Id, _analyzersAddedInBatch.Select(a => a.GetReference())));
ClearAndZeroCapacity(_analyzersAddedInBatch);

// Analyzer reference removing...
foreach (var analyzerReference in _analyzersRemovedInBatch)
{
solution = solution.RemoveAnalyzerReference(Id, analyzerReference.GetReference());
solutionChanges.UpdateSolutionForProjectAction(
Id,
newSolution: solutionChanges.Solution.RemoveAnalyzerReference(Id, analyzerReference.GetReference()));
}

ClearAndZeroCapacity(_analyzersRemovedInBatch);

// Other property modifications...
foreach (var propertyModification in _projectPropertyModificationsInBatch)
{
solution = propertyModification(solution);
solutionChanges.UpdateSolutionForProjectAction(
Id,
propertyModification(solutionChanges.Solution));
}

ClearAndZeroCapacity(_projectPropertyModificationsInBatch);

return solution;
return solutionChanges;
});

foreach (var (documentId, textContainer) in documentsToOpen)
Expand Down Expand Up @@ -1430,20 +1452,32 @@ public void ReorderFiles(ImmutableArray<string> filePaths)
}
else
{
_project._workspace.ApplyBatchChangeToProject(_project.Id, solution => solution.WithProjectDocumentsOrder(_project.Id, documentIds.ToImmutable()));
_project._workspace.ApplyBatchChangeToWorkspace(solution =>
{
var solutionChanges = new SolutionChangeAccumulator(solution);
solutionChanges.UpdateSolutionForProjectAction(
_project.Id,
solutionChanges.Solution.WithProjectDocumentsOrder(_project.Id, documentIds.ToImmutable()));
return solutionChanges;
});
}
}
}

internal Solution UpdateSolutionForBatch(
Solution solution,
internal void UpdateSolutionForBatch(
SolutionChangeAccumulator solutionChanges,
ImmutableArray<string>.Builder documentFileNamesAdded,
List<(DocumentId documentId, SourceTextContainer textContainer)> documentsToOpen,
Func<Solution, ImmutableArray<DocumentInfo>, Solution> addDocuments,
Func<Solution, DocumentId, Solution> removeDocument)
WorkspaceChangeKind addDocumentChangeKind,
Func<Solution, DocumentId, Solution> removeDocument,
WorkspaceChangeKind removeDocumentChangeKind)
{
// Document adding...
solution = addDocuments(solution, _documentsAddedInBatch.ToImmutable());
solutionChanges.UpdateSolutionForDocumentAction(
newSolution: addDocuments(solutionChanges.Solution, _documentsAddedInBatch.ToImmutable()),
changeKind: addDocumentChangeKind,
documentIds: _documentsAddedInBatch.Select(d => d.Id));

foreach (var documentInfo in _documentsAddedInBatch)
{
Expand All @@ -1460,19 +1494,21 @@ internal Solution UpdateSolutionForBatch(
// Document removing...
foreach (var documentId in _documentsRemovedInBatch)
{
solution = removeDocument(solution, documentId);
solutionChanges.UpdateSolutionForDocumentAction(removeDocument(solutionChanges.Solution, documentId),
removeDocumentChangeKind,
SpecializedCollections.SingletonEnumerable(documentId));
}

ClearAndZeroCapacity(_documentsRemovedInBatch);

// Update project's order of documents.
if (_orderedDocumentsInBatch != null)
{
solution = solution.WithProjectDocumentsOrder(_project.Id, _orderedDocumentsInBatch);
solutionChanges.UpdateSolutionForProjectAction(
_project.Id,
solutionChanges.Solution.WithProjectDocumentsOrder(_project.Id, _orderedDocumentsInBatch));
_orderedDocumentsInBatch = null;
}

return solution;
}

private DocumentInfo CreateDocumentInfoFromFileInfo(DynamicFileInfo fileInfo, IEnumerable<string> folders)
Expand Down
17 changes: 10 additions & 7 deletions src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1436,22 +1436,25 @@ public void ApplyChangeToWorkspace(Action<Workspace> action)
/// </summary>
/// <remarks>This is needed to synchronize with <see cref="ApplyChangeToWorkspace(Action{Workspace})" /> to avoid any races. This
/// method could be moved down to the core Workspace layer and then could use the synchronization lock there.</remarks>
/// <param name="projectId">The <see cref="ProjectId" /> to change.</param>
/// <param name="mutation">A function that, given the old <see cref="CodeAnalysis.Project"/> will produce a new one.</param>
public void ApplyBatchChangeToProject(ProjectId projectId, Func<CodeAnalysis.Solution, CodeAnalysis.Solution> mutation)
public void ApplyBatchChangeToWorkspace(Func<CodeAnalysis.Solution, SolutionChangeAccumulator> mutation)
{
lock (_gate)
{
var oldSolution = this.CurrentSolution;
var newSolution = mutation(oldSolution);
var solutionChangeAccumulator = mutation(oldSolution);

if (oldSolution == newSolution)
if (!solutionChangeAccumulator.HasChange)
{
return;
}

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

Expand Down
1 change: 1 addition & 0 deletions src/VisualStudio/Core/Test/Microsoft.VisualStudio.LanguageServices.UnitTests.vbproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
<PackageReference Include="Microsoft.VisualStudio.Progression.Interfaces" Version="$(MicrosoftVisualStudioProgressionInterfacesVersion)" />
<PackageReference Include="Microsoft.VisualStudio.GraphModel" Version="$(MicrosoftVisualStudioGraphModelVersion)" />
<PackageReference Include="Microsoft.VisualStudio.ComponentModelHost" Version="$(MicrosoftVisualStudioComponentModelHostVersion)" />
<PackageReference Include="Xunit.Combinatorial" Version="$(XunitCombinatorialVersion)" PrivateAssets="all" />

<!-- This is needed by the Progression and Graph Model APIs at runtime -->
<PackageReference Include="Microsoft.VisualStudio.Diagnostics.PerformanceProvider" Version="$(MicrosoftVisualStudioDiagnosticsPerformanceProviderVersion)" />
Expand Down
Loading

0 comments on commit dbfb4e7

Please sign in to comment.