From 8cb92eefbdd6f8c59a1f441d7cdb2003e8b887bc Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 23 Aug 2021 07:57:56 -0700 Subject: [PATCH 1/3] Bulk update project when several documents change Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1365014 --- .../Host/RemoteWorkspace.SolutionCreator.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index 4c9794f4a63fc..cb4677b1ae7e3 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -242,6 +242,7 @@ await _assetProvider.GetAssetAsync( { project = await UpdateDocumentsAsync( project, + newProjectChecksums, project.State.DocumentStates.States.Values, oldProjectChecksums.Documents, newProjectChecksums.Documents, @@ -254,6 +255,7 @@ await _assetProvider.GetAssetAsync( { project = await UpdateDocumentsAsync( project, + newProjectChecksums, project.State.AdditionalDocumentStates.States.Values, oldProjectChecksums.AdditionalDocuments, newProjectChecksums.AdditionalDocuments, @@ -266,6 +268,7 @@ await _assetProvider.GetAssetAsync( { project = await UpdateDocumentsAsync( project, + newProjectChecksums, project.State.AnalyzerConfigDocumentStates.States.Values, oldProjectChecksums.AnalyzerConfigDocuments, newProjectChecksums.AnalyzerConfigDocuments, @@ -337,6 +340,7 @@ private async Task UpdateProjectInfoAsync(Project project, Checksum inf private async Task UpdateDocumentsAsync( Project project, + ProjectStateChecksums projectChecksums, IEnumerable existingTextDocumentStates, ChecksumCollection oldChecksums, ChecksumCollection newChecksums, @@ -356,6 +360,14 @@ private async Task UpdateDocumentsAsync( var oldMap = await GetDocumentMapAsync(existingTextDocumentStates, olds.Object).ConfigureAwait(false); var newMap = await GetDocumentMapAsync(_assetProvider, news.Object).ConfigureAwait(false); + // If more than two documents changed during a single update, perform a bulk synchronization on the + // project to avoid large numbers of small synchronization calls during document updates. + // 🔗 https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1365014 + if (newMap.Count > 2) + { + await _assetProvider.SynchronizeProjectAssetsAsync(new[] { projectChecksums.Checksum }, _cancellationToken).ConfigureAwait(false); + } + // added document ImmutableArray.Builder? lazyDocumentsToAdd = null; foreach (var (documentId, newDocumentChecksums) in newMap) From 787530b0f2fef431e19596cd87bbe69817b8e009 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 23 Aug 2021 07:58:23 -0700 Subject: [PATCH 2/3] Add debug assertion to clarify code flow --- .../Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index cb4677b1ae7e3..37d81cffed904 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -479,6 +480,8 @@ private async Task> GetDocumentMa foreach (var kv in documentChecksums) { + Debug.Assert(assetProvider.EnsureCacheEntryIfExists(kv.Item2.Info), "Expected the prior call to GetAssetsAsync to obtain all items for this loop."); + var info = await assetProvider.GetAssetAsync(kv.Item2.Info, _cancellationToken).ConfigureAwait(false); map.Add(info.Id, kv.Item2); } From 93c7e4bab175c7b70f6ac7221d29216d27a8d1ce Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 25 Aug 2021 15:13:29 -0700 Subject: [PATCH 3/3] Add a test for batch synchronizing projects --- .../Services/ServiceHubServicesTests.cs | 87 +++++++++++++++---- 1 file changed, 71 insertions(+), 16 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs b/src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs index 497caeefbe996..b524c5dc4695f 100644 --- a/src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs @@ -280,8 +280,10 @@ await solution.State.GetChecksumAsync(CancellationToken.None), await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None)); } - [Fact] - public async Task TestRemoteHostSynchronizeIncrementalUpdate() + [Theory] + [CombinatorialData] + [WorkItem(1365014, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1365014")] + public async Task TestRemoteHostSynchronizeIncrementalUpdate(bool applyInBatch) { using var workspace = CreateWorkspace(); @@ -301,14 +303,14 @@ await solution.State.GetChecksumAsync(CancellationToken.None), await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None)); // incrementally update - solution = await VerifyIncrementalUpdatesAsync(remoteWorkspace, client, solution, csAddition: " ", vbAddition: " "); + solution = await VerifyIncrementalUpdatesAsync(remoteWorkspace, client, solution, applyInBatch, csAddition: " ", vbAddition: " "); Assert.Equal( await solution.State.GetChecksumAsync(CancellationToken.None), await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None)); // incrementally update - solution = await VerifyIncrementalUpdatesAsync(remoteWorkspace, client, solution, csAddition: "\r\nclass Addition { }", vbAddition: "\r\nClass VB\r\nEnd Class"); + solution = await VerifyIncrementalUpdatesAsync(remoteWorkspace, client, solution, applyInBatch, csAddition: "\r\nclass Addition { }", vbAddition: "\r\nClass VB\r\nEnd Class"); Assert.Equal( await solution.State.GetChecksumAsync(CancellationToken.None), @@ -349,7 +351,7 @@ public void TestRemoteWorkspaceCircularReferences() Assert.NotNull(solution); } - private async Task VerifyIncrementalUpdatesAsync(Workspace remoteWorkspace, RemoteHostClient client, Solution solution, string csAddition, string vbAddition) + private async Task VerifyIncrementalUpdatesAsync(Workspace remoteWorkspace, RemoteHostClient client, Solution solution, bool applyInBatch, string csAddition, string vbAddition) { var remoteSolution = remoteWorkspace.CurrentSolution; var projectIds = solution.ProjectIds; @@ -358,6 +360,7 @@ private async Task VerifyIncrementalUpdatesAsync(Workspace remoteWorks { var projectName = $"Project{i}"; var project = solution.GetProject(projectIds[i]); + var changedDocuments = new List(); var documentIds = project.DocumentIds; for (var j = 0; j < documentIds.Count; j++) @@ -365,12 +368,31 @@ private async Task VerifyIncrementalUpdatesAsync(Workspace remoteWorks var documentName = $"Document{j}"; var currentSolution = UpdateSolution(solution, projectName, documentName, csAddition, vbAddition); - await UpdatePrimaryWorkspace(client, currentSolution); + changedDocuments.Add(documentName); + + solution = currentSolution; + + if (!applyInBatch) + { + await UpdateAndVerifyAsync(); + } + } + + if (applyInBatch) + { + await UpdateAndVerifyAsync(); + } + + async Task UpdateAndVerifyAsync() + { + var documentNames = changedDocuments.ToImmutableArray(); + changedDocuments.Clear(); + + await UpdatePrimaryWorkspace(client, solution); var currentRemoteSolution = remoteWorkspace.CurrentSolution; - VerifyStates(remoteSolution, currentRemoteSolution, projectName, documentName); + VerifyStates(remoteSolution, currentRemoteSolution, projectName, documentNames); - solution = currentSolution; remoteSolution = currentRemoteSolution; Assert.Equal( @@ -382,17 +404,17 @@ await solution.State.GetChecksumAsync(CancellationToken.None), return solution; } - private static void VerifyStates(Solution solution1, Solution solution2, string projectName, string documentName) + private static void VerifyStates(Solution solution1, Solution solution2, string projectName, ImmutableArray documentNames) { Assert.True(solution1.Workspace is RemoteWorkspace); Assert.True(solution2.Workspace is RemoteWorkspace); SetEqual(solution1.ProjectIds, solution2.ProjectIds); - var (project, document) = GetProjectAndDocument(solution1, projectName, documentName); + var (project, documents) = GetProjectAndDocuments(solution1, projectName, documentNames); var projectId = project.Id; - var documentId = document.Id; + var documentIds = documents.SelectAsArray(document => document.Id); var projectIds = solution1.ProjectIds; for (var i = 0; i < projectIds.Count; i++) @@ -406,12 +428,12 @@ private static void VerifyStates(Solution solution1, Solution solution2, string { SetEqual(solution1.GetProject(currentProjectId).DocumentIds, solution2.GetProject(currentProjectId).DocumentIds); - var documentIds = solution1.GetProject(currentProjectId).DocumentIds; - for (var j = 0; j < documentIds.Count; j++) + var documentIdsInProject = solution1.GetProject(currentProjectId).DocumentIds; + for (var j = 0; j < documentIdsInProject.Count; j++) { - var currentDocumentId = documentIds[j]; + var currentDocumentId = documentIdsInProject[j]; - var documentStateShouldSame = documentId != currentDocumentId; + var documentStateShouldSame = !documentIds.Contains(currentDocumentId); Assert.Equal(documentStateShouldSame, object.ReferenceEquals(solution1.GetDocument(currentDocumentId).State, solution2.GetDocument(currentDocumentId).State)); } } @@ -444,7 +466,7 @@ private static SourceText GetNewText(Document document, string csAddition, strin return SourceText.From(document.State.GetTextSynchronously(CancellationToken.None).ToString() + vbAddition); } - private static (Project, Document) GetProjectAndDocument(Solution solution, string projectName, string documentName) + private static (Project project, Document document) GetProjectAndDocument(Solution solution, string projectName, string documentName) { var project = solution.Projects.First(p => string.Equals(p.Name, projectName, StringComparison.OrdinalIgnoreCase)); var document = project.Documents.First(d => string.Equals(d.Name, documentName, StringComparison.OrdinalIgnoreCase)); @@ -452,6 +474,15 @@ private static (Project, Document) GetProjectAndDocument(Solution solution, stri return (project, document); } + private static (Project project, ImmutableArray documents) GetProjectAndDocuments(Solution solution, string projectName, ImmutableArray documentNames) + { + var project = solution.Projects.First(p => string.Equals(p.Name, projectName, StringComparison.OrdinalIgnoreCase)); + var documents = documentNames.SelectAsArray( + documentName => project.Documents.First(d => string.Equals(d.Name, documentName, StringComparison.OrdinalIgnoreCase))); + + return (project, documents); + } + // make sure we always move remote workspace forward private int _solutionVersion = 0; @@ -502,6 +533,30 @@ private static Solution Populate(Solution solution) "cs additional file content2" }, Array.Empty()); + solution = AddProject(solution, LanguageNames.CSharp, new[] + { + "class CS { }", + "class CS2 { }", + "class CS3 { }", + "class CS4 { }", + "class CS5 { }", + }, new[] + { + "cs additional file content" + }, Array.Empty()); + + solution = AddProject(solution, LanguageNames.VisualBasic, new[] + { + "Class VB\r\nEnd Class", + "Class VB2\r\nEnd Class", + "Class VB3\r\nEnd Class", + "Class VB4\r\nEnd Class", + "Class VB5\r\nEnd Class", + }, new[] + { + "vb additional file content" + }, Array.Empty()); + return solution; }