提交 46d2fa07 编写于 作者: J Jason Malinowski

Be smarter for how we raise WorkspaceChange events when ending batches

When we completed a batch, we'd always raise a generic ProjectChanged
event, even if the change could be represented more precisely: if we
only added a single Document we could still raise DocumentAdded but
we'd still raise the generic ProjectChanged. This now raises more
precise events, when possible. This should allow for downstream
optimizations: adding a new document won't have to rerun syntax
analyzers on everything else, for example.

There was also a second issue where if the batch was closed and
nothing happened at all, we'd still raise a ProjectChanged. This was
because the old code tried having a special case where if it mutated
the Solution and ended up with the same Solution at the end, it'd
skip the actual change. However there was a bug where if it called
Solution.AddProjectReferences or Solution.AddMetadataReferences and
passed an empty list, it'd make a new solution snapshot. That's not
necessary to do, so we optimize that at the Workspaces layer.

Fixes https://github.com/dotnet/roslyn/issues/34309
上级 b980a389
// 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;
}
}
}
}
......@@ -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) =>
......@@ -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)
......@@ -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);
}
......@@ -420,7 +428,9 @@ private void OnBatchScopeDisposed()
if (projectReference != null)
{
solution = solution.RemoveProjectReference(Id, projectReference);
solutionChanges.UpdateSolutionForProjectAction(
Id,
solutionChanges.Solution.RemoveProjectReference(Id, projectReference));
}
else
{
......@@ -430,32 +440,42 @@ 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);
......@@ -463,12 +483,14 @@ private void OnBatchScopeDisposed()
// 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)
......@@ -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)
{
......@@ -1460,7 +1494,9 @@ public void ReorderFiles(ImmutableArray<string> filePaths)
// Document removing...
foreach (var documentId in _documentsRemovedInBatch)
{
solution = removeDocument(solution, documentId);
solutionChanges.UpdateSolutionForDocumentAction(removeDocument(solutionChanges.Solution, documentId),
removeDocumentChangeKind,
SpecializedCollections.SingletonEnumerable(documentId));
}
ClearAndZeroCapacity(_documentsRemovedInBatch);
......@@ -1468,11 +1504,11 @@ public void ReorderFiles(ImmutableArray<string> filePaths)
// 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)
......
......@@ -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);
}
}
......
......@@ -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)" />
......
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework
Imports Roslyn.Test.Utilities
Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
<[UseExportProvider]>
Public Class WorkspaceChangedEventTests
<WpfTheory>
<CombinatorialData>
Public Async Sub AddingASingleSourceFileRaisesDocumentAdded(addInBatch As Boolean)
Using environment = New TestEnvironment()
Dim project = environment.ProjectFactory.CreateAndAddToWorkspace("Project", LanguageNames.CSharp)
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Using If(addInBatch, project.CreateBatchScope(), Nothing)
project.AddSourceFile("Z:\Test.vb")
End Using
Dim change = Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync())
Assert.Equal(WorkspaceChangeKind.DocumentAdded, change.Kind)
Assert.Equal(project.Id, change.ProjectId)
Assert.Equal(environment.Workspace.CurrentSolution.Projects.Single().DocumentIds.Single(), change.DocumentId)
End Using
End Sub
<WpfFact>
Public Async Sub AddingTwoDocumentsInBatchRaisesProjectChanged()
Using environment = New TestEnvironment()
Dim project = environment.ProjectFactory.CreateAndAddToWorkspace("Project", LanguageNames.CSharp)
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Using project.CreateBatchScope()
project.AddSourceFile("Z:\Test1.vb")
project.AddSourceFile("Z:\Test2.vb")
End Using
Dim change = Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync())
Assert.Equal(WorkspaceChangeKind.ProjectChanged, change.Kind)
Assert.Equal(project.Id, change.ProjectId)
Assert.Null(change.DocumentId)
End Using
End Sub
<WpfTheory>
<CombinatorialData>
Public Async Sub AddingASingleAdditionalFileInABatchRaisesDocumentAdded(addInBatch As Boolean)
Using environment = New TestEnvironment()
Dim project = environment.ProjectFactory.CreateAndAddToWorkspace("Project", LanguageNames.CSharp)
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Using If(addInBatch, project.CreateBatchScope(), Nothing)
project.AddAdditionalFile("Z:\Test.vb")
End Using
Dim change = Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync())
Assert.Equal(WorkspaceChangeKind.AdditionalDocumentAdded, change.Kind)
Assert.Equal(project.Id, change.ProjectId)
Assert.Equal(environment.Workspace.CurrentSolution.Projects.Single().AdditionalDocumentIds.Single(), change.DocumentId)
End Using
End Sub
<WpfTheory>
<CombinatorialData>
Public Async Sub AddingASingleMetadataReferenceRaisesProjectChanged(addInBatch As Boolean)
Using environment = New TestEnvironment()
Dim project = environment.ProjectFactory.CreateAndAddToWorkspace("Project", LanguageNames.CSharp)
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Using If(addInBatch, project.CreateBatchScope(), Nothing)
project.AddMetadataReference("Z:\Test.dll", MetadataReferenceProperties.Assembly)
End Using
Dim change = Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync())
Assert.Equal(WorkspaceChangeKind.ProjectChanged, change.Kind)
Assert.Equal(project.Id, change.ProjectId)
Assert.Null(change.DocumentId)
End Using
End Sub
<WpfFact>
<WorkItem(34309, "https://github.com/dotnet/roslyn/issues/34309")>
Public Async Sub StartingAndEndingBatchWithNoChangesDoesNothing()
Using environment = New TestEnvironment()
Dim project = environment.ProjectFactory.CreateAndAddToWorkspace("Project", LanguageNames.CSharp)
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Dim startingSolution = environment.Workspace.CurrentSolution
project.CreateBatchScope().Dispose()
Assert.Empty(Await workspaceChangeEvents.GetNewChangeEventsAsync())
Assert.Same(startingSolution, environment.Workspace.CurrentSolution)
End Using
End Sub
End Class
End Namespace
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Shared.TestHooks
Imports Roslyn.Test.Utilities
Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework
Friend Class WorkspaceChangeWatcher
Implements IDisposable
Private ReadOnly _environment As TestEnvironment
Private ReadOnly _asynchronousOperationWaiter As IAsynchronousOperationWaiter
Private _changeEvents As New List(Of WorkspaceChangeEventArgs)
Public Sub New(environment As TestEnvironment)
_environment = environment
Dim listenerProvider = environment.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)()
_asynchronousOperationWaiter = listenerProvider.GetWaiter(FeatureAttribute.Workspace)
AddHandler environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged
End Sub
Private Sub OnWorkspaceChanged(sender As Object, e As WorkspaceChangeEventArgs)
_changeEvents.Add(e)
End Sub
Friend Async Function GetNewChangeEventsAsync() As Task(Of IEnumerable(Of WorkspaceChangeEventArgs))
Await _asynchronousOperationWaiter.CreateExpeditedWaitTask()
' Return the events so far, clearing the list if somebody wants to ask for further events
Dim changeEvents = _changeEvents
_changeEvents = New List(Of WorkspaceChangeEventArgs)()
Return changeEvents
End Function
Public Sub Dispose() Implements IDisposable.Dispose
RemoveHandler _environment.Workspace.WorkspaceChanged, AddressOf OnWorkspaceChanged
End Sub
End Class
End Namespace
......@@ -852,6 +852,11 @@ public SolutionState AddProjectReferences(ProjectId projectId, IEnumerable<Proje
throw new ArgumentNullException(nameof(projectReferences));
}
if (!projectReferences.Any())
{
return this;
}
CheckContainsProject(projectId);
foreach (var referencedProject in projectReferences)
......@@ -1067,6 +1072,11 @@ public SolutionState AddAnalyzerReferences(ProjectId projectId, IEnumerable<Anal
throw new ArgumentNullException(nameof(analyzerReferences));
}
if (!analyzerReferences.Any())
{
return this;
}
CheckContainsProject(projectId);
return this.ForkProject(this.GetProjectState(projectId).AddAnalyzerReferences(analyzerReferences));
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册