From 2975b86d0af27a5784faa6e82e7c9d8436a1958e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 5 Apr 2024 23:18:16 -0700 Subject: [PATCH 01/40] improve asset hints to speed up searching --- .../Portable/Workspace/Solution/AssetHint.cs | 4 +- .../Workspace/Solution/StateChecksums.cs | 64 ++++--------------- .../Remote/Core/AbstractAssetProvider.cs | 10 +-- .../ServiceHub/Host/ChecksumSynchronizer.cs | 10 +-- .../Host/RemoteWorkspace.SolutionCreator.cs | 34 +++++----- 5 files changed, 39 insertions(+), 83 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 7f035466d93dd..f96c199416c61 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -13,13 +13,15 @@ namespace Microsoft.CodeAnalysis.Serialization; [DataContract] internal readonly struct AssetHint { - public static readonly AssetHint None = default; + public static AssetHint SolutionOnly = default; [DataMember(Order = 0)] public readonly ProjectId? ProjectId; [DataMember(Order = 1)] public readonly DocumentId? DocumentId; + public bool IsSolutionOnly => ProjectId is null; + private AssetHint(ProjectId? projectId, DocumentId? documentId) { ProjectId = projectId; diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index d99b463c915db..7b1cb6f03169a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -256,61 +256,19 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - if (assetHint.ProjectId != null) - { - Contract.ThrowIfTrue( - projectCone != null && !projectCone.Contains(assetHint.ProjectId), - "Requesting an asset outside of the cone explicitly being asked for!"); - - var projectState = solution.GetProjectState(assetHint.ProjectId); - if (projectState != null && - projectState.TryGetStateChecksums(out var projectStateChecksums)) - { - await projectStateChecksums.FindAsync(projectState, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); - } - } - else - { - Contract.ThrowIfTrue(assetHint.DocumentId != null); - - // Before doing a depth-first-search *into* each project, first run across all the project at their top - // level. This ensures that when we are trying to sync the projects referenced by a SolutionStateChecksums' - // instance that we don't unnecessarily walk all documents looking just for those. - - foreach (var (projectId, projectState) in solution.ProjectStates) - { - if (searchingChecksumsLeft.Count == 0) - break; - - // If we're syncing a project cone, no point at all at looking at child projects of the solution that - // are not in that cone. - if (projectCone != null && !projectCone.Contains(projectId)) - continue; - - if (projectState.TryGetStateChecksums(out var projectStateChecksums) && - searchingChecksumsLeft.Remove(projectStateChecksums.Checksum)) - { - result[projectStateChecksums.Checksum] = projectStateChecksums; - } - } + if (assetHint.IsSolutionOnly) + return; - // Now actually do the depth first search into each project. + Contract.ThrowIfNull(assetHint.ProjectId); + Contract.ThrowIfTrue( + projectCone != null && !projectCone.Contains(assetHint.ProjectId), + "Requesting an asset outside of the cone explicitly being asked for!"); - foreach (var (projectId, projectState) in solution.ProjectStates) - { - if (searchingChecksumsLeft.Count == 0) - break; - - // If we're syncing a project cone, no point at all at looking at child projects of the solution that - // are not in that cone. - if (projectCone != null && !projectCone.Contains(projectId)) - continue; - - // It's possible not all all our projects have checksums. Specifically, we may have only been asked to - // compute the checksum tree for a subset of projects that were all that a feature needed. - if (projectState.TryGetStateChecksums(out var projectStateChecksums)) - await projectStateChecksums.FindAsync(projectState, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); - } + var projectState = solution.GetProjectState(assetHint.ProjectId); + if (projectState != null && + projectState.TryGetStateChecksums(out var projectStateChecksums)) + { + await projectStateChecksums.FindAsync(projectState, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } } } diff --git a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs index 7ff66cc7fa912..781546b3d6b65 100644 --- a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs +++ b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs @@ -24,17 +24,17 @@ internal abstract class AbstractAssetProvider public async Task CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { - var solutionCompilationChecksums = await GetAssetAsync(AssetHint.None, solutionChecksum, cancellationToken).ConfigureAwait(false); - var solutionChecksums = await GetAssetAsync(AssetHint.None, solutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + var solutionCompilationChecksums = await GetAssetAsync(AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); + var solutionChecksums = await GetAssetAsync(AssetHint.SolutionOnly, solutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); - var solutionAttributes = await GetAssetAsync(AssetHint.None, solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); - await GetAssetAsync(AssetHint.None, solutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); + var solutionAttributes = await GetAssetAsync(AssetHint.SolutionOnly, solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + await GetAssetAsync(AssetHint.SolutionOnly, solutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); using var _ = ArrayBuilder.GetInstance(solutionChecksums.Projects.Length, out var projects); foreach (var (projectChecksum, projectId) in solutionChecksums.Projects) projects.Add(await CreateProjectInfoAsync(projectId, projectChecksum, cancellationToken).ConfigureAwait(false)); - var analyzerReferences = await CreateCollectionAsync(AssetHint.None, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); + var analyzerReferences = await CreateCollectionAsync(AssetHint.SolutionOnly, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); return SolutionInfo.Create( solutionAttributes.Id, solutionAttributes.Version, solutionAttributes.FilePath, projects.ToImmutableAndClear(), analyzerReferences).WithTelemetryId(solutionAttributes.TelemetryId); diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index b888a6044ce31..e47cc7ed14c9c 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -41,9 +41,9 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, // first, get solution checksum object for the given solution checksum var solutionCompilationChecksumObject = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, solutionChecksum, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); solutionChecksumObject = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, solutionCompilationChecksumObject.SolutionState, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, solutionCompilationChecksumObject.SolutionState, cancellationToken).ConfigureAwait(false); // second, get direct children of the solution { @@ -51,17 +51,17 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, checksums.Add(solutionCompilationChecksumObject.SourceGeneratorExecutionVersionMap); solutionChecksumObject.AddAllTo(checksums); - await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.None, checksums, results: null, cancellationToken).ConfigureAwait(false); + await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); } } // third and last get direct children for all projects and documents in the solution - foreach (var (projectChecksum, _) in solutionChecksumObject.Projects) + foreach (var (projectChecksum, projectId) in solutionChecksumObject.Projects) { // These GetAssetAsync calls should be fast since they were just retrieved above. There's a small // chance the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. var projectStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, projectChecksum, cancellationToken).ConfigureAwait(false); + assetHint: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); await SynchronizeProjectAssetsAsync(projectStateChecksums, cancellationToken).ConfigureAwait(false); } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index dcbe3accfd542..e91dc020428a2 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -39,12 +39,12 @@ private readonly struct SolutionCreator(HostServices hostServices, AssetProvider public async Task IsIncrementalUpdateAsync(Checksum newSolutionChecksum, CancellationToken cancellationToken) { var newSolutionCompilationChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionChecksum, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); var newSolutionChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); var newSolutionInfo = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); // if either solution id or file path changed, then we consider it as new solution return _baseSolution.Id == newSolutionInfo.Id && _baseSolution.FilePath == newSolutionInfo.FilePath; @@ -61,9 +61,9 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca solution = solution.WithoutFrozenSourceGeneratedDocuments(); var newSolutionCompilationChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionChecksum, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); var newSolutionChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); var oldSolutionCompilationChecksums = await solution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); var oldSolutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); @@ -71,7 +71,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca if (oldSolutionChecksums.Attributes != newSolutionChecksums.Attributes) { var newSolutionInfo = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); // if either id or file path has changed, then this is not update Contract.ThrowIfFalse(solution.Id == newSolutionInfo.Id && solution.FilePath == newSolutionInfo.FilePath); @@ -86,7 +86,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca if (oldSolutionChecksums.AnalyzerReferences.Checksum != newSolutionChecksums.AnalyzerReferences.Checksum) { solution = solution.WithAnalyzerReferences(await _assetProvider.CreateCollectionAsync( - assetHint: AssetHint.None, newSolutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); + assetHint: AssetHint.SolutionOnly, newSolutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); } if (newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.HasValue && @@ -99,10 +99,10 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca for (var i = 0; i < count; i++) { var identity = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.Value[i], cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.Value[i], cancellationToken).ConfigureAwait(false); var documentStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Checksums[i], cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Checksums[i], cancellationToken).ConfigureAwait(false); var serializableSourceText = await _assetProvider.GetAssetAsync(assetHint: newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Ids[i], documentStateChecksums.Text, cancellationToken).ConfigureAwait(false); @@ -118,7 +118,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap) { var newVersions = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.None, newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); // The execution version map will be for the entire solution on the host side. However, we may // only be syncing over a partial cone. In that case, filter down the version map we apply to @@ -220,16 +220,12 @@ private async Task UpdateProjectsAsync( // sync over the *info* about all the added/changed projects. We'll want the info so we can determine // what actually changed. - using var _5 = PooledHashSet.GetInstance(out var newChecksumsToSync); - newChecksumsToSync.AddRange(newProjectIdToChecksum.Values); - - var newProjectStateChecksums = await _assetProvider.GetAssetsAsync( - assetHint: AssetHint.None, newChecksumsToSync, cancellationToken).ConfigureAwait(false); - - foreach (var (checksum, newProjectStateChecksum) in newProjectStateChecksums) + foreach (var (projectId, checksum) in newProjectIdToChecksum) { - Contract.ThrowIfTrue(checksum != newProjectStateChecksum.Checksum); - newProjectIdToStateChecksums.Add(newProjectStateChecksum.ProjectId, newProjectStateChecksum); + var newProjectStateChecksums = await _assetProvider.GetAssetAsync( + assetHint: projectId, checksum, cancellationToken).ConfigureAwait(false); + Contract.ThrowIfTrue(newProjectStateChecksums.Checksum != checksum); + newProjectIdToStateChecksums.Add(projectId, newProjectStateChecksums); } // Now that we've collected the old and new project state checksums, we can actually process them to From 9204f2589bb56631a310f20bc2b5b6170977c4d5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 5 Apr 2024 23:36:04 -0700 Subject: [PATCH 02/40] update pinning --- .../Workspace/Solution/StateChecksums.cs | 7 + .../ServiceHub/Host/SolutionAssetCache.cs | 370 +++++++++--------- 2 files changed, 201 insertions(+), 176 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 7b1cb6f03169a..98bda2efb8bc0 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -442,6 +442,13 @@ public void Serialize(ObjectWriter writer) this.Text.WriteTo(writer); } + public void AddAllTo(HashSet checksums) + { + checksums.AddIfNotNullChecksum(this.Checksum); + checksums.AddIfNotNullChecksum(this.Info); + checksums.AddIfNotNullChecksum(this.Text); + } + public static DocumentStateChecksums Deserialize(ObjectReader reader) { return new DocumentStateChecksums( diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs index c167b7162380b..24c6740ae014d 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs @@ -11,231 +11,249 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Serialization; using Roslyn.Utilities; -namespace Microsoft.CodeAnalysis.Remote +namespace Microsoft.CodeAnalysis.Remote; + +internal sealed class SolutionAssetCache { - internal sealed class SolutionAssetCache + static SolutionAssetCache() { - static SolutionAssetCache() - { - // CRITICAL: The size SharedStopwatch is the size of a TimeSpan (which itself is the size of a long). This - // allows stopwatches to be atomically overwritten, without a concern for torn writes, as long as we're - // running on 64bit machines. Make sure this value doesn't change as that will cause these current - // consumers to be invalid. - RoslynDebug.Assert(Marshal.SizeOf(typeof(SharedStopwatch)) == 8); - } + // CRITICAL: The size SharedStopwatch is the size of a TimeSpan (which itself is the size of a long). This + // allows stopwatches to be atomically overwritten, without a concern for torn writes, as long as we're + // running on 64bit machines. Make sure this value doesn't change as that will cause these current + // consumers to be invalid. + RoslynDebug.Assert(Marshal.SizeOf(typeof(SharedStopwatch)) == 8); + } - /// - /// Workspace we are associated with. When we purge items from teh cache, we will avoid any items associated - /// with the items in its 'CurrentSolution'. - /// - private readonly RemoteWorkspace? _remoteWorkspace; - - /// - /// Time interval we check storage for cleanup - /// - private readonly TimeSpan _cleanupIntervalTimeSpan; - - /// - /// Time span data can sit inside of cache () without being used. - /// after that, it will be removed from the cache. - /// - private readonly TimeSpan _purgeAfterTimeSpan; - - /// - /// Time we will wait after the last activity before doing explicit GC cleanup. - /// We monitor all resource access and service call to track last activity time. - /// - /// We do this since 64bit process can hold onto quite big unused memory when - /// OOP is running as AnyCpu - /// - private readonly TimeSpan _gcAfterTimeSpan; - - private readonly ConcurrentDictionary _assets = new(concurrencyLevel: 4, capacity: 10); - - private DateTime _lastGCRun; - private DateTime _lastActivityTime; - - // constructor for testing - public SolutionAssetCache() - { - } + /// + /// Workspace we are associated with. When we purge items from teh cache, we will avoid any items associated + /// with the items in its 'CurrentSolution'. + /// + private readonly RemoteWorkspace? _remoteWorkspace; + + /// + /// Time interval we check storage for cleanup + /// + private readonly TimeSpan _cleanupIntervalTimeSpan; + + /// + /// Time span data can sit inside of cache () without being used. + /// after that, it will be removed from the cache. + /// + private readonly TimeSpan _purgeAfterTimeSpan; + + /// + /// Time we will wait after the last activity before doing explicit GC cleanup. + /// We monitor all resource access and service call to track last activity time. + /// + /// We do this since 64bit process can hold onto quite big unused memory when + /// OOP is running as AnyCpu + /// + private readonly TimeSpan _gcAfterTimeSpan; + + private readonly ConcurrentDictionary _assets = new(concurrencyLevel: 4, capacity: 10); + + private DateTime _lastGCRun; + private DateTime _lastActivityTime; + + // constructor for testing + public SolutionAssetCache() + { + } - /// - /// Create central data cache - /// - /// time interval to clean up - /// time unused data can sit in the cache - /// time we wait before it call GC since last activity - public SolutionAssetCache(RemoteWorkspace? remoteWorkspace, TimeSpan cleanupInterval, TimeSpan purgeAfter, TimeSpan gcAfter) - { - _remoteWorkspace = remoteWorkspace; - _cleanupIntervalTimeSpan = cleanupInterval; - _purgeAfterTimeSpan = purgeAfter; - _gcAfterTimeSpan = gcAfter; + /// + /// Create central data cache + /// + /// time interval to clean up + /// time unused data can sit in the cache + /// time we wait before it call GC since last activity + public SolutionAssetCache(RemoteWorkspace? remoteWorkspace, TimeSpan cleanupInterval, TimeSpan purgeAfter, TimeSpan gcAfter) + { + _remoteWorkspace = remoteWorkspace; + _cleanupIntervalTimeSpan = cleanupInterval; + _purgeAfterTimeSpan = purgeAfter; + _gcAfterTimeSpan = gcAfter; - _lastActivityTime = DateTime.UtcNow; - _lastGCRun = DateTime.UtcNow; + _lastActivityTime = DateTime.UtcNow; + _lastGCRun = DateTime.UtcNow; - Task.Run(CleanAssetsAsync, CancellationToken.None); - } + Task.Run(CleanAssetsAsync, CancellationToken.None); + } - public object GetOrAdd(Checksum checksum, object value) - { - UpdateLastActivityTime(); + public object GetOrAdd(Checksum checksum, object value) + { + UpdateLastActivityTime(); - var entry = _assets.GetOrAdd(checksum, new Entry(value)); - Update(entry); - return entry.Object; - } + var entry = _assets.GetOrAdd(checksum, new Entry(value)); + Update(entry); + return entry.Object; + } - public bool TryGetAsset(Checksum checksum, [MaybeNullWhen(false)] out T value) - { - UpdateLastActivityTime(); + public bool TryGetAsset(Checksum checksum, [MaybeNullWhen(false)] out T value) + { + UpdateLastActivityTime(); - using (Logger.LogBlock(FunctionId.AssetStorage_TryGetAsset, Checksum.GetChecksumLogInfo, checksum, CancellationToken.None)) + using (Logger.LogBlock(FunctionId.AssetStorage_TryGetAsset, Checksum.GetChecksumLogInfo, checksum, CancellationToken.None)) + { + if (!_assets.TryGetValue(checksum, out var entry)) { - if (!_assets.TryGetValue(checksum, out var entry)) - { - value = default; - return false; - } + value = default; + return false; + } - // Update timestamp - Update(entry); + // Update timestamp + Update(entry); - value = (T)entry.Object; - return true; - } + value = (T)entry.Object; + return true; } + } - public bool ContainsAsset(Checksum checksum) - => _assets.ContainsKey(checksum); + public bool ContainsAsset(Checksum checksum) + => _assets.ContainsKey(checksum); - public void UpdateLastActivityTime() - => _lastActivityTime = DateTime.UtcNow; + public void UpdateLastActivityTime() + => _lastActivityTime = DateTime.UtcNow; - private static void Update(Entry entry) - { - // Stopwatch wraps a TimeSpan (which is only 64bits) (asserted in our shared constructor). so this - // assignment can be done safely without a concern for torn writes on 64 systems. - // - // Note: on 32 bit systems there could be an issue here both with a torn write/read or torn write/write. We - // think that's probably ok as a torn read only leads to suboptimal behavior (dropping something early, or - // keeping something around till the next purge), and a torn write should likely still lead to reasonable - // data being written as both writers will likely still write something reasonable once both writes go - // through. e.g. if you have a writer writing 1234-5678 and one writing 1235-0000, then getting 1235-5678 - // or 1234-0000 is still fine as a final outcome. - entry.Stopwatch = SharedStopwatch.StartNew(); - } + private static void Update(Entry entry) + { + // Stopwatch wraps a TimeSpan (which is only 64bits) (asserted in our shared constructor). so this + // assignment can be done safely without a concern for torn writes on 64 systems. + // + // Note: on 32 bit systems there could be an issue here both with a torn write/read or torn write/write. We + // think that's probably ok as a torn read only leads to suboptimal behavior (dropping something early, or + // keeping something around till the next purge), and a torn write should likely still lead to reasonable + // data being written as both writers will likely still write something reasonable once both writes go + // through. e.g. if you have a writer writing 1234-5678 and one writing 1235-0000, then getting 1235-5678 + // or 1234-0000 is still fine as a final outcome. + entry.Stopwatch = SharedStopwatch.StartNew(); + } - private async Task CleanAssetsAsync() + private async Task CleanAssetsAsync() + { + // Todo: associate this with a real CancellationToken that can shutdown this work. + var cancellationToken = CancellationToken.None; + while (!cancellationToken.IsCancellationRequested) { - // Todo: associate this with a real CancellationToken that can shutdown this work. - var cancellationToken = CancellationToken.None; - while (!cancellationToken.IsCancellationRequested) - { - await CleanAssetsWorkerAsync(cancellationToken).ConfigureAwait(false); + await CleanAssetsWorkerAsync(cancellationToken).ConfigureAwait(false); - ForceGC(); + ForceGC(); - await Task.Delay(_cleanupIntervalTimeSpan, cancellationToken).ConfigureAwait(false); - } + await Task.Delay(_cleanupIntervalTimeSpan, cancellationToken).ConfigureAwait(false); } + } - private void ForceGC() + private void ForceGC() + { + // if there was no activity since last GC run. we don't have anything to do + if (_lastGCRun >= _lastActivityTime) { - // if there was no activity since last GC run. we don't have anything to do - if (_lastGCRun >= _lastActivityTime) - { - return; - } + return; + } - var current = DateTime.UtcNow; - if (current - _lastActivityTime < _gcAfterTimeSpan) - { - // we are having activities. - return; - } + var current = DateTime.UtcNow; + if (current - _lastActivityTime < _gcAfterTimeSpan) + { + // we are having activities. + return; + } - using (Logger.LogBlock(FunctionId.AssetStorage_ForceGC, CancellationToken.None)) + using (Logger.LogBlock(FunctionId.AssetStorage_ForceGC, CancellationToken.None)) + { + // we didn't have activity for 5 min. spend some time to drop + // unused memory + for (var i = 0; i < 3; i++) { - // we didn't have activity for 5 min. spend some time to drop - // unused memory - for (var i = 0; i < 3; i++) - { - GC.Collect(); - } + GC.Collect(); } + } + + // update gc run time + _lastGCRun = current; + } - // update gc run time - _lastGCRun = current; + private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationToken) + { + if (_assets.IsEmpty) + { + // no asset, nothing to do. + return; } - private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationToken) + using (Logger.LogBlock(FunctionId.AssetStorage_CleanAssets, cancellationToken)) { - if (_assets.IsEmpty) - { - // no asset, nothing to do. - return; - } + // Ensure that if our remote workspace has a current solution, that we don't purge any items associated + // with that solution. + using var _1 = PooledHashSet.GetInstance(out var pinnedChecksums); + var computePinnedChecksums = false; - using (Logger.LogBlock(FunctionId.AssetStorage_CleanAssets, cancellationToken)) + foreach (var (checksum, entry) in _assets) { - // Ensure that if our remote workspace has a current solution, that we don't purge any items associated - // with that solution. - PooledHashSet? pinnedChecksums = null; - try - { - foreach (var (checksum, entry) in _assets) - { - // If not enough time has passed, keep in the cache. - if (entry.Stopwatch.Elapsed <= _purgeAfterTimeSpan) - continue; - - // If this is a checksum we want to pin, do not remove it. - if (pinnedChecksums == null) - { - pinnedChecksums = PooledHashSet.GetInstance(); - await AddPinnedChecksumsAsync(pinnedChecksums, cancellationToken).ConfigureAwait(false); - } - - if (pinnedChecksums.Contains(checksum)) - continue; - - _assets.TryRemove(checksum, out _); - } - } - finally + // If not enough time has passed, keep in the cache. + if (entry.Stopwatch.Elapsed <= _purgeAfterTimeSpan) + continue; + + // If this is a checksum we want to pin, do not remove it. + if (!computePinnedChecksums) { - pinnedChecksums?.Free(); + await AddPinnedChecksumsAsync(pinnedChecksums, cancellationToken).ConfigureAwait(false); + computePinnedChecksums = true; } + + if (pinnedChecksums.Contains(checksum)) + continue; + + _assets.TryRemove(checksum, out _); } } + } - private async ValueTask AddPinnedChecksumsAsync(HashSet pinnedChecksums, CancellationToken cancellationToken) - { - if (_remoteWorkspace is null) - return; + private async ValueTask AddPinnedChecksumsAsync(HashSet pinnedChecksums, CancellationToken cancellationToken) + { + if (_remoteWorkspace is null) + return; - var checksums = await _remoteWorkspace.CurrentSolution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - checksums.AddAllTo(pinnedChecksums); - } + var currentSolution = _remoteWorkspace.CurrentSolution; + var compilationStateChecksums = await currentSolution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); + compilationStateChecksums.AddAllTo(pinnedChecksums); - private sealed class Entry + Contract.ThrowIfFalse(currentSolution.CompilationState.SolutionState.TryGetStateChecksums(out var stateChecksums)); + stateChecksums.AddAllTo(pinnedChecksums); + + foreach (var (_, projectState) in currentSolution.CompilationState.SolutionState.ProjectStates) { - public SharedStopwatch Stopwatch = SharedStopwatch.StartNew(); + Contract.ThrowIfFalse(projectState.TryGetStateChecksums(out var projectStateChecksums)); + projectStateChecksums.AddAllTo(pinnedChecksums); + + AddAll(projectState.DocumentStates); + AddAll(projectState.AdditionalDocumentStates); + AddAll(projectState.AnalyzerConfigDocumentStates); + } - // This can't change for same checksum - public readonly object Object; + return; - public Entry(object @object) + void AddAll(TextDocumentStates states) where TState : TextDocumentState + { + foreach (var (_, documentState) in states.States) { - Object = @object; + Contract.ThrowIfFalse(documentState.TryGetStateChecksums(out var documentChecksums)); + documentChecksums.AddAllTo(pinnedChecksums); } } } + + private sealed class Entry + { + public SharedStopwatch Stopwatch = SharedStopwatch.StartNew(); + + // This can't change for same checksum + public readonly object Object; + + public Entry(object @object) + { + Object = @object; + } + } } From 51681b69cc6fb2db7874255a353a555868b9ea20 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 5 Apr 2024 23:50:02 -0700 Subject: [PATCH 03/40] in progress --- .../Remote/Core/SolutionAssetStorage.Scope.cs | 4 +- .../Remote/Core/SolutionAssetStorage.cs | 5 +- .../ServiceHub/Host/RemoteSolutionCache.cs | 7 +++ .../ServiceHub/Host/RemoteWorkspaceManager.cs | 10 ++-- .../Host/RemoteWorkspace_SolutionCaching.cs | 16 +++++++ .../ServiceHub/Host/SolutionAssetCache.cs | 47 ++++++++++++------- .../Remote/ServiceHub/Host/TestUtils.cs | 4 +- 7 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index ce07e27e23cb1..8772dc30edba7 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -93,14 +93,14 @@ public readonly struct TestAccessor(Scope scope) /// Retrieve asset of a specified available within from /// the storage. /// - public async ValueTask GetAssetAsync(Checksum checksum, CancellationToken cancellationToken) + public async ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksum == Checksum.Null); using var checksumPool = Creator.CreateChecksumSet(checksum); using var _ = Creator.CreateResultMap(out var resultPool); - await scope.FindAssetsAsync(AssetHint.None, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); + await scope.FindAssetsAsync(assetHint, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(resultPool.Count != 1); var (resultingChecksum, value) = resultPool.First(); diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index 841b07901f7a8..fdd0eaa83b00e 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Serialization; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Remote; @@ -126,9 +127,9 @@ internal TestAccessor(SolutionAssetStorage solutionAssetStorage) _solutionAssetStorage = solutionAssetStorage; } - public async ValueTask GetRequiredAssetAsync(Checksum checksum, CancellationToken cancellationToken) + public async ValueTask GetRequiredAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) { - return await _solutionAssetStorage._checksumToScope.Single().Value.GetTestAccessor().GetAssetAsync(checksum, cancellationToken).ConfigureAwait(false); + return await _solutionAssetStorage._checksumToScope.Single().Value.GetTestAccessor().GetAssetAsync(assetHint, checksum, cancellationToken).ConfigureAwait(false); } public bool IsPinned(Checksum checksum) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteSolutionCache.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteSolutionCache.cs index 75660559eabdc..d5b3c3be1ef77 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteSolutionCache.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteSolutionCache.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Remote; @@ -158,6 +159,12 @@ public void ReportTelemetry() })); } + public void AddAllTo(HashSet solutions) + { + foreach (var node in _cacheNodes) + solutions.AddIfNotNull(node.Solution); + } + private sealed class CacheNode(TChecksum checksum) { public readonly TChecksum Checksum = checksum; diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspaceManager.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspaceManager.cs index 61916c58da957..b6a17c45389b9 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspaceManager.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspaceManager.cs @@ -37,11 +37,11 @@ internal class RemoteWorkspaceManager /// allowing it to get too full. /// /// Also note that the asset cache will not remove items associated with the of the workspace it is created against. This ensures that the assets - /// associated with the solution that most closely corresponds to what the user is working with will stay pinned - /// on the remote side and not get purged just because the user stopped interactive for a while. This ensures - /// the next sync (which likely overlaps heavily with the current solution) will not force the same assets to be - /// resent. + /// cref="Workspace.CurrentSolution"/> of the workspace it is created against (as well as any recent in-flight + /// solutions). This ensures that the assets associated with the solution that most closely corresponds to what + /// the user is working with will stay pinned on the remote side and not get purged just because the user + /// stopped interactive for a while. This ensures the next sync (which likely overlaps heavily with the current + /// solution) will not force the same assets to be resent. /// /// /// CleanupInterval=30s gives what feels to be a reasonable non-aggressive amount of time to let the cache diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs index a5890aba70d40..32d56a5b21036 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs @@ -4,6 +4,9 @@ using System; using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Remote @@ -111,5 +114,18 @@ private void CheckCacheInvariants_NoLock() Contract.ThrowIfTrue(solutionChecksum != solution.SolutionChecksum); } } + + /// + /// Gets all the solution instances this remote workspace knows about because of the primary solution or any + /// in-flight operations. + /// + public async ValueTask AddPinnedSolutionsAsync(HashSet solutions, CancellationToken cancellationToken) + { + using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) + { + solutions.AddIfNotNull(_lastRequestedPrimaryBranchSolution.solution); + _lastRequestedAnyBranchSolutions.AddAllTo(solutions); + }; + } } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs index 24c6740ae014d..870848f414c9b 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs @@ -186,7 +186,7 @@ private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationTok // Ensure that if our remote workspace has a current solution, that we don't purge any items associated // with that solution. using var _1 = PooledHashSet.GetInstance(out var pinnedChecksums); - var computePinnedChecksums = false; + var computePinnedInformation = false; foreach (var (checksum, entry) in _assets) { @@ -195,10 +195,10 @@ private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationTok continue; // If this is a checksum we want to pin, do not remove it. - if (!computePinnedChecksums) + if (!computePinnedInformation) { await AddPinnedChecksumsAsync(pinnedChecksums, cancellationToken).ConfigureAwait(false); - computePinnedChecksums = true; + computePinnedInformation = true; } if (pinnedChecksums.Contains(checksum)) @@ -214,24 +214,39 @@ private async ValueTask AddPinnedChecksumsAsync(HashSet pinnedChecksum if (_remoteWorkspace is null) return; - var currentSolution = _remoteWorkspace.CurrentSolution; - var compilationStateChecksums = await currentSolution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - compilationStateChecksums.AddAllTo(pinnedChecksums); + using var _1 = PooledHashSet.GetInstance(out var pinnedSolutions); - Contract.ThrowIfFalse(currentSolution.CompilationState.SolutionState.TryGetStateChecksums(out var stateChecksums)); - stateChecksums.AddAllTo(pinnedChecksums); + // Collect all the solutions the remote workspace has pinned. + await _remoteWorkspace.AddPinnedSolutionsAsync(pinnedSolutions, cancellationToken).ConfigureAwait(false); - foreach (var (_, projectState) in currentSolution.CompilationState.SolutionState.ProjectStates) + // Then add all relevant info from those pinned solutions so the set that we will not let go of. + foreach (var pinnedSolution in pinnedSolutions) + await AddPinnedChecksumsAsync(pinnedSolution).ConfigureAwait(false); + + return; + + async ValueTask AddPinnedChecksumsAsync(Solution pinnedSolution) { - Contract.ThrowIfFalse(projectState.TryGetStateChecksums(out var projectStateChecksums)); - projectStateChecksums.AddAllTo(pinnedChecksums); + // Get the checksums for the local solution. Note that this will ensure that all child checksums are + // computed. As such, we can just use TryGetXXX below to get them. + var compilationState = pinnedSolution.CompilationState; + var compilationStateChecksums = await compilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); + compilationStateChecksums.AddAllTo(pinnedChecksums); - AddAll(projectState.DocumentStates); - AddAll(projectState.AdditionalDocumentStates); - AddAll(projectState.AnalyzerConfigDocumentStates); - } + var solutionState = compilationState.SolutionState; + Contract.ThrowIfFalse(solutionState.TryGetStateChecksums(out var stateChecksums)); + stateChecksums.AddAllTo(pinnedChecksums); - return; + foreach (var (_, projectState) in solutionState.ProjectStates) + { + Contract.ThrowIfFalse(projectState.TryGetStateChecksums(out var projectStateChecksums)); + projectStateChecksums.AddAllTo(pinnedChecksums); + + AddAll(projectState.DocumentStates); + AddAll(projectState.AdditionalDocumentStates); + AddAll(projectState.AnalyzerConfigDocumentStates); + } + } void AddAll(TextDocumentStates states) where TState : TextDocumentState { diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index e942c5e44f5fd..08e6b49748a5b 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -161,8 +161,8 @@ private static void AddAllTo(DocumentStateChecksums documentStateChecksums, Hash } /// - /// create checksum to correspoing object map from solution - /// this map should contain every parts of solution that can be used to re-create the solution back + /// create checksum to corresponding object map from solution this map should contain every parts of solution + /// that can be used to re-create the solution back /// public static async Task> GetAssetMapAsync(this Solution solution, CancellationToken cancellationToken) { From f685cb1c70a2e491f9f43cc96551073823c0839c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:00:19 -0700 Subject: [PATCH 04/40] in progress --- .../Workspace/Solution/StateChecksums.cs | 76 +++++++++++++++---- .../Remote/Core/AbstractAssetProvider.cs | 2 +- .../Remote/Core/ISolutionAssetProvider.cs | 2 +- .../Remote/Core/SolutionAssetProvider.cs | 6 +- .../Remote/Core/SolutionAssetStorage.Scope.cs | 4 +- .../Remote/ServiceHub/Host/AssetProvider.cs | 6 +- .../Remote/ServiceHub/Host/IAssetSource.cs | 2 +- .../Host/RemoteWorkspace.SolutionCreator.cs | 3 - .../ServiceHub/Host/SolutionAssetSource.cs | 2 +- .../Remote/ServiceHub/Host/TestUtils.cs | 14 ++-- 10 files changed, 81 insertions(+), 36 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 98bda2efb8bc0..0ccd75a9114f1 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -111,7 +111,7 @@ public static SolutionCompilationStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionCompilationState compilationState, ProjectCone? projectCone, - AssetHint assetHint, + AssetHint? assetHint, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -135,7 +135,7 @@ public async Task FindAsync( Contract.ThrowIfFalse(FrozenSourceGeneratedDocumentIdentities.HasValue); // This could either be the checksum for the text (which we'll use our regular helper for first)... - await ChecksumCollection.FindAsync(compilationState.FrozenSourceGeneratedDocumentStates, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await ChecksumCollection.FindAsync(compilationState.FrozenSourceGeneratedDocumentStates, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); // ... or one of the identities. In this case, we'll use the fact that there's a 1:1 correspondence between the // two collections we hold onto. @@ -235,7 +235,7 @@ public static SolutionStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionState solution, ProjectCone? projectCone, - AssetHint assetHint, + AssetHint? assetHintOpt, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -256,19 +256,67 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - if (assetHint.IsSolutionOnly) - return; + if (assetHintOpt != null) + { + var assetHint = assetHintOpt.Value; + // Caller said they were only looking for solution level assets. no need to go any further. + if (assetHint.IsSolutionOnly) + return; + + Contract.ThrowIfNull(assetHint.ProjectId); + Contract.ThrowIfTrue( + projectCone != null && !projectCone.Contains(assetHint.ProjectId), + "Requesting an asset outside of the cone explicitly being asked for!"); + + var projectState = solution.GetProjectState(assetHint.ProjectId); + if (projectState != null && + projectState.TryGetStateChecksums(out var projectStateChecksums)) + { + await projectStateChecksums.FindAsync(projectState, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + } + } + else + { + // Full search, used for test purposes. - Contract.ThrowIfNull(assetHint.ProjectId); - Contract.ThrowIfTrue( - projectCone != null && !projectCone.Contains(assetHint.ProjectId), - "Requesting an asset outside of the cone explicitly being asked for!"); + // Before doing a depth-first-search *into* each project, first run across all the project at their top + // level. This ensures that when we are trying to sync the projects referenced by a SolutionStateChecksums' + // instance that we don't unnecessarily walk all documents looking just for those. - var projectState = solution.GetProjectState(assetHint.ProjectId); - if (projectState != null && - projectState.TryGetStateChecksums(out var projectStateChecksums)) - { - await projectStateChecksums.FindAsync(projectState, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + foreach (var (projectId, projectState) in solution.ProjectStates) + { + if (searchingChecksumsLeft.Count == 0) + break; + + // If we're syncing a project cone, no point at all at looking at child projects of the solution that + // are not in that cone. + if (projectCone != null && !projectCone.Contains(projectId)) + continue; + + if (projectState.TryGetStateChecksums(out var projectStateChecksums) && + searchingChecksumsLeft.Remove(projectStateChecksums.Checksum)) + { + result[projectStateChecksums.Checksum] = projectStateChecksums; + } + } + + // Now actually do the depth first search into each project. + + foreach (var (projectId, projectState) in solution.ProjectStates) + { + if (searchingChecksumsLeft.Count == 0) + break; + + // If we're syncing a project cone, no point at all at looking at child projects of the solution that + // are not in that cone. + if (projectCone != null && !projectCone.Contains(projectId)) + continue; + + // It's possible not all all our projects have checksums. Specifically, we may have only been asked to + // compute the checksum tree for a subset of projects that were all that a feature needed. + if (projectState.TryGetStateChecksums(out var projectStateChecksums)) + await projectStateChecksums.FindAsync(projectState, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + } } } } diff --git a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs index 781546b3d6b65..bafa77b81ccf8 100644 --- a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs +++ b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs @@ -20,7 +20,7 @@ internal abstract class AbstractAssetProvider /// /// return data of type T whose checksum is the given checksum /// - public abstract ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken); + public abstract ValueTask GetAssetAsync(AssetHint? assetHint, Checksum checksum, CancellationToken cancellationToken); public async Task CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { diff --git a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs index b57bf95fcb2b2..1a0995998a556 100644 --- a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs @@ -26,5 +26,5 @@ internal interface ISolutionAssetProvider /// save substantially on performance by avoiding having to search the full solution tree to find matching items for /// a particular checksum. ValueTask WriteAssetsAsync( - PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken); + PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs index 52a3f75670cc7..66a9fbe476e8a 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs @@ -28,7 +28,7 @@ internal sealed class SolutionAssetProvider(SolutionServices services) : ISoluti public ValueTask WriteAssetsAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint assetHint, + AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { @@ -41,7 +41,7 @@ public ValueTask WriteAssetsAsync( using var _ = FlowControlHelper.TrySuppressFlow(); return WriteAssetsSuppressedFlowAsync(pipeWriter, solutionChecksum, assetHint, checksums, cancellationToken); - async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { // The responsibility is on us (as per the requirements of RemoteCallback.InvokeAsync) to Complete the // pipewriter. This will signal to streamjsonrpc that the writer passed into it is complete, which will @@ -65,7 +65,7 @@ async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum s private async ValueTask WriteAssetsWorkerAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint assetHint, + AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index 8772dc30edba7..38159334e9ea2 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -45,7 +45,7 @@ public void Dispose() /// the storage. /// public async Task AddAssetsAsync( - AssetHint assetHint, + AssetHint? assetHint, ReadOnlyMemory checksums, Dictionary assetMap, CancellationToken cancellationToken) @@ -65,7 +65,7 @@ public async Task AddAssetsAsync( } private async Task FindAssetsAsync( - AssetHint assetHint, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) + AssetHint? assetHint, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) { var solutionState = this.CompilationState; diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index fd72dfbe20d76..35d639e349402 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -31,7 +31,7 @@ internal sealed partial class AssetProvider(Checksum solutionChecksum, SolutionA private readonly IAssetSource _assetSource = assetSource; public override async ValueTask GetAssetAsync( - AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) + AssetHint? assetHint, Checksum checksum, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksum == Checksum.Null); if (_assetCache.TryGetAsset(checksum, out var asset)) @@ -113,7 +113,7 @@ public async ValueTask SynchronizeProjectAssetsAsync(ProjectStateChecksums proje } public async ValueTask SynchronizeAssetsAsync( - AssetHint assetHint, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) + AssetHint? assetHint, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksums.Contains(Checksum.Null)); if (checksums.Count == 0) @@ -193,7 +193,7 @@ void AddResult(Checksum checksum, object result) } private async ValueTask> RequestAssetsAsync( - AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { #if NETCOREAPP Contract.ThrowIfTrue(checksums.Span.Contains(Checksum.Null)); diff --git a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs index 840e043daa46a..9807556f35022 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs @@ -16,5 +16,5 @@ namespace Microsoft.CodeAnalysis.Remote; internal interface IAssetSource { ValueTask> GetAssetsAsync( - Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); + Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index e91dc020428a2..a3eef0a8a5c9c 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -3,14 +3,11 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Frozen; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs index 4917033beae78..9d9cd943d0869 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs @@ -18,7 +18,7 @@ internal sealed class SolutionAssetSource(ServiceBrokerClient client) : IAssetSo public async ValueTask> GetAssetsAsync( Checksum solutionChecksum, - AssetHint assetHint, + AssetHint? assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken) diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index 08e6b49748a5b..272587bcce992 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -104,7 +104,7 @@ async Task>> GetAssetFromAssetServiceAsync(I foreach (var checksum in checksums) { items.Add(new KeyValuePair(checksum, await assetService.GetAssetAsync( - assetHint: AssetHint.None, checksum, CancellationToken.None).ConfigureAwait(false))); + assetHint: null, checksum, CancellationToken.None).ConfigureAwait(false))); } return items; @@ -115,9 +115,9 @@ async Task> GetAllChildrenChecksumsAsync(Checksum solutionChec var set = new HashSet(); var solutionCompilationChecksums = await assetService.GetAssetAsync( - assetHint: AssetHint.None, solutionChecksum, CancellationToken.None).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, solutionChecksum, CancellationToken.None).ConfigureAwait(false); var solutionChecksums = await assetService.GetAssetAsync( - assetHint: AssetHint.None, solutionCompilationChecksums.SolutionState, CancellationToken.None).ConfigureAwait(false); + assetHint: AssetHint.SolutionOnly, solutionCompilationChecksums.SolutionState, CancellationToken.None).ConfigureAwait(false); solutionCompilationChecksums.AddAllTo(set); solutionChecksums.AddAllTo(set); @@ -197,17 +197,17 @@ public static async Task AppendAssetMapAsync( if (projectId == null) { var compilationChecksums = await solution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: AssetHint.None, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: null, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var frozenSourceGeneratedDocumentState in solution.CompilationState.FrozenSourceGeneratedDocumentStates?.States.Values ?? []) { var documentChecksums = await frozenSourceGeneratedDocumentState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: AssetHint.None, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: null, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); } var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(solutionChecksums.ProjectCone != null); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, assetHint: AssetHint.None, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, assetHintOpt: null, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var project in solution.Projects) await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); @@ -219,7 +219,7 @@ public static async Task AppendAssetMapAsync( var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectCone.Equals(solutionChecksums.ProjectCone)); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetHint: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetHintOpt: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); var project = solution.GetRequiredProject(projectId); await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); From c04ebe73d55de0079ba3b837e396c433396b808a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:02:11 -0700 Subject: [PATCH 05/40] In progress --- src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs | 4 ++-- src/Workspaces/Remote/Core/SolutionAssetStorage.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index 38159334e9ea2..0d19173d2fe50 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -93,14 +93,14 @@ public readonly struct TestAccessor(Scope scope) /// Retrieve asset of a specified available within from /// the storage. /// - public async ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) + public async ValueTask GetAssetAsync(Checksum checksum, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksum == Checksum.Null); using var checksumPool = Creator.CreateChecksumSet(checksum); using var _ = Creator.CreateResultMap(out var resultPool); - await scope.FindAssetsAsync(assetHint, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); + await scope.FindAssetsAsync(assetHint: null, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(resultPool.Count != 1); var (resultingChecksum, value) = resultPool.First(); diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index fdd0eaa83b00e..6ca9561dd8006 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -127,9 +127,9 @@ internal TestAccessor(SolutionAssetStorage solutionAssetStorage) _solutionAssetStorage = solutionAssetStorage; } - public async ValueTask GetRequiredAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) + public async ValueTask GetRequiredAssetAsync(Checksum checksum, CancellationToken cancellationToken) { - return await _solutionAssetStorage._checksumToScope.Single().Value.GetTestAccessor().GetAssetAsync(assetHint, checksum, cancellationToken).ConfigureAwait(false); + return await _solutionAssetStorage._checksumToScope.Single().Value.GetTestAccessor().GetAssetAsync(checksum, cancellationToken).ConfigureAwait(false); } public bool IsPinned(Checksum checksum) From 3290c29596ca394dbf33d5eafb81f6292e291acd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:07:00 -0700 Subject: [PATCH 06/40] special test value --- .../Portable/Workspace/Solution/AssetHint.cs | 19 +++++++++--- .../Workspace/Solution/StateChecksums.cs | 30 ++----------------- .../Remote/Core/AbstractAssetProvider.cs | 2 +- .../Remote/Core/ISolutionAssetProvider.cs | 2 +- .../Remote/Core/SolutionAssetProvider.cs | 6 ++-- .../Remote/Core/SolutionAssetStorage.Scope.cs | 4 +-- .../Remote/ServiceHub/Host/AssetProvider.cs | 6 ++-- .../Remote/ServiceHub/Host/IAssetSource.cs | 2 +- .../ServiceHub/Host/SolutionAssetSource.cs | 2 +- 9 files changed, 30 insertions(+), 43 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index f96c199416c61..26eb47b7baae9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -13,21 +13,32 @@ namespace Microsoft.CodeAnalysis.Serialization; [DataContract] internal readonly struct AssetHint { + /// + /// Instance that will only look up solution-level data when searching for checksums. + /// public static AssetHint SolutionOnly = default; + /// + /// Special instance, allowed in tests, that can do a full lookup across the entire checksum tree. + /// + public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, true); + [DataMember(Order = 0)] public readonly ProjectId? ProjectId; [DataMember(Order = 1)] public readonly DocumentId? DocumentId; + [DataMember(Order = 2)] + public readonly bool IsFullLookup_ForTestingPurposesOnly; - public bool IsSolutionOnly => ProjectId is null; + public bool IsSolutionOnly => !IsFullLookup_ForTestingPurposesOnly && ProjectId is null; - private AssetHint(ProjectId? projectId, DocumentId? documentId) + private AssetHint(ProjectId? projectId, DocumentId? documentId, bool isFullLookup_ForTestingPurposesOnly) { ProjectId = projectId; DocumentId = documentId; + IsFullLookup_ForTestingPurposesOnly = isFullLookup_ForTestingPurposesOnly; } - public static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null); - public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId); + public static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null, false); + public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId, false); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 0ccd75a9114f1..c0e66f2120481 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -111,7 +111,7 @@ public static SolutionCompilationStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionCompilationState compilationState, ProjectCone? projectCone, - AssetHint? assetHint, + AssetHint assetHint, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -235,7 +235,7 @@ public static SolutionStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionState solution, ProjectCone? projectCone, - AssetHint? assetHintOpt, + AssetHint assetHint, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -256,9 +256,8 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - if (assetHintOpt != null) + if (!assetHint.IsFullLookup_ForTestingPurposesOnly) { - var assetHint = assetHintOpt.Value; // Caller said they were only looking for solution level assets. no need to go any further. if (assetHint.IsSolutionOnly) return; @@ -279,29 +278,6 @@ public async Task FindAsync( { // Full search, used for test purposes. - // Before doing a depth-first-search *into* each project, first run across all the project at their top - // level. This ensures that when we are trying to sync the projects referenced by a SolutionStateChecksums' - // instance that we don't unnecessarily walk all documents looking just for those. - - foreach (var (projectId, projectState) in solution.ProjectStates) - { - if (searchingChecksumsLeft.Count == 0) - break; - - // If we're syncing a project cone, no point at all at looking at child projects of the solution that - // are not in that cone. - if (projectCone != null && !projectCone.Contains(projectId)) - continue; - - if (projectState.TryGetStateChecksums(out var projectStateChecksums) && - searchingChecksumsLeft.Remove(projectStateChecksums.Checksum)) - { - result[projectStateChecksums.Checksum] = projectStateChecksums; - } - } - - // Now actually do the depth first search into each project. - foreach (var (projectId, projectState) in solution.ProjectStates) { if (searchingChecksumsLeft.Count == 0) diff --git a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs index bafa77b81ccf8..781546b3d6b65 100644 --- a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs +++ b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs @@ -20,7 +20,7 @@ internal abstract class AbstractAssetProvider /// /// return data of type T whose checksum is the given checksum /// - public abstract ValueTask GetAssetAsync(AssetHint? assetHint, Checksum checksum, CancellationToken cancellationToken); + public abstract ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken); public async Task CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { diff --git a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs index 1a0995998a556..b57bf95fcb2b2 100644 --- a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs @@ -26,5 +26,5 @@ internal interface ISolutionAssetProvider /// save substantially on performance by avoiding having to search the full solution tree to find matching items for /// a particular checksum. ValueTask WriteAssetsAsync( - PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken); + PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs index 66a9fbe476e8a..52a3f75670cc7 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs @@ -28,7 +28,7 @@ internal sealed class SolutionAssetProvider(SolutionServices services) : ISoluti public ValueTask WriteAssetsAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint? assetHint, + AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { @@ -41,7 +41,7 @@ public ValueTask WriteAssetsAsync( using var _ = FlowControlHelper.TrySuppressFlow(); return WriteAssetsSuppressedFlowAsync(pipeWriter, solutionChecksum, assetHint, checksums, cancellationToken); - async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { // The responsibility is on us (as per the requirements of RemoteCallback.InvokeAsync) to Complete the // pipewriter. This will signal to streamjsonrpc that the writer passed into it is complete, which will @@ -65,7 +65,7 @@ async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum s private async ValueTask WriteAssetsWorkerAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint? assetHint, + AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index 0d19173d2fe50..3e227a5dca94b 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -45,7 +45,7 @@ public void Dispose() /// the storage. /// public async Task AddAssetsAsync( - AssetHint? assetHint, + ssetHint? assetHint, ReadOnlyMemory checksums, Dictionary assetMap, CancellationToken cancellationToken) @@ -65,7 +65,7 @@ public async Task AddAssetsAsync( } private async Task FindAssetsAsync( - AssetHint? assetHint, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) + AssetHint assetHint, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) { var solutionState = this.CompilationState; diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index 35d639e349402..fd72dfbe20d76 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -31,7 +31,7 @@ internal sealed partial class AssetProvider(Checksum solutionChecksum, SolutionA private readonly IAssetSource _assetSource = assetSource; public override async ValueTask GetAssetAsync( - AssetHint? assetHint, Checksum checksum, CancellationToken cancellationToken) + AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksum == Checksum.Null); if (_assetCache.TryGetAsset(checksum, out var asset)) @@ -113,7 +113,7 @@ public async ValueTask SynchronizeProjectAssetsAsync(ProjectStateChecksums proje } public async ValueTask SynchronizeAssetsAsync( - AssetHint? assetHint, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) + AssetHint assetHint, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksums.Contains(Checksum.Null)); if (checksums.Count == 0) @@ -193,7 +193,7 @@ void AddResult(Checksum checksum, object result) } private async ValueTask> RequestAssetsAsync( - AssetHint? assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) { #if NETCOREAPP Contract.ThrowIfTrue(checksums.Span.Contains(Checksum.Null)); diff --git a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs index 9807556f35022..840e043daa46a 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs @@ -16,5 +16,5 @@ namespace Microsoft.CodeAnalysis.Remote; internal interface IAssetSource { ValueTask> GetAssetsAsync( - Checksum solutionChecksum, AssetHint? assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); + Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs index 9d9cd943d0869..4917033beae78 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs @@ -18,7 +18,7 @@ internal sealed class SolutionAssetSource(ServiceBrokerClient client) : IAssetSo public async ValueTask> GetAssetsAsync( Checksum solutionChecksum, - AssetHint? assetHint, + AssetHint assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken) From 1a2ee5a8ff71630f28ce0f8f571675ce6edda21a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:07:50 -0700 Subject: [PATCH 07/40] fix' --- src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index 3e227a5dca94b..3dcfa4cbca45b 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -45,7 +45,7 @@ public void Dispose() /// the storage. /// public async Task AddAssetsAsync( - ssetHint? assetHint, + AssetHint assetHint, ReadOnlyMemory checksums, Dictionary assetMap, CancellationToken cancellationToken) @@ -100,7 +100,7 @@ public async ValueTask GetAssetAsync(Checksum checksum, CancellationToke using var checksumPool = Creator.CreateChecksumSet(checksum); using var _ = Creator.CreateResultMap(out var resultPool); - await scope.FindAssetsAsync(assetHint: null, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); + await scope.FindAssetsAsync(AssetHint.FullLookupForTesting, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(resultPool.Count != 1); var (resultingChecksum, value) = resultPool.First(); From df48653f0fe20e8fe8042e0ee155c6473e13cd8c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:10:11 -0700 Subject: [PATCH 08/40] Update tests --- .../Core/Test.Next/Services/AssetProviderTests.cs | 6 +++--- src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs index 58ed5c28a5928..0fc6cd594f0e3 100644 --- a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs @@ -53,10 +53,10 @@ private static async Task TestAssetAsync(object data) var assetSource = new SimpleAssetSource(workspace.Services.GetService(), new Dictionary() { { checksum, data } }); var provider = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService()); - var stored = await provider.GetAssetAsync(assetHint: AssetHint.None, checksum, CancellationToken.None); + var stored = await provider.GetAssetAsync(AssetHint.FullLookupForTesting, checksum, CancellationToken.None); Assert.Equal(data, stored); - var stored2 = await provider.GetAssetsAsync(assetHint: AssetHint.None, [checksum], CancellationToken.None); + var stored2 = await provider.GetAssetsAsync(AssetHint.FullLookupForTesting, [checksum], CancellationToken.None); Assert.Equal(1, stored2.Length); Assert.Equal(checksum, stored2[0].Item1); @@ -83,7 +83,7 @@ public async Task TestAssetSynchronization() var assetSource = new SimpleAssetSource(workspace.Services.GetService(), map); var service = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService()); - await service.SynchronizeAssetsAsync(assetHint: AssetHint.None, new HashSet(map.Keys), results: null, CancellationToken.None); + await service.SynchronizeAssetsAsync(AssetHint.FullLookupForTesting, new HashSet(map.Keys), results: null, CancellationToken.None); foreach (var kv in map) { diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index 272587bcce992..b243d02066513 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -104,7 +104,7 @@ async Task>> GetAssetFromAssetServiceAsync(I foreach (var checksum in checksums) { items.Add(new KeyValuePair(checksum, await assetService.GetAssetAsync( - assetHint: null, checksum, CancellationToken.None).ConfigureAwait(false))); + AssetHint.FullLookupForTesting, checksum, CancellationToken.None).ConfigureAwait(false))); } return items; @@ -197,17 +197,17 @@ public static async Task AppendAssetMapAsync( if (projectId == null) { var compilationChecksums = await solution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: null, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var frozenSourceGeneratedDocumentState in solution.CompilationState.FrozenSourceGeneratedDocumentStates?.States.Values ?? []) { var documentChecksums = await frozenSourceGeneratedDocumentState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, assetHint: null, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); } var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(solutionChecksums.ProjectCone != null); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, assetHintOpt: null, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var project in solution.Projects) await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); @@ -219,7 +219,7 @@ public static async Task AppendAssetMapAsync( var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectCone.Equals(solutionChecksums.ProjectCone)); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetHintOpt: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetHint: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); var project = solution.GetRequiredProject(projectId); await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); From 60bdda637950b3c39debc58ef50efd2cc2ba27a4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:11:05 -0700 Subject: [PATCH 09/40] Update docs --- src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 26eb47b7baae9..60f927e72b905 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -19,7 +19,7 @@ internal readonly struct AssetHint public static AssetHint SolutionOnly = default; /// - /// Special instance, allowed in tests, that can do a full lookup across the entire checksum tree. + /// Special instance, allowed only in tests, that can do a full lookup across the entire checksum tree. /// public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, true); From b76ced54bff0c82a5eae68e930efcf46d8f20c12 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:17:19 -0700 Subject: [PATCH 10/40] fix --- .../Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs index 32d56a5b21036..9cbe826289f6c 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs @@ -123,6 +123,7 @@ public async ValueTask AddPinnedSolutionsAsync(HashSet solutions, Canc { using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { + solutions.Add(this.CurrentSolution); solutions.AddIfNotNull(_lastRequestedPrimaryBranchSolution.solution); _lastRequestedAnyBranchSolutions.AddAllTo(solutions); }; From d31205245226ba257828d2749b066a8c166f07be Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:30:11 -0700 Subject: [PATCH 11/40] in progress --- .../Remote/ServiceHub/Host/ChecksumSynchronizer.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index e47cc7ed14c9c..aefdd7e583b34 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -49,8 +49,8 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, { using var _ = PooledHashSet.GetInstance(out var checksums); - checksums.Add(solutionCompilationChecksumObject.SourceGeneratorExecutionVersionMap); - solutionChecksumObject.AddAllTo(checksums); + solutionCompilationChecksumObject.AddAllTo(checksums); + solutionChecksumObject.AnalyzerReferences.AddAllTo(checksums); await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); } } @@ -58,8 +58,6 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, // third and last get direct children for all projects and documents in the solution foreach (var (projectChecksum, projectId) in solutionChecksumObject.Projects) { - // These GetAssetAsync calls should be fast since they were just retrieved above. There's a small - // chance the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. var projectStateChecksums = await _assetProvider.GetAssetAsync( assetHint: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); await SynchronizeProjectAssetsAsync(projectStateChecksums, cancellationToken).ConfigureAwait(false); From 65befaa8b2e5dbf21221a1096df69797dff74d21 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 00:36:51 -0700 Subject: [PATCH 12/40] Simplify --- .../ServiceHub/Host/ChecksumSynchronizer.cs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index aefdd7e583b34..1e639e122cd42 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -34,29 +34,36 @@ public async ValueTask SynchronizeAssetsAsync( public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { - SolutionStateChecksums solutionChecksumObject; + SolutionStateChecksums stateChecksums; using (await s_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - // this will make 4 round trip to data source (VS) to get all assets that belong to the given solution checksum - - // first, get solution checksum object for the given solution checksum - var solutionCompilationChecksumObject = await _assetProvider.GetAssetAsync( + // first, get top level solution state for the given solution checksum + var compilationStateChecksums = await _assetProvider.GetAssetAsync( assetHint: AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); - solutionChecksumObject = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, solutionCompilationChecksumObject.SolutionState, cancellationToken).ConfigureAwait(false); - // second, get direct children of the solution + // second, get direct children of the solution compilation state. + { + using var _ = PooledHashSet.GetInstance(out var checksums); + + compilationStateChecksums.AddAllTo(checksums); + await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); + } + + // third, get direct children of the solution state. { + stateChecksums = await _assetProvider.GetAssetAsync( + assetHint: AssetHint.SolutionOnly, compilationStateChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + using var _ = PooledHashSet.GetInstance(out var checksums); - solutionCompilationChecksumObject.AddAllTo(checksums); - solutionChecksumObject.AnalyzerReferences.AddAllTo(checksums); + checksums.Add(stateChecksums.Attributes); + stateChecksums.AnalyzerReferences.AddAllTo(checksums); await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); } } - // third and last get direct children for all projects and documents in the solution - foreach (var (projectChecksum, projectId) in solutionChecksumObject.Projects) + // fourth, get all projects and documents in the solution + foreach (var (projectChecksum, projectId) in stateChecksums.Projects) { var projectStateChecksums = await _assetProvider.GetAssetAsync( assetHint: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); From 9b71463bd9fc46ffb8a0b84fc7bf1172576d2244 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:04:05 -0700 Subject: [PATCH 13/40] Simplify --- .../Portable/Workspace/Solution/AssetHint.cs | 19 +++++++--- .../Workspace/Solution/StateChecksums.cs | 27 +++++++++++++- .../ServiceHub/Host/ChecksumSynchronizer.cs | 35 +++++++++---------- 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 60f927e72b905..d54149c12f3cd 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -18,27 +18,36 @@ internal readonly struct AssetHint /// public static AssetHint SolutionOnly = default; + /// + /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for + /// checksums. It will not descend into projects. + /// + public static AssetHint SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); + /// /// Special instance, allowed only in tests, that can do a full lookup across the entire checksum tree. /// - public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, true); + public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); [DataMember(Order = 0)] public readonly ProjectId? ProjectId; [DataMember(Order = 1)] public readonly DocumentId? DocumentId; - [DataMember(Order = 2)] + [DataMember(Order = 3)] + public readonly bool TopLevelProjects; + [DataMember(Order = 4)] public readonly bool IsFullLookup_ForTestingPurposesOnly; public bool IsSolutionOnly => !IsFullLookup_ForTestingPurposesOnly && ProjectId is null; - private AssetHint(ProjectId? projectId, DocumentId? documentId, bool isFullLookup_ForTestingPurposesOnly) + private AssetHint(ProjectId? projectId, DocumentId? documentId, bool topLevelProjects, bool isFullLookup_ForTestingPurposesOnly) { ProjectId = projectId; DocumentId = documentId; + TopLevelProjects = topLevelProjects; IsFullLookup_ForTestingPurposesOnly = isFullLookup_ForTestingPurposesOnly; } - public static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null, false); - public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId, false); + public static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); + public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index c0e66f2120481..6203d2ef82696 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -256,12 +256,38 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; + if (assetHint.TopLevelProjects) + { + // Caller is trying to fetch the top level ProjectStateChecksums as well. Look for those without diving deeper. + foreach (var (projectId, projectState) in solution.ProjectStates) + { + if (searchingChecksumsLeft.Count == 0) + break; + + // If we're syncing a project cone, no point at all at looking at child projects of the solution that + // are not in that cone. + if (projectCone != null && !projectCone.Contains(projectId)) + continue; + + if (projectState.TryGetStateChecksums(out var projectStateChecksums) && + searchingChecksumsLeft.Remove(projectStateChecksums.Checksum)) + { + result[projectStateChecksums.Checksum] = projectStateChecksums; + } + } + + if (searchingChecksumsLeft.Count == 0) + return; + } + if (!assetHint.IsFullLookup_ForTestingPurposesOnly) { // Caller said they were only looking for solution level assets. no need to go any further. if (assetHint.IsSolutionOnly) return; + // Since we're not solution-only, we must have a project id being requested. Dive into that project alone + // to search for the remaining checksums. Contract.ThrowIfNull(assetHint.ProjectId); Contract.ThrowIfTrue( projectCone != null && !projectCone.Contains(assetHint.ProjectId), @@ -277,7 +303,6 @@ public async Task FindAsync( else { // Full search, used for test purposes. - foreach (var (projectId, projectState) in solution.ProjectStates) { if (searchingChecksumsLeft.Count == 0) diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index 1e639e122cd42..03c4b84705049 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -34,6 +34,8 @@ public async ValueTask SynchronizeAssetsAsync( public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { + using var _1 = PooledDictionary.GetInstance(out var checksumToObjects); + SolutionStateChecksums stateChecksums; using (await s_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { @@ -41,32 +43,27 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, var compilationStateChecksums = await _assetProvider.GetAssetAsync( assetHint: AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); - // second, get direct children of the solution compilation state. - { - using var _ = PooledHashSet.GetInstance(out var checksums); + using var _2 = PooledHashSet.GetInstance(out var checksums); - compilationStateChecksums.AddAllTo(checksums); - await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); - } + // second, get direct children of the solution compilation state. + compilationStateChecksums.AddAllTo(checksums); + await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); // third, get direct children of the solution state. - { - stateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, compilationStateChecksums.SolutionState, cancellationToken).ConfigureAwait(false); - - using var _ = PooledHashSet.GetInstance(out var checksums); - - checksums.Add(stateChecksums.Attributes); - stateChecksums.AnalyzerReferences.AddAllTo(checksums); - await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); - } + stateChecksums = await _assetProvider.GetAssetAsync( + assetHint: AssetHint.SolutionOnly, compilationStateChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + + // Ask for solutions and top-level projects as the solution checksums will contain the checksums for + // the project states and we want to get that all in one batch. + checksums.Clear(); + stateChecksums.AddAllTo(checksums); + await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionAndTopLevelProjectsOnly, checksums, checksumToObjects, cancellationToken).ConfigureAwait(false); } // fourth, get all projects and documents in the solution - foreach (var (projectChecksum, projectId) in stateChecksums.Projects) + foreach (var (projectChecksum, _) in stateChecksums.Projects) { - var projectStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); + var projectStateChecksums = (ProjectStateChecksums)checksumToObjects[projectChecksum]; await SynchronizeProjectAssetsAsync(projectStateChecksums, cancellationToken).ConfigureAwait(false); } } From aa3c405f1fef9d10daf7d8c146bea0b6a1944868 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:05:06 -0700 Subject: [PATCH 14/40] Simplify --- .../Core/Portable/Workspace/Solution/StateChecksums.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 6203d2ef82696..2e461a46ba396 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -253,9 +253,6 @@ public async Task FindAsync( ChecksumCollection.Find(solution.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); - if (searchingChecksumsLeft.Count == 0) - return; - if (assetHint.TopLevelProjects) { // Caller is trying to fetch the top level ProjectStateChecksums as well. Look for those without diving deeper. @@ -275,11 +272,11 @@ public async Task FindAsync( result[projectStateChecksums.Checksum] = projectStateChecksums; } } - - if (searchingChecksumsLeft.Count == 0) - return; } + if (searchingChecksumsLeft.Count == 0) + return; + if (!assetHint.IsFullLookup_ForTestingPurposesOnly) { // Caller said they were only looking for solution level assets. no need to go any further. From 7163426c29e21cc55896fa13eadbda17b8373bab Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:07:38 -0700 Subject: [PATCH 15/40] comment --- src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index d54149c12f3cd..3448ab7313456 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -25,7 +25,8 @@ internal readonly struct AssetHint public static AssetHint SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); /// - /// Special instance, allowed only in tests, that can do a full lookup across the entire checksum tree. + /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum + /// tree. Should not be used in normal release-mode product code. /// public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); From 616478a16c48d61eaccad45fb915d1b0448f49d9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:10:39 -0700 Subject: [PATCH 16/40] restore --- .../Host/RemoteWorkspace.SolutionCreator.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index a3eef0a8a5c9c..44b76316777d4 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -215,14 +215,16 @@ private async Task UpdateProjectsAsync( oldProjectIdToStateChecksums.Add(projectId, oldProjectStateChecksums); } - // sync over the *info* about all the added/changed projects. We'll want the info so we can determine - // what actually changed. - foreach (var (projectId, checksum) in newProjectIdToChecksum) + using var _5 = PooledHashSet.GetInstance(out var newChecksumsToSync); + newChecksumsToSync.AddRange(newProjectIdToChecksum.Values); + + var newProjectStateChecksums = await _assetProvider.GetAssetsAsync( + assetHint: AssetHint.SolutionAndTopLevelProjectsOnly, newChecksumsToSync, cancellationToken).ConfigureAwait(false); + + foreach (var (checksum, newProjectStateChecksum) in newProjectStateChecksums) { - var newProjectStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: projectId, checksum, cancellationToken).ConfigureAwait(false); - Contract.ThrowIfTrue(newProjectStateChecksums.Checksum != checksum); - newProjectIdToStateChecksums.Add(projectId, newProjectStateChecksums); + Contract.ThrowIfTrue(checksum != newProjectStateChecksum.Checksum); + newProjectIdToStateChecksums.Add(newProjectStateChecksum.ProjectId, newProjectStateChecksum); } // Now that we've collected the old and new project state checksums, we can actually process them to From 4a27911bf1b66ddefb28ca691f2031d3e92fe75c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:20:22 -0700 Subject: [PATCH 17/40] docs --- .../ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs index 9cbe826289f6c..65acd364c7f93 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs @@ -123,8 +123,17 @@ public async ValueTask AddPinnedSolutionsAsync(HashSet solutions, Canc { using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { + // Ensure everything in the workspace's current solution is pinned. We def don't want any of its data + // dropped from the checksum->asset cache. solutions.Add(this.CurrentSolution); + + // Also the data for the last 'current solution' this workspace had that we actually got an OOP request + // for. this is commonly the same as CurrentSolution, but technically could be slightly behind if the + // primary solution just got updated. solutions.AddIfNotNull(_lastRequestedPrimaryBranchSolution.solution); + + // Also add the last few forked solutions we were asked about. As with the above solutions, there's a + // reasonable chance it will refer to data needed by future oop calls. _lastRequestedAnyBranchSolutions.AddAllTo(solutions); }; } From 5a20b4191099b3684dc27475bf7cbd3f9995eec2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:21:18 -0700 Subject: [PATCH 18/40] Simplify --- src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs index 870848f414c9b..9d7ddc0d3a3b7 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs @@ -186,7 +186,6 @@ private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationTok // Ensure that if our remote workspace has a current solution, that we don't purge any items associated // with that solution. using var _1 = PooledHashSet.GetInstance(out var pinnedChecksums); - var computePinnedInformation = false; foreach (var (checksum, entry) in _assets) { @@ -195,11 +194,8 @@ private async ValueTask CleanAssetsWorkerAsync(CancellationToken cancellationTok continue; // If this is a checksum we want to pin, do not remove it. - if (!computePinnedInformation) - { + if (pinnedChecksums.Count == 0) await AddPinnedChecksumsAsync(pinnedChecksums, cancellationToken).ConfigureAwait(false); - computePinnedInformation = true; - } if (pinnedChecksums.Contains(checksum)) continue; From d729f63aab96a020f12cc3985c99860b0b0217a9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 11:46:14 -0700 Subject: [PATCH 19/40] lint --- .../Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs index 65acd364c7f93..119d58327f58f 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs @@ -135,7 +135,7 @@ public async ValueTask AddPinnedSolutionsAsync(HashSet solutions, Canc // Also add the last few forked solutions we were asked about. As with the above solutions, there's a // reasonable chance it will refer to data needed by future oop calls. _lastRequestedAnyBranchSolutions.AddAllTo(solutions); - }; + } } } } From 502fccb4759bc152d8a8d738b26dda501f01e163 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 12:02:50 -0700 Subject: [PATCH 20/40] remove --- .../Remote/Core/SolutionAssetStorage.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs index 6ca9561dd8006..8e87958c46eb3 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.cs @@ -131,25 +131,5 @@ public async ValueTask GetRequiredAssetAsync(Checksum checksum, Cancella { return await _solutionAssetStorage._checksumToScope.Single().Value.GetTestAccessor().GetAssetAsync(checksum, cancellationToken).ConfigureAwait(false); } - - public bool IsPinned(Checksum checksum) - { - lock (_solutionAssetStorage._gate) - { - return _solutionAssetStorage._checksumToScope.TryGetValue(checksum, out var scope) && - scope.RefCount >= 1; - } - } - - public int PinnedScopesCount - { - get - { - lock (_solutionAssetStorage._gate) - { - return _solutionAssetStorage._checksumToScope.Count; - } - } - } } } From 63665c9ab37bd88faf2f3ff3d4887b9e408bc8d1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 15:57:05 -0700 Subject: [PATCH 21/40] Switch to using index operator --- .../Collections/ImmutableSegmentedListExtensions.cs | 4 ++-- src/Dependencies/Collections/SegmentedArray`1.cs | 2 +- src/Dependencies/PooledObjects/ArrayBuilder.cs | 2 +- .../RegularExpressions/CSharpRegexParserTests.cs | 2 +- .../IntelliSense/QuickInfo/IntellisenseQuickInfoBuilder.cs | 4 ++-- .../Portable/LineSeparators/CSharpLineSeparatorService.cs | 2 +- .../RegularExpressions/LanguageServices/RegexBraceMatcher.cs | 2 +- .../CommonObjectFormatter.Visitor.FormattedMember.cs | 2 +- .../Hosting/ObjectFormatter/CommonObjectFormatter.Visitor.cs | 2 +- src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs | 2 +- src/VisualStudio/Core/Def/Preview/FileChange.cs | 2 +- .../Core/Def/ValueTracking/ValueTrackingTree.xaml.cs | 2 +- src/VisualStudio/Core/Def/Venus/ContainedDocument.cs | 2 +- .../AbstractCodeModelService.AbstractNodeNameGenerator.cs | 2 +- .../LiveShare/Impl/Client/RemoteLanguageServiceWorkspace.cs | 2 +- .../Core/Portable/Classification/ClassifierHelper.cs | 2 +- src/Workspaces/MSBuildTest/MSBuildWorkspaceTestBase.cs | 4 ++-- .../SyntaxFacts/AbstractDocumentationCommentService.cs | 2 +- .../Compiler/Core/SymbolKey/SymbolKey.SymbolKeyWriter.cs | 2 +- .../Compiler/Core/Utilities/EditDistance.cs | 2 +- .../Workspace/CSharp/CodeGeneration/EnumMemberGenerator.cs | 2 +- 21 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs b/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs index 19ad004d99d95..feb9da60efdc2 100644 --- a/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs +++ b/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs @@ -65,7 +65,7 @@ public static T Last(this ImmutableSegmentedList immutableList) // In the event of an empty list, generate the same exception // that the linq extension method would. return immutableList.Count > 0 - ? immutableList[immutableList.Count - 1] + ? immutableList[^1] : Enumerable.Last(immutableList); } @@ -77,7 +77,7 @@ public static T Last(this ImmutableSegmentedList.Builder builder) // In the event of an empty list, generate the same exception // that the linq extension method would. return builder.Count > 0 - ? builder[builder.Count - 1] + ? builder[^1] : Enumerable.Last(builder); } diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index ac7f5d9801ce2..f30e8a415a4cd 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -89,7 +89,7 @@ public SegmentedArray(int length) // Avoid using (length & s_offsetMask) because it doesn't handle a last page size of s_segmentSize. var lastPageSize = length - ((_items.Length - 1) << SegmentShift); - _items[_items.Length - 1] = new T[lastPageSize]; + _items[^1] = new T[lastPageSize]; _length = length; } } diff --git a/src/Dependencies/PooledObjects/ArrayBuilder.cs b/src/Dependencies/PooledObjects/ArrayBuilder.cs index b0246977a88b4..6ee729ff8ba55 100644 --- a/src/Dependencies/PooledObjects/ArrayBuilder.cs +++ b/src/Dependencies/PooledObjects/ArrayBuilder.cs @@ -321,7 +321,7 @@ public void CopyTo(T[] array, int start) } public T Last() - => _builder[_builder.Count - 1]; + => _builder[^1]; internal T? LastOrDefault() => Count == 0 ? default : Last(); diff --git a/src/EditorFeatures/CSharpTest2/EmbeddedLanguages/RegularExpressions/CSharpRegexParserTests.cs b/src/EditorFeatures/CSharpTest2/EmbeddedLanguages/RegularExpressions/CSharpRegexParserTests.cs index eb6e97d8a621b..8b61883e334bd 100644 --- a/src/EditorFeatures/CSharpTest2/EmbeddedLanguages/RegularExpressions/CSharpRegexParserTests.cs +++ b/src/EditorFeatures/CSharpTest2/EmbeddedLanguages/RegularExpressions/CSharpRegexParserTests.cs @@ -344,7 +344,7 @@ private static void CheckCharacters(VirtualCharSequence virtualChars, ref int po private static string And(params string[] regexes) { - var conj = $"({regexes[regexes.Length - 1]})"; + var conj = $"({regexes[^1]})"; for (var i = regexes.Length - 2; i >= 0; i--) conj = $"(?({regexes[i]}){conj}|[0-[0]])"; diff --git a/src/EditorFeatures/Core/IntelliSense/QuickInfo/IntellisenseQuickInfoBuilder.cs b/src/EditorFeatures/Core/IntelliSense/QuickInfo/IntellisenseQuickInfoBuilder.cs index 3515897e3310b..6cff75d9f36c9 100644 --- a/src/EditorFeatures/Core/IntelliSense/QuickInfo/IntellisenseQuickInfoBuilder.cs +++ b/src/EditorFeatures/Core/IntelliSense/QuickInfo/IntellisenseQuickInfoBuilder.cs @@ -81,8 +81,8 @@ private static async Task BuildInteractiveContentAsync( // Stack the first paragraph of the documentation comments with the last line of the description // to avoid vertical padding between the two. - var lastElement = elements[elements.Count - 1]; - elements[elements.Count - 1] = new ContainerElement( + var lastElement = elements[^1]; + elements[^1] = new ContainerElement( ContainerElementStyle.Stacked, lastElement, element); diff --git a/src/Features/CSharp/Portable/LineSeparators/CSharpLineSeparatorService.cs b/src/Features/CSharp/Portable/LineSeparators/CSharpLineSeparatorService.cs index 75f87a9720437..36af95476fd8a 100644 --- a/src/Features/CSharp/Portable/LineSeparators/CSharpLineSeparatorService.cs +++ b/src/Features/CSharp/Portable/LineSeparators/CSharpLineSeparatorService.cs @@ -299,7 +299,7 @@ private static void ProcessNodeList(SyntaxList children, ArrayBuilder= 0 && Name != null && Name.Length >= 2 && Name[0] == '[' && Name[Name.Length - 1] == ']'; + return Index >= 0 && Name != null && Name.Length >= 2 && Name[0] == '[' && Name[^1] == ']'; } public bool AppendAsCollectionEntry(Builder result) diff --git a/src/Scripting/Core/Hosting/ObjectFormatter/CommonObjectFormatter.Visitor.cs b/src/Scripting/Core/Hosting/ObjectFormatter/CommonObjectFormatter.Visitor.cs index 5c90553b23d48..2303b85fedd59 100644 --- a/src/Scripting/Core/Hosting/ObjectFormatter/CommonObjectFormatter.Visitor.cs +++ b/src/Scripting/Core/Hosting/ObjectFormatter/CommonObjectFormatter.Visitor.cs @@ -697,7 +697,7 @@ private void FormatMultidimensionalArrayElements(Builder result, Array array, bo string _; FormatObjectRecursive(result, array.GetValue(indices), isRoot: false, debuggerDisplayName: out _); - indices[indices.Length - 1]++; + indices[^1]++; flatIndex++; } } diff --git a/src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs b/src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs index 12578c5fe84c1..5e6e6808b042c 100644 --- a/src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs +++ b/src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs @@ -59,7 +59,7 @@ public NavInfo( _basePresentationNodes = CreateNodes(expandDottedNames: false); _symbolType = _basePresentationNodes.Length > 0 - ? _basePresentationNodes[_basePresentationNodes.Length - 1].ListType + ? _basePresentationNodes[^1].ListType : 0; } diff --git a/src/VisualStudio/Core/Def/Preview/FileChange.cs b/src/VisualStudio/Core/Def/Preview/FileChange.cs index 2effe84546440..37c83672c3755 100644 --- a/src/VisualStudio/Core/Def/Preview/FileChange.cs +++ b/src/VisualStudio/Core/Def/Preview/FileChange.cs @@ -135,7 +135,7 @@ private static string GetDisplayText(string excerpt) var split = excerpt.Split(new[] { "\r\n" }, StringSplitOptions.RemoveEmptyEntries); if (split.Length > 1) { - return string.Format("{0} ... {1}", split[0].Trim(), split[split.Length - 1].Trim()); + return string.Format("{0} ... {1}", split[0].Trim(), split[^1].Trim()); } } diff --git a/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingTree.xaml.cs b/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingTree.xaml.cs index 8692768d94ba5..011c3c055f5bf 100644 --- a/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingTree.xaml.cs +++ b/src/VisualStudio/Core/Def/ValueTracking/ValueTrackingTree.xaml.cs @@ -114,7 +114,7 @@ private TreeViewItemBase GetPreviousItem() { if (ValueTrackingTreeView.SelectedItem is null) { - return (TreeViewItemBase)ValueTrackingTreeView.Items[ValueTrackingTreeView.Items.Count - 1]; + return (TreeViewItemBase)ValueTrackingTreeView.Items[^1]; } var item = (TreeViewItemBase)ValueTrackingTreeView.SelectedItem; diff --git a/src/VisualStudio/Core/Def/Venus/ContainedDocument.cs b/src/VisualStudio/Core/Def/Venus/ContainedDocument.cs index 5dbb5103ff05b..659163fe9c21b 100644 --- a/src/VisualStudio/Core/Def/Venus/ContainedDocument.cs +++ b/src/VisualStudio/Core/Def/Venus/ContainedDocument.cs @@ -1073,7 +1073,7 @@ private bool IsCodeBlock(ITextSnapshot surfaceSnapshot, int position, char ch) private static bool CheckCode(ITextSnapshot snapshot, int position, char ch, string tag, bool checkAt = true) { - if (ch != tag[tag.Length - 1] || position < tag.Length) + if (ch != tag[^1] || position < tag.Length) { return false; } diff --git a/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.AbstractNodeNameGenerator.cs b/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.AbstractNodeNameGenerator.cs index d6e7c57d32ec6..8abf72e54eb40 100644 --- a/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.AbstractNodeNameGenerator.cs +++ b/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.AbstractNodeNameGenerator.cs @@ -23,7 +23,7 @@ protected abstract class AbstractNodeNameGenerator protected static void AppendDotIfNeeded(StringBuilder builder) { if (builder.Length > 0 && - char.IsLetterOrDigit(builder[builder.Length - 1])) + char.IsLetterOrDigit(builder[^1])) { builder.Append('.'); } diff --git a/src/VisualStudio/LiveShare/Impl/Client/RemoteLanguageServiceWorkspace.cs b/src/VisualStudio/LiveShare/Impl/Client/RemoteLanguageServiceWorkspace.cs index ef96a72e3debb..b6eff7bf64e92 100644 --- a/src/VisualStudio/LiveShare/Impl/Client/RemoteLanguageServiceWorkspace.cs +++ b/src/VisualStudio/LiveShare/Impl/Client/RemoteLanguageServiceWorkspace.cs @@ -175,7 +175,7 @@ private async Task UpdatePathsToRemoteFilesAsync(CollaborationSession session) #pragma warning disable CS8602 // Dereference of a possibly null reference. (Can localRoot be null here?) var splitRoot = localRoot.TrimEnd('\\').Split('\\'); #pragma warning restore CS8602 // Dereference of a possibly null reference. - splitRoot[splitRoot.Length - 1] = "~external"; + splitRoot[^1] = "~external"; var externalPath = string.Join("\\", splitRoot) + "\\"; remoteRootPaths.Add(localRoot); diff --git a/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs b/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs index 1d45eb4e6ab7b..478c0fe6e76f2 100644 --- a/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs +++ b/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs @@ -277,7 +277,7 @@ public static void MergeParts 0) { - return classBlock.Implements[classBlock.Implements.Count - 1].FullSpan.End; + return classBlock.Implements[^1].FullSpan.End; } else if (classBlock.Inherits.Count > 0) { - return classBlock.Inherits[classBlock.Inherits.Count - 1].FullSpan.End; + return classBlock.Inherits[^1].FullSpan.End; } else { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractDocumentationCommentService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractDocumentationCommentService.cs index 8234264c462f6..6c75e926003b1 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractDocumentationCommentService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractDocumentationCommentService.cs @@ -185,7 +185,7 @@ private static bool HasLeadingWhitespace(string tokenText) => tokenText.Length > 0 && char.IsWhiteSpace(tokenText[0]); private static bool HasTrailingWhitespace(string tokenText) - => tokenText.Length > 0 && char.IsWhiteSpace(tokenText[tokenText.Length - 1]); + => tokenText.Length > 0 && char.IsWhiteSpace(tokenText[^1]); public string GetBannerText(SyntaxNode documentationCommentTriviaSyntax, int maxBannerLength, CancellationToken cancellationToken) => GetBannerText((TDocumentationCommentTriviaSyntax)documentationCommentTriviaSyntax, maxBannerLength, cancellationToken); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/SymbolKey/SymbolKey.SymbolKeyWriter.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/SymbolKey/SymbolKey.SymbolKeyWriter.cs index 372759c1b67c3..fa33b1cebae1c 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/SymbolKey/SymbolKey.SymbolKeyWriter.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/SymbolKey/SymbolKey.SymbolKeyWriter.cs @@ -516,7 +516,7 @@ public void PushMethod(IMethodSymbol method) public void PopMethod(IMethodSymbol method) { Contract.ThrowIfTrue(_methodSymbolStack.Count == 0); - Contract.ThrowIfFalse(method.Equals(_methodSymbolStack[_methodSymbolStack.Count - 1])); + Contract.ThrowIfFalse(method.Equals(_methodSymbolStack[^1])); _methodSymbolStack.RemoveAt(_methodSymbolStack.Count - 1); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EditDistance.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EditDistance.cs index 086cb0e922331..e32225e85e0a0 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EditDistance.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EditDistance.cs @@ -178,7 +178,7 @@ private static int GetEditDistanceWorker(ReadOnlySpan source, ReadOnlySpan // First: // Determine the common prefix/suffix portions of the strings. We don't even need to // consider them as they won't add anything to the edit cost. - while (source.Length > 0 && source[source.Length - 1] == target[target.Length - 1]) + while (source.Length > 0 && source[^1] == target[^1]) { source = source[..^1]; target = target[..^1]; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/EnumMemberGenerator.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/EnumMemberGenerator.cs index aa9928a932d1a..7241330c650b3 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/EnumMemberGenerator.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/EnumMemberGenerator.cs @@ -39,7 +39,7 @@ internal static EnumDeclarationSyntax AddEnumMemberTo(EnumDeclarationSyntax dest { var lastMember = members.Last(); var trailingTrivia = lastMember.GetTrailingTrivia(); - members[members.Count - 1] = lastMember.WithTrailingTrivia(); + members[^1] = lastMember.WithTrailingTrivia(); members.Add(SyntaxFactory.Token(SyntaxKind.CommaToken).WithTrailingTrivia(trailingTrivia)); members.Add(member); } From 98eec08c5da6b06b381268a6dc7a367a17dbdcd5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 16:01:26 -0700 Subject: [PATCH 22/40] revert --- .../Collections/ImmutableSegmentedListExtensions.cs | 4 ++-- src/Dependencies/Collections/SegmentedArray`1.cs | 2 +- src/Dependencies/PooledObjects/ArrayBuilder.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs b/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs index feb9da60efdc2..19ad004d99d95 100644 --- a/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs +++ b/src/Dependencies/Collections/ImmutableSegmentedListExtensions.cs @@ -65,7 +65,7 @@ public static T Last(this ImmutableSegmentedList immutableList) // In the event of an empty list, generate the same exception // that the linq extension method would. return immutableList.Count > 0 - ? immutableList[^1] + ? immutableList[immutableList.Count - 1] : Enumerable.Last(immutableList); } @@ -77,7 +77,7 @@ public static T Last(this ImmutableSegmentedList.Builder builder) // In the event of an empty list, generate the same exception // that the linq extension method would. return builder.Count > 0 - ? builder[^1] + ? builder[builder.Count - 1] : Enumerable.Last(builder); } diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index f30e8a415a4cd..ac7f5d9801ce2 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -89,7 +89,7 @@ public SegmentedArray(int length) // Avoid using (length & s_offsetMask) because it doesn't handle a last page size of s_segmentSize. var lastPageSize = length - ((_items.Length - 1) << SegmentShift); - _items[^1] = new T[lastPageSize]; + _items[_items.Length - 1] = new T[lastPageSize]; _length = length; } } diff --git a/src/Dependencies/PooledObjects/ArrayBuilder.cs b/src/Dependencies/PooledObjects/ArrayBuilder.cs index 6ee729ff8ba55..b0246977a88b4 100644 --- a/src/Dependencies/PooledObjects/ArrayBuilder.cs +++ b/src/Dependencies/PooledObjects/ArrayBuilder.cs @@ -321,7 +321,7 @@ public void CopyTo(T[] array, int start) } public T Last() - => _builder[^1]; + => _builder[_builder.Count - 1]; internal T? LastOrDefault() => Count == 0 ? default : Last(); From 39900f79624956003e1e0b3b719e76fd8ee668ba Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 16:13:06 -0700 Subject: [PATCH 23/40] make readonly --- .../Core/Portable/Workspace/Solution/AssetHint.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 3448ab7313456..3c219e0874cb9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -16,19 +16,19 @@ internal readonly struct AssetHint /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static AssetHint SolutionOnly = default; + public static readonly AssetHint SolutionOnly = default; /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static AssetHint SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); + public static readonly AssetHint SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static AssetHint FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); + public static readonly AssetHint FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); [DataMember(Order = 0)] public readonly ProjectId? ProjectId; From 800ed1dc9c47f66a551a3898f8aaf99577951682 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 16:18:00 -0700 Subject: [PATCH 24/40] rename type --- .../Portable/Workspace/Solution/AssetHint.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 3c219e0874cb9..db0c8f0c37e02 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -7,28 +7,29 @@ namespace Microsoft.CodeAnalysis.Serialization; /// -/// Optional information passed with an asset synchronization request to allow the request to be scoped down to a -/// particular or . +/// Required information passed with an asset synchronization request to tell the host where to scope the request to. In +/// particular, this is often used to scope to a particular or to avoid +/// having to search the entire solution. /// [DataContract] -internal readonly struct AssetHint +internal readonly struct AssetPath { /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static readonly AssetHint SolutionOnly = default; + public static readonly AssetPath SolutionOnly = default; /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetHint SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetHint FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); + public static readonly AssetPath FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); [DataMember(Order = 0)] public readonly ProjectId? ProjectId; @@ -41,7 +42,7 @@ internal readonly struct AssetHint public bool IsSolutionOnly => !IsFullLookup_ForTestingPurposesOnly && ProjectId is null; - private AssetHint(ProjectId? projectId, DocumentId? documentId, bool topLevelProjects, bool isFullLookup_ForTestingPurposesOnly) + private AssetPath(ProjectId? projectId, DocumentId? documentId, bool topLevelProjects, bool isFullLookup_ForTestingPurposesOnly) { ProjectId = projectId; DocumentId = documentId; @@ -49,6 +50,6 @@ private AssetHint(ProjectId? projectId, DocumentId? documentId, bool topLevelPro IsFullLookup_ForTestingPurposesOnly = isFullLookup_ForTestingPurposesOnly; } - public static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); - public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); + public static implicit operator AssetPath(ProjectId projectId) => new(projectId, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); + public static implicit operator AssetPath(DocumentId documentId) => new(documentId.ProjectId, documentId, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); } From 805d0ea67eac7d5d5ed270b2342388bcb99ccdd2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 16:20:17 -0700 Subject: [PATCH 25/40] rename type --- .../Remote/SerializationValidator.cs | 2 +- .../Test.Next/Services/AssetProviderTests.cs | 6 +-- .../Workspace/Solution/StateChecksums.cs | 22 +++++----- .../Fakes/SimpleAssetSource.cs | 2 +- .../Remote/Core/AbstractAssetProvider.cs | 36 ++++++++-------- .../Remote/Core/ISolutionAssetProvider.cs | 4 +- .../Remote/Core/SolutionAssetProvider.cs | 12 +++--- .../Remote/Core/SolutionAssetStorage.Scope.cs | 12 +++--- .../Remote/ServiceHub/Host/AssetProvider.cs | 16 +++---- .../ServiceHub/Host/ChecksumSynchronizer.cs | 18 ++++---- .../Remote/ServiceHub/Host/IAssetSource.cs | 2 +- .../Host/RemoteWorkspace.SolutionCreator.cs | 42 +++++++++---------- .../ServiceHub/Host/SolutionAssetSource.cs | 4 +- .../Remote/ServiceHub/Host/TestUtils.cs | 20 ++++----- 14 files changed, 99 insertions(+), 99 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs b/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs index 8b661cec79e46..7851c17671124 100644 --- a/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs +++ b/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs @@ -28,7 +28,7 @@ private sealed class AssetProvider : AbstractAssetProvider public AssetProvider(SerializationValidator validator) => _validator = validator; - public override async ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) + public override async ValueTask GetAssetAsync(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken) => await _validator.GetValueAsync(checksum).ConfigureAwait(false); } diff --git a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs index 0fc6cd594f0e3..e06e64efbf1ee 100644 --- a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs @@ -53,10 +53,10 @@ private static async Task TestAssetAsync(object data) var assetSource = new SimpleAssetSource(workspace.Services.GetService(), new Dictionary() { { checksum, data } }); var provider = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService()); - var stored = await provider.GetAssetAsync(AssetHint.FullLookupForTesting, checksum, CancellationToken.None); + var stored = await provider.GetAssetAsync(AssetPath.FullLookupForTesting, checksum, CancellationToken.None); Assert.Equal(data, stored); - var stored2 = await provider.GetAssetsAsync(AssetHint.FullLookupForTesting, [checksum], CancellationToken.None); + var stored2 = await provider.GetAssetsAsync(AssetPath.FullLookupForTesting, [checksum], CancellationToken.None); Assert.Equal(1, stored2.Length); Assert.Equal(checksum, stored2[0].Item1); @@ -83,7 +83,7 @@ public async Task TestAssetSynchronization() var assetSource = new SimpleAssetSource(workspace.Services.GetService(), map); var service = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService()); - await service.SynchronizeAssetsAsync(AssetHint.FullLookupForTesting, new HashSet(map.Keys), results: null, CancellationToken.None); + await service.SynchronizeAssetsAsync(AssetPath.FullLookupForTesting, new HashSet(map.Keys), results: null, CancellationToken.None); foreach (var kv in map) { diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 2e461a46ba396..5679153035ab4 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -111,7 +111,7 @@ public static SolutionCompilationStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionCompilationState compilationState, ProjectCone? projectCone, - AssetHint assetHint, + AssetPath assetPath, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -157,13 +157,13 @@ public async Task FindAsync( // If we're not in a project cone, start the search at the top most state-checksum corresponding to the // entire solution. Contract.ThrowIfFalse(solutionState.TryGetStateChecksums(out var solutionChecksums)); - await solutionChecksums.FindAsync(solutionState, projectCone, assetHint, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solutionState, projectCone, assetPath, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } else { // Otherwise, grab the top-most state checksum for this cone and search within that. Contract.ThrowIfFalse(solutionState.TryGetStateChecksums(projectCone.RootProjectId, out var solutionChecksums)); - await solutionChecksums.FindAsync(solutionState, projectCone, assetHint, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solutionState, projectCone, assetPath, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } } } @@ -235,7 +235,7 @@ public static SolutionStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( SolutionState solution, ProjectCone? projectCone, - AssetHint assetHint, + AssetPath assetPath, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -253,7 +253,7 @@ public async Task FindAsync( ChecksumCollection.Find(solution.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); - if (assetHint.TopLevelProjects) + if (assetPath.TopLevelProjects) { // Caller is trying to fetch the top level ProjectStateChecksums as well. Look for those without diving deeper. foreach (var (projectId, projectState) in solution.ProjectStates) @@ -277,24 +277,24 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - if (!assetHint.IsFullLookup_ForTestingPurposesOnly) + if (!assetPath.IsFullLookup_ForTestingPurposesOnly) { // Caller said they were only looking for solution level assets. no need to go any further. - if (assetHint.IsSolutionOnly) + if (assetPath.IsSolutionOnly) return; // Since we're not solution-only, we must have a project id being requested. Dive into that project alone // to search for the remaining checksums. - Contract.ThrowIfNull(assetHint.ProjectId); + Contract.ThrowIfNull(assetPath.ProjectId); Contract.ThrowIfTrue( - projectCone != null && !projectCone.Contains(assetHint.ProjectId), + projectCone != null && !projectCone.Contains(assetPath.ProjectId), "Requesting an asset outside of the cone explicitly being asked for!"); - var projectState = solution.GetProjectState(assetHint.ProjectId); + var projectState = solution.GetProjectState(assetPath.ProjectId); if (projectState != null && projectState.TryGetStateChecksums(out var projectStateChecksums)) { - await projectStateChecksums.FindAsync(projectState, assetHint.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await projectStateChecksums.FindAsync(projectState, assetPath.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } } else diff --git a/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs b/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs index 60dd23543517c..5482ce1b62a35 100644 --- a/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs +++ b/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.Remote.Testing; internal sealed class SimpleAssetSource(ISerializerService serializerService, IReadOnlyDictionary map) : IAssetSource { public ValueTask> GetAssetsAsync( - Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, ISerializerService deserializerService, CancellationToken cancellationToken) + Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, ISerializerService deserializerService, CancellationToken cancellationToken) { var results = new List(); diff --git a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs index 781546b3d6b65..01882d93eda60 100644 --- a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs +++ b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs @@ -20,21 +20,21 @@ internal abstract class AbstractAssetProvider /// /// return data of type T whose checksum is the given checksum /// - public abstract ValueTask GetAssetAsync(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken); + public abstract ValueTask GetAssetAsync(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken); public async Task CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { - var solutionCompilationChecksums = await GetAssetAsync(AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); - var solutionChecksums = await GetAssetAsync(AssetHint.SolutionOnly, solutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + var solutionCompilationChecksums = await GetAssetAsync(AssetPath.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); + var solutionChecksums = await GetAssetAsync(AssetPath.SolutionOnly, solutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); - var solutionAttributes = await GetAssetAsync(AssetHint.SolutionOnly, solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); - await GetAssetAsync(AssetHint.SolutionOnly, solutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); + var solutionAttributes = await GetAssetAsync(AssetPath.SolutionOnly, solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + await GetAssetAsync(AssetPath.SolutionOnly, solutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); using var _ = ArrayBuilder.GetInstance(solutionChecksums.Projects.Length, out var projects); foreach (var (projectChecksum, projectId) in solutionChecksums.Projects) projects.Add(await CreateProjectInfoAsync(projectId, projectChecksum, cancellationToken).ConfigureAwait(false)); - var analyzerReferences = await CreateCollectionAsync(AssetHint.SolutionOnly, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); + var analyzerReferences = await CreateCollectionAsync(AssetPath.SolutionOnly, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); return SolutionInfo.Create( solutionAttributes.Id, solutionAttributes.Version, solutionAttributes.FilePath, projects.ToImmutableAndClear(), analyzerReferences).WithTelemetryId(solutionAttributes.TelemetryId); @@ -42,19 +42,19 @@ public async Task CreateSolutionInfoAsync(Checksum solutionChecksu public async Task CreateProjectInfoAsync(ProjectId projectId, Checksum projectChecksum, CancellationToken cancellationToken) { - var projectChecksums = await GetAssetAsync(assetHint: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); + var projectChecksums = await GetAssetAsync(assetPath: projectId, projectChecksum, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectId == projectChecksums.ProjectId); - var attributes = await GetAssetAsync(assetHint: projectId, projectChecksums.Info, cancellationToken).ConfigureAwait(false); + var attributes = await GetAssetAsync(assetPath: projectId, projectChecksums.Info, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(RemoteSupportedLanguages.IsSupported(attributes.Language)); var compilationOptions = attributes.FixUpCompilationOptions( - await GetAssetAsync(assetHint: projectId, projectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false)); - var parseOptions = await GetAssetAsync(assetHint: projectId, projectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false); + await GetAssetAsync(assetPath: projectId, projectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false)); + var parseOptions = await GetAssetAsync(assetPath: projectId, projectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false); - var projectReferences = await CreateCollectionAsync(assetHint: projectId, projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false); - var metadataReferences = await CreateCollectionAsync(assetHint: projectId, projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false); - var analyzerReferences = await CreateCollectionAsync(assetHint: projectId, projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); + var projectReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false); + var metadataReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false); + var analyzerReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); var documentInfos = await CreateDocumentInfosAsync(projectChecksums.Documents).ConfigureAwait(false); var additionalDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AdditionalDocuments).ConfigureAwait(false); @@ -89,11 +89,11 @@ async Task> CreateDocumentInfosAsync(ChecksumsAndId public async Task CreateDocumentInfoAsync( DocumentId documentId, Checksum documentChecksum, CancellationToken cancellationToken) { - var documentSnapshot = await GetAssetAsync(assetHint: documentId, documentChecksum, cancellationToken).ConfigureAwait(false); + var documentSnapshot = await GetAssetAsync(assetPath: documentId, documentChecksum, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(documentId != documentSnapshot.DocumentId); - var attributes = await GetAssetAsync(assetHint: documentId, documentSnapshot.Info, cancellationToken).ConfigureAwait(false); - var serializableSourceText = await GetAssetAsync(assetHint: documentId, documentSnapshot.Text, cancellationToken).ConfigureAwait(false); + var attributes = await GetAssetAsync(assetPath: documentId, documentSnapshot.Info, cancellationToken).ConfigureAwait(false); + var serializableSourceText = await GetAssetAsync(assetPath: documentId, documentSnapshot.Text, cancellationToken).ConfigureAwait(false); var text = await serializableSourceText.GetTextAsync(cancellationToken).ConfigureAwait(false); var textLoader = TextLoader.From(TextAndVersion.Create(text, VersionStamp.Create(), attributes.FilePath)); @@ -103,14 +103,14 @@ public async Task CreateDocumentInfoAsync( } public async Task> CreateCollectionAsync( - AssetHint assetHint, ChecksumCollection checksums, CancellationToken cancellationToken) where T : class + AssetPath assetPath, ChecksumCollection checksums, CancellationToken cancellationToken) where T : class { using var _ = ArrayBuilder.GetInstance(checksums.Count, out var assets); foreach (var checksum in checksums) { cancellationToken.ThrowIfCancellationRequested(); - assets.Add(await GetAssetAsync(assetHint, checksum, cancellationToken).ConfigureAwait(false)); + assets.Add(await GetAssetAsync(assetPath, checksum, cancellationToken).ConfigureAwait(false)); } return assets.ToImmutableAndClear(); diff --git a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs index b57bf95fcb2b2..3c37c867d4b49 100644 --- a/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/ISolutionAssetProvider.cs @@ -22,9 +22,9 @@ internal interface ISolutionAssetProvider /// The writer to write the assets into. Implementations of this method must call on it (in the event of failure or success). Failing to do so will lead to hangs on /// the code that reads from the corresponding side of this. - /// Optional project and document ids to scope the search for checksums down to. This can + /// Optional project and document ids to scope the search for checksums down to. This can /// save substantially on performance by avoiding having to search the full solution tree to find matching items for /// a particular checksum. ValueTask WriteAssetsAsync( - PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken); + PipeWriter pipeWriter, Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs index 52a3f75670cc7..d4df3ec47e989 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs @@ -28,7 +28,7 @@ internal sealed class SolutionAssetProvider(SolutionServices services) : ISoluti public ValueTask WriteAssetsAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint assetHint, + AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken) { @@ -39,9 +39,9 @@ public ValueTask WriteAssetsAsync( // âš  DO NOT AWAIT INSIDE THE USING. The Dispose method that restores ExecutionContext flow must run on the // same thread where SuppressFlow was originally run. using var _ = FlowControlHelper.TrySuppressFlow(); - return WriteAssetsSuppressedFlowAsync(pipeWriter, solutionChecksum, assetHint, checksums, cancellationToken); + return WriteAssetsSuppressedFlowAsync(pipeWriter, solutionChecksum, assetPath, checksums, cancellationToken); - async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken) { // The responsibility is on us (as per the requirements of RemoteCallback.InvokeAsync) to Complete the // pipewriter. This will signal to streamjsonrpc that the writer passed into it is complete, which will @@ -49,7 +49,7 @@ async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum s Exception? exception = null; try { - await WriteAssetsWorkerAsync(pipeWriter, solutionChecksum, assetHint, checksums, cancellationToken).ConfigureAwait(false); + await WriteAssetsWorkerAsync(pipeWriter, solutionChecksum, assetPath, checksums, cancellationToken).ConfigureAwait(false); } catch (Exception ex) when ((exception = ex) == null) { @@ -65,7 +65,7 @@ async ValueTask WriteAssetsSuppressedFlowAsync(PipeWriter pipeWriter, Checksum s private async ValueTask WriteAssetsWorkerAsync( PipeWriter pipeWriter, Checksum solutionChecksum, - AssetHint assetHint, + AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken) { @@ -75,7 +75,7 @@ private async ValueTask WriteAssetsWorkerAsync( using var _ = Creator.CreateResultMap(out var resultMap); - await scope.AddAssetsAsync(assetHint, checksums, resultMap, cancellationToken).ConfigureAwait(false); + await scope.AddAssetsAsync(assetPath, checksums, resultMap, cancellationToken).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs index 3dcfa4cbca45b..e4b6ae9f169a8 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetStorage.Scope.cs @@ -45,7 +45,7 @@ public void Dispose() /// the storage. /// public async Task AddAssetsAsync( - AssetHint assetHint, + AssetPath assetPath, ReadOnlyMemory checksums, Dictionary assetMap, CancellationToken cancellationToken) @@ -58,14 +58,14 @@ public async Task AddAssetsAsync( var numberOfChecksumsToSearch = checksumsToFind.Count; Contract.ThrowIfTrue(checksumsToFind.Contains(Checksum.Null)); - await FindAssetsAsync(assetHint, checksumsToFind, assetMap, cancellationToken).ConfigureAwait(false); + await FindAssetsAsync(assetPath, checksumsToFind, assetMap, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(checksumsToFind.Count > 0); Contract.ThrowIfTrue(assetMap.Count != numberOfChecksumsToSearch); } private async Task FindAssetsAsync( - AssetHint assetHint, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) + AssetPath assetPath, HashSet remainingChecksumsToFind, Dictionary result, CancellationToken cancellationToken) { var solutionState = this.CompilationState; @@ -74,13 +74,13 @@ private async Task FindAssetsAsync( // If we're not in a project cone, start the search at the top most state-checksum corresponding to the // entire solution. Contract.ThrowIfFalse(solutionState.TryGetStateChecksums(out var stateChecksums)); - await stateChecksums.FindAsync(solutionState, this.ProjectCone, assetHint, remainingChecksumsToFind, result, cancellationToken).ConfigureAwait(false); + await stateChecksums.FindAsync(solutionState, this.ProjectCone, assetPath, remainingChecksumsToFind, result, cancellationToken).ConfigureAwait(false); } else { // Otherwise, grab the top-most state checksum for this cone and search within that. Contract.ThrowIfFalse(solutionState.TryGetStateChecksums(this.ProjectCone.RootProjectId, out var stateChecksums)); - await stateChecksums.FindAsync(solutionState, this.ProjectCone, assetHint, remainingChecksumsToFind, result, cancellationToken).ConfigureAwait(false); + await stateChecksums.FindAsync(solutionState, this.ProjectCone, assetPath, remainingChecksumsToFind, result, cancellationToken).ConfigureAwait(false); } } @@ -100,7 +100,7 @@ public async ValueTask GetAssetAsync(Checksum checksum, CancellationToke using var checksumPool = Creator.CreateChecksumSet(checksum); using var _ = Creator.CreateResultMap(out var resultPool); - await scope.FindAssetsAsync(AssetHint.FullLookupForTesting, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); + await scope.FindAssetsAsync(AssetPath.FullLookupForTesting, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(resultPool.Count != 1); var (resultingChecksum, value) = resultPool.First(); diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index fd72dfbe20d76..3ecd0415ec421 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -31,7 +31,7 @@ internal sealed partial class AssetProvider(Checksum solutionChecksum, SolutionA private readonly IAssetSource _assetSource = assetSource; public override async ValueTask GetAssetAsync( - AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken) + AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksum == Checksum.Null); if (_assetCache.TryGetAsset(checksum, out var asset)) @@ -41,19 +41,19 @@ public override async ValueTask GetAssetAsync( checksums.Add(checksum); using var _2 = PooledDictionary.GetInstance(out var results); - await this.SynchronizeAssetsAsync(assetHint, checksums, results, cancellationToken).ConfigureAwait(false); + await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); return (T)results[checksum]; } public async ValueTask> GetAssetsAsync( - AssetHint assetHint, HashSet checksums, CancellationToken cancellationToken) + AssetPath assetPath, HashSet checksums, CancellationToken cancellationToken) { using var _ = PooledDictionary.GetInstance(out var results); // bulk synchronize checksums first var syncer = new ChecksumSynchronizer(this); - await syncer.SynchronizeAssetsAsync(assetHint, checksums, results, cancellationToken).ConfigureAwait(false); + await syncer.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); var result = new (Checksum checksum, T asset)[checksums.Count]; var index = 0; @@ -113,7 +113,7 @@ public async ValueTask SynchronizeProjectAssetsAsync(ProjectStateChecksums proje } public async ValueTask SynchronizeAssetsAsync( - AssetHint assetHint, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) + AssetPath assetPath, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksums.Contains(Checksum.Null)); if (checksums.Count == 0) @@ -165,7 +165,7 @@ public async ValueTask SynchronizeAssetsAsync( if (missingChecksumsCount > 0) { var missingChecksumsMemory = new ReadOnlyMemory(missingChecksums, 0, missingChecksumsCount); - var missingAssets = await RequestAssetsAsync(assetHint, missingChecksumsMemory, cancellationToken).ConfigureAwait(false); + var missingAssets = await RequestAssetsAsync(assetPath, missingChecksumsMemory, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(missingChecksumsMemory.Length != missingAssets.Length); @@ -193,7 +193,7 @@ void AddResult(Checksum checksum, object result) } private async ValueTask> RequestAssetsAsync( - AssetHint assetHint, ReadOnlyMemory checksums, CancellationToken cancellationToken) + AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken) { #if NETCOREAPP Contract.ThrowIfTrue(checksums.Span.Contains(Checksum.Null)); @@ -204,6 +204,6 @@ private async ValueTask> RequestAssetsAsync( if (checksums.Length == 0) return []; - return await _assetSource.GetAssetsAsync(_solutionChecksum, assetHint, checksums, _serializerService, cancellationToken).ConfigureAwait(false); + return await _assetSource.GetAssetsAsync(_solutionChecksum, assetPath, checksums, _serializerService, cancellationToken).ConfigureAwait(false); } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index 03c4b84705049..c43f62b20d2f6 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -21,14 +21,14 @@ private readonly struct ChecksumSynchronizer(AssetProvider assetProvider) private readonly AssetProvider _assetProvider = assetProvider; public async ValueTask SynchronizeAssetsAsync( - AssetHint assetHint, + AssetPath assetPath, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) { using (await s_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) { - await _assetProvider.SynchronizeAssetsAsync(assetHint, checksums, results, cancellationToken).ConfigureAwait(false); + await _assetProvider.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); } } @@ -41,23 +41,23 @@ public async ValueTask SynchronizeSolutionAssetsAsync(Checksum solutionChecksum, { // first, get top level solution state for the given solution checksum var compilationStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, solutionChecksum, cancellationToken).ConfigureAwait(false); using var _2 = PooledHashSet.GetInstance(out var checksums); // second, get direct children of the solution compilation state. compilationStateChecksums.AddAllTo(checksums); - await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); + await _assetProvider.SynchronizeAssetsAsync(assetPath: AssetPath.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); // third, get direct children of the solution state. stateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, compilationStateChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, compilationStateChecksums.SolutionState, cancellationToken).ConfigureAwait(false); // Ask for solutions and top-level projects as the solution checksums will contain the checksums for // the project states and we want to get that all in one batch. checksums.Clear(); stateChecksums.AddAllTo(checksums); - await _assetProvider.SynchronizeAssetsAsync(assetHint: AssetHint.SolutionAndTopLevelProjectsOnly, checksums, checksumToObjects, cancellationToken).ConfigureAwait(false); + await _assetProvider.SynchronizeAssetsAsync(assetPath: AssetPath.SolutionAndTopLevelProjectsOnly, checksums, checksumToObjects, cancellationToken).ConfigureAwait(false); } // fourth, get all projects and documents in the solution @@ -93,7 +93,7 @@ private async ValueTask SynchronizeProjectAssets_NoLockAsync(ProjectStateChecksu // First synchronize all the top-level info about this project. await _assetProvider.SynchronizeAssetsAsync( - assetHint: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); + assetPath: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); checksums.Clear(); @@ -103,7 +103,7 @@ await _assetProvider.SynchronizeAssetsAsync( await CollectChecksumChildrenAsync(this, projectChecksum.AnalyzerConfigDocuments.Checksums).ConfigureAwait(false); await _assetProvider.SynchronizeAssetsAsync( - assetHint: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); + assetPath: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); async ValueTask CollectChecksumChildrenAsync(ChecksumSynchronizer @this, ChecksumCollection collection) { @@ -112,7 +112,7 @@ async ValueTask CollectChecksumChildrenAsync(ChecksumSynchronizer @this, Checksu // These GetAssetAsync calls should be fast since they were just retrieved above. There's a small // chance the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. var checksumObject = await @this._assetProvider.GetAssetAsync( - assetHint: projectChecksum.ProjectId, checksum, cancellationToken).ConfigureAwait(false); + assetPath: projectChecksum.ProjectId, checksum, cancellationToken).ConfigureAwait(false); checksums.Add(checksumObject.Info); checksums.Add(checksumObject.Text); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs index 840e043daa46a..d7fcab7984e13 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs @@ -16,5 +16,5 @@ namespace Microsoft.CodeAnalysis.Remote; internal interface IAssetSource { ValueTask> GetAssetsAsync( - Checksum solutionChecksum, AssetHint assetHint, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); + Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index 44b76316777d4..f4a6468b84379 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -36,12 +36,12 @@ private readonly struct SolutionCreator(HostServices hostServices, AssetProvider public async Task IsIncrementalUpdateAsync(Checksum newSolutionChecksum, CancellationToken cancellationToken) { var newSolutionCompilationChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); var newSolutionChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); var newSolutionInfo = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); // if either solution id or file path changed, then we consider it as new solution return _baseSolution.Id == newSolutionInfo.Id && _baseSolution.FilePath == newSolutionInfo.FilePath; @@ -58,9 +58,9 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca solution = solution.WithoutFrozenSourceGeneratedDocuments(); var newSolutionCompilationChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionChecksum, cancellationToken).ConfigureAwait(false); var newSolutionChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionCompilationChecksums.SolutionState, cancellationToken).ConfigureAwait(false); var oldSolutionCompilationChecksums = await solution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); var oldSolutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); @@ -68,7 +68,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca if (oldSolutionChecksums.Attributes != newSolutionChecksums.Attributes) { var newSolutionInfo = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionChecksums.Attributes, cancellationToken).ConfigureAwait(false); // if either id or file path has changed, then this is not update Contract.ThrowIfFalse(solution.Id == newSolutionInfo.Id && solution.FilePath == newSolutionInfo.FilePath); @@ -83,7 +83,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca if (oldSolutionChecksums.AnalyzerReferences.Checksum != newSolutionChecksums.AnalyzerReferences.Checksum) { solution = solution.WithAnalyzerReferences(await _assetProvider.CreateCollectionAsync( - assetHint: AssetHint.SolutionOnly, newSolutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); + assetPath: AssetPath.SolutionOnly, newSolutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); } if (newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.HasValue && @@ -96,12 +96,12 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca for (var i = 0; i < count; i++) { var identity = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.Value[i], cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentIdentities.Value[i], cancellationToken).ConfigureAwait(false); var documentStateChecksums = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Checksums[i], cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Checksums[i], cancellationToken).ConfigureAwait(false); - var serializableSourceText = await _assetProvider.GetAssetAsync(assetHint: newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Ids[i], documentStateChecksums.Text, cancellationToken).ConfigureAwait(false); + var serializableSourceText = await _assetProvider.GetAssetAsync(assetPath: newSolutionCompilationChecksums.FrozenSourceGeneratedDocuments.Value.Ids[i], documentStateChecksums.Text, cancellationToken).ConfigureAwait(false); var generationDateTime = newSolutionCompilationChecksums.FrozenSourceGeneratedDocumentGenerationDateTimes[i]; var text = await serializableSourceText.GetTextAsync(cancellationToken).ConfigureAwait(false); @@ -115,7 +115,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap) { var newVersions = await _assetProvider.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, newSolutionCompilationChecksums.SourceGeneratorExecutionVersionMap, cancellationToken).ConfigureAwait(false); // The execution version map will be for the entire solution on the host side. However, we may // only be syncing over a partial cone. In that case, filter down the version map we apply to @@ -219,7 +219,7 @@ private async Task UpdateProjectsAsync( newChecksumsToSync.AddRange(newProjectIdToChecksum.Values); var newProjectStateChecksums = await _assetProvider.GetAssetsAsync( - assetHint: AssetHint.SolutionAndTopLevelProjectsOnly, newChecksumsToSync, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.SolutionAndTopLevelProjectsOnly, newChecksumsToSync, cancellationToken).ConfigureAwait(false); foreach (var (checksum, newProjectStateChecksum) in newProjectStateChecksums) { @@ -310,35 +310,35 @@ private async Task UpdateProjectAsync(Project project, ProjectStateChe project = project.WithCompilationOptions( project.State.ProjectInfo.Attributes.FixUpCompilationOptions( await _assetProvider.GetAssetAsync( - assetHint: project.Id, newProjectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false))); + assetPath: project.Id, newProjectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false))); } // changed parse options if (oldProjectChecksums.ParseOptions != newProjectChecksums.ParseOptions) { project = project.WithParseOptions(await _assetProvider.GetAssetAsync( - assetHint: project.Id, newProjectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false)); + assetPath: project.Id, newProjectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false)); } // changed project references if (oldProjectChecksums.ProjectReferences.Checksum != newProjectChecksums.ProjectReferences.Checksum) { project = project.WithProjectReferences(await _assetProvider.CreateCollectionAsync( - assetHint: project.Id, newProjectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false)); + assetPath: project.Id, newProjectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false)); } // changed metadata references if (oldProjectChecksums.MetadataReferences.Checksum != newProjectChecksums.MetadataReferences.Checksum) { project = project.WithMetadataReferences(await _assetProvider.CreateCollectionAsync( - assetHint: project.Id, newProjectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false)); + assetPath: project.Id, newProjectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false)); } // changed analyzer references if (oldProjectChecksums.AnalyzerReferences.Checksum != newProjectChecksums.AnalyzerReferences.Checksum) { project = project.WithAnalyzerReferences(await _assetProvider.CreateCollectionAsync( - assetHint: project.Id, newProjectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); + assetPath: project.Id, newProjectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); } // changed analyzer references @@ -389,7 +389,7 @@ await _assetProvider.GetAssetAsync( private async Task UpdateProjectInfoAsync(Project project, Checksum infoChecksum, CancellationToken cancellationToken) { var newProjectAttributes = await _assetProvider.GetAssetAsync( - assetHint: project.Id, infoChecksum, cancellationToken).ConfigureAwait(false); + assetPath: project.Id, infoChecksum, cancellationToken).ConfigureAwait(false); // there is no API to change these once project is created Contract.ThrowIfFalse(project.State.ProjectInfo.Attributes.Id == newProjectAttributes.Id); @@ -504,7 +504,7 @@ private async Task UpdateDocumentsAsync( newChecksumsToSync.AddRange(newDocumentIdToChecksum.Values); var documentStateChecksums = await _assetProvider.GetAssetsAsync( - assetHint: project.Id, newChecksumsToSync, cancellationToken).ConfigureAwait(false); + assetPath: project.Id, newChecksumsToSync, cancellationToken).ConfigureAwait(false); foreach (var (checksum, documentStateChecksum) in documentStateChecksums) { @@ -599,7 +599,7 @@ private async Task UpdateDocumentAsync(TextDocument document, DocumentS if (oldDocumentChecksums.Text != newDocumentChecksums.Text) { var serializableSourceText = await _assetProvider.GetAssetAsync( - assetHint: document.Id, newDocumentChecksums.Text, cancellationToken).ConfigureAwait(false); + assetPath: document.Id, newDocumentChecksums.Text, cancellationToken).ConfigureAwait(false); var sourceText = await serializableSourceText.GetTextAsync(cancellationToken).ConfigureAwait(false); document = document.Kind switch @@ -617,7 +617,7 @@ private async Task UpdateDocumentAsync(TextDocument document, DocumentS private async Task UpdateDocumentInfoAsync(TextDocument document, Checksum infoChecksum, CancellationToken cancellationToken) { var newDocumentInfo = await _assetProvider.GetAssetAsync( - assetHint: document.Id, infoChecksum, cancellationToken).ConfigureAwait(false); + assetPath: document.Id, infoChecksum, cancellationToken).ConfigureAwait(false); // there is no api to change these once document is created Contract.ThrowIfFalse(document.State.Attributes.Id == newDocumentInfo.Id); diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs index 4917033beae78..d81767fbddda4 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs @@ -18,7 +18,7 @@ internal sealed class SolutionAssetSource(ServiceBrokerClient client) : IAssetSo public async ValueTask> GetAssetsAsync( Checksum solutionChecksum, - AssetHint assetHint, + AssetPath assetPath, ReadOnlyMemory checksums, ISerializerService serializerService, CancellationToken cancellationToken) @@ -30,7 +30,7 @@ public async ValueTask> GetAssetsAsync( _client, SolutionAssetProvider.ServiceDescriptor, (callback, cancellationToken) => callback.InvokeAsync( - (proxy, pipeWriter, cancellationToken) => proxy.WriteAssetsAsync(pipeWriter, solutionChecksum, assetHint, checksums, cancellationToken), + (proxy, pipeWriter, cancellationToken) => proxy.WriteAssetsAsync(pipeWriter, solutionChecksum, assetPath, checksums, cancellationToken), (pipeReader, cancellationToken) => RemoteHostAssetSerialization.ReadDataAsync(pipeReader, solutionChecksum, checksums.Length, serializerService, cancellationToken), cancellationToken), cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index b243d02066513..f9a05245d6529 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -104,7 +104,7 @@ async Task>> GetAssetFromAssetServiceAsync(I foreach (var checksum in checksums) { items.Add(new KeyValuePair(checksum, await assetService.GetAssetAsync( - AssetHint.FullLookupForTesting, checksum, CancellationToken.None).ConfigureAwait(false))); + AssetPath.FullLookupForTesting, checksum, CancellationToken.None).ConfigureAwait(false))); } return items; @@ -115,9 +115,9 @@ async Task> GetAllChildrenChecksumsAsync(Checksum solutionChec var set = new HashSet(); var solutionCompilationChecksums = await assetService.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, solutionChecksum, CancellationToken.None).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, solutionChecksum, CancellationToken.None).ConfigureAwait(false); var solutionChecksums = await assetService.GetAssetAsync( - assetHint: AssetHint.SolutionOnly, solutionCompilationChecksums.SolutionState, CancellationToken.None).ConfigureAwait(false); + assetPath: AssetPath.SolutionOnly, solutionCompilationChecksums.SolutionState, CancellationToken.None).ConfigureAwait(false); solutionCompilationChecksums.AddAllTo(set); solutionChecksums.AddAllTo(set); @@ -125,7 +125,7 @@ async Task> GetAllChildrenChecksumsAsync(Checksum solutionChec foreach (var (projectChecksum, projectId) in solutionChecksums.Projects) { var projectChecksums = await assetService.GetAssetAsync( - assetHint: projectId, projectChecksum, CancellationToken.None).ConfigureAwait(false); + assetPath: projectId, projectChecksum, CancellationToken.None).ConfigureAwait(false); projectChecksums.AddAllTo(set); await AddDocumentsAsync(projectId, projectChecksums.Documents, set).ConfigureAwait(false); @@ -141,7 +141,7 @@ async Task AddDocumentsAsync(ProjectId projectId, ChecksumsAndIds do foreach (var (documentChecksum, documentId) in documents) { var documentChecksums = await assetService.GetAssetAsync( - assetHint: documentId, documentChecksum, CancellationToken.None).ConfigureAwait(false); + assetPath: documentId, documentChecksum, CancellationToken.None).ConfigureAwait(false); AddAllTo(documentChecksums, checksums); } } @@ -197,17 +197,17 @@ public static async Task AppendAssetMapAsync( if (projectId == null) { var compilationChecksums = await solution.CompilationState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetPath.FullLookupForTesting, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var frozenSourceGeneratedDocumentState in solution.CompilationState.FrozenSourceGeneratedDocumentStates?.States.Values ?? []) { var documentChecksums = await frozenSourceGeneratedDocumentState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone: null, AssetPath.FullLookupForTesting, Flatten(documentChecksums), map, cancellationToken).ConfigureAwait(false); } var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(solutionChecksums.ProjectCone != null); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, AssetHint.FullLookupForTesting, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone: null, AssetPath.FullLookupForTesting, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); foreach (var project in solution.Projects) await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); @@ -215,11 +215,11 @@ public static async Task AppendAssetMapAsync( else { var (compilationChecksums, projectCone) = await solution.CompilationState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone, assetHint: projectId, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone, assetPath: projectId, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectCone.Equals(solutionChecksums.ProjectCone)); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetHint: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetPath: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); var project = solution.GetRequiredProject(projectId); await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); From 4b7c554b47186eba5a9912808215e4f72f3043b3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 16:25:42 -0700 Subject: [PATCH 26/40] Use enum --- .../Portable/Workspace/Solution/AssetHint.cs | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index db0c8f0c37e02..3bdb23f4be97d 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -17,39 +17,46 @@ internal readonly struct AssetPath /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static readonly AssetPath SolutionOnly = default; + public static readonly AssetPath SolutionOnly = new(kind: AssetPathKind.Solution); /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(projectId: null, documentId: null, topLevelProjects: true, isFullLookup_ForTestingPurposesOnly: false); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.SolutionAndTopLevelProjects); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetPath FullLookupForTesting = new(projectId: null, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: true); + public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.FullLookupForTests); [DataMember(Order = 0)] - public readonly ProjectId? ProjectId; + private readonly AssetPathKind _kind; [DataMember(Order = 1)] + public readonly ProjectId? ProjectId; + [DataMember(Order = 2)] public readonly DocumentId? DocumentId; - [DataMember(Order = 3)] - public readonly bool TopLevelProjects; - [DataMember(Order = 4)] - public readonly bool IsFullLookup_ForTestingPurposesOnly; - public bool IsSolutionOnly => !IsFullLookup_ForTestingPurposesOnly && ProjectId is null; + public bool TopLevelProjects => _kind == AssetPathKind.SolutionAndTopLevelProjects; + public bool IsFullLookup_ForTestingPurposesOnly => _kind == AssetPathKind.FullLookupForTests; + public bool IsSolutionOnly => _kind == AssetPathKind.Solution; - private AssetPath(ProjectId? projectId, DocumentId? documentId, bool topLevelProjects, bool isFullLookup_ForTestingPurposesOnly) + private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? documentId = null) { + _kind = kind; ProjectId = projectId; DocumentId = documentId; - TopLevelProjects = topLevelProjects; - IsFullLookup_ForTestingPurposesOnly = isFullLookup_ForTestingPurposesOnly; } - public static implicit operator AssetPath(ProjectId projectId) => new(projectId, documentId: null, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); - public static implicit operator AssetPath(DocumentId documentId) => new(documentId.ProjectId, documentId, topLevelProjects: false, isFullLookup_ForTestingPurposesOnly: false); + public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.ProjectOrDocument, projectId, documentId: null); + public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.ProjectOrDocument, documentId.ProjectId, documentId); + + private enum AssetPathKind + { + Solution = 0, + SolutionAndTopLevelProjects = 1, + FullLookupForTests = 2, + ProjectOrDocument = 3, + } } From dfe6eecc5549388c928dcc86420d7436c7c175c6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 6 Apr 2024 17:30:46 -0700 Subject: [PATCH 27/40] revert --- src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs b/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs index 478c0fe6e76f2..1d45eb4e6ab7b 100644 --- a/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs +++ b/src/Workspaces/Core/Portable/Classification/ClassifierHelper.cs @@ -277,7 +277,7 @@ public static void MergeParts Date: Sun, 7 Apr 2024 08:00:54 -0700 Subject: [PATCH 28/40] Slight change to AssetPath --- .../Portable/Workspace/Solution/AssetHint.cs | 28 ++-- .../Workspace/Solution/StateChecksums.cs | 149 ++++++++++-------- 2 files changed, 96 insertions(+), 81 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 3bdb23f4be97d..e091595257404 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Runtime.Serialization; namespace Microsoft.CodeAnalysis.Serialization; @@ -23,13 +24,13 @@ internal readonly struct AssetPath /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.SolutionAndTopLevelProjects); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.FullLookupForTests); + public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents); [DataMember(Order = 0)] private readonly AssetPathKind _kind; @@ -38,10 +39,6 @@ internal readonly struct AssetPath [DataMember(Order = 2)] public readonly DocumentId? DocumentId; - public bool TopLevelProjects => _kind == AssetPathKind.SolutionAndTopLevelProjects; - public bool IsFullLookup_ForTestingPurposesOnly => _kind == AssetPathKind.FullLookupForTests; - public bool IsSolutionOnly => _kind == AssetPathKind.Solution; - private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? documentId = null) { _kind = kind; @@ -49,14 +46,21 @@ private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? d DocumentId = documentId; } - public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.ProjectOrDocument, projectId, documentId: null); - public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.ProjectOrDocument, documentId.ProjectId, documentId); + public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution; + public bool IncludeTopLevelProjects => (_kind & AssetPathKind.TopLevelProjects) == AssetPathKind.TopLevelProjects; + public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects; + public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents; + + + public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Projects, projectId, documentId: null); + public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.Documents, documentId.ProjectId, documentId); + [Flags] private enum AssetPathKind { - Solution = 0, - SolutionAndTopLevelProjects = 1, - FullLookupForTests = 2, - ProjectOrDocument = 3, + Solution = 1, + TopLevelProjects = 1 >> 1, + Projects = 1 >> 2, + Documents = 1 >> 3, } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index 5679153035ab4..b322780a1695c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -244,16 +244,21 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - // verify input - if (searchingChecksumsLeft.Remove(Checksum)) - result[Checksum] = this; + if (assetPath.IncludeSolution) + { + if (searchingChecksumsLeft.Remove(Checksum)) + result[Checksum] = this; - if (searchingChecksumsLeft.Remove(Attributes)) - result[Attributes] = solution.SolutionAttributes; + if (searchingChecksumsLeft.Remove(Attributes)) + result[Attributes] = solution.SolutionAttributes; - ChecksumCollection.Find(solution.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); + ChecksumCollection.Find(solution.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); + + if (searchingChecksumsLeft.Count == 0) + return; + } - if (assetPath.TopLevelProjects) + if (assetPath.IncludeTopLevelProjects) { // Caller is trying to fetch the top level ProjectStateChecksums as well. Look for those without diving deeper. foreach (var (projectId, projectState) in solution.ProjectStates) @@ -272,48 +277,46 @@ public async Task FindAsync( result[projectStateChecksums.Checksum] = projectStateChecksums; } } - } - - if (searchingChecksumsLeft.Count == 0) - return; - if (!assetPath.IsFullLookup_ForTestingPurposesOnly) - { - // Caller said they were only looking for solution level assets. no need to go any further. - if (assetPath.IsSolutionOnly) + if (searchingChecksumsLeft.Count == 0) return; + } - // Since we're not solution-only, we must have a project id being requested. Dive into that project alone - // to search for the remaining checksums. - Contract.ThrowIfNull(assetPath.ProjectId); - Contract.ThrowIfTrue( - projectCone != null && !projectCone.Contains(assetPath.ProjectId), - "Requesting an asset outside of the cone explicitly being asked for!"); - - var projectState = solution.GetProjectState(assetPath.ProjectId); - if (projectState != null && - projectState.TryGetStateChecksums(out var projectStateChecksums)) + if (assetPath.IncludeProjects || assetPath.IncludeDocuments) + { + if (assetPath.ProjectId is not null) { - await projectStateChecksums.FindAsync(projectState, assetPath.DocumentId, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + // Dive into this project to search for the remaining checksums. + Contract.ThrowIfTrue( + projectCone != null && !projectCone.Contains(assetPath.ProjectId), + "Requesting an asset outside of the cone explicitly being asked for!"); + + var projectState = solution.GetProjectState(assetPath.ProjectId); + if (projectState != null && + projectState.TryGetStateChecksums(out var projectStateChecksums)) + { + await projectStateChecksums.FindAsync(projectState, assetPath, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + } } - } - else - { - // Full search, used for test purposes. - foreach (var (projectId, projectState) in solution.ProjectStates) + else { - if (searchingChecksumsLeft.Count == 0) - break; - - // If we're syncing a project cone, no point at all at looking at child projects of the solution that - // are not in that cone. - if (projectCone != null && !projectCone.Contains(projectId)) - continue; + // Full search, used for test purposes. + foreach (var (projectId, projectState) in solution.ProjectStates) + { + if (searchingChecksumsLeft.Count == 0) + break; + + // If we're syncing a project cone, no point at all at looking at child projects of the solution that + // are not in that cone. + if (projectCone != null && !projectCone.Contains(projectId)) + continue; + + // It's possible not all all our projects have checksums. Specifically, we may have only been asked to + // compute the checksum tree for a subset of projects that were all that a feature needed. + if (projectState.TryGetStateChecksums(out var projectStateChecksums)) + await projectStateChecksums.FindAsync(projectState, assetPath, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + } - // It's possible not all all our projects have checksums. Specifically, we may have only been asked to - // compute the checksum tree for a subset of projects that were all that a feature needed. - if (projectState.TryGetStateChecksums(out var projectStateChecksums)) - await projectStateChecksums.FindAsync(projectState, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } } } @@ -418,7 +421,7 @@ public static ProjectStateChecksums Deserialize(ObjectReader reader) public async Task FindAsync( ProjectState state, - DocumentId? hintDocument, + AssetPath assetPath, HashSet searchingChecksumsLeft, Dictionary result, CancellationToken cancellationToken) @@ -431,40 +434,48 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - if (searchingChecksumsLeft.Remove(Checksum)) + if (assetPath.IncludeProjects) { - result[Checksum] = this; - } + if (searchingChecksumsLeft.Remove(Checksum)) + { + result[Checksum] = this; + } - // It's normal for callers to just want to sync a single ProjectStateChecksum. So quickly check this, without - // doing all the expensive linear work below if we can bail out early here. - if (searchingChecksumsLeft.Count == 0) - return; + // It's normal for callers to just want to sync a single ProjectStateChecksum. So quickly check this, without + // doing all the expensive linear work below if we can bail out early here. + if (searchingChecksumsLeft.Count == 0) + return; - if (searchingChecksumsLeft.Remove(Info)) - { - result[Info] = state.ProjectInfo.Attributes; - } + if (searchingChecksumsLeft.Remove(Info)) + { + result[Info] = state.ProjectInfo.Attributes; + } - if (searchingChecksumsLeft.Remove(CompilationOptions)) - { - Contract.ThrowIfNull(state.CompilationOptions, "We should not be trying to serialize a project with no compilation options; RemoteSupportedLanguages.IsSupported should have filtered it out."); - result[CompilationOptions] = state.CompilationOptions; - } + if (searchingChecksumsLeft.Remove(CompilationOptions)) + { + Contract.ThrowIfNull(state.CompilationOptions, "We should not be trying to serialize a project with no compilation options; RemoteSupportedLanguages.IsSupported should have filtered it out."); + result[CompilationOptions] = state.CompilationOptions; + } - if (searchingChecksumsLeft.Remove(ParseOptions)) - { - Contract.ThrowIfNull(state.ParseOptions, "We should not be trying to serialize a project with no compilation options; RemoteSupportedLanguages.IsSupported should have filtered it out."); - result[ParseOptions] = state.ParseOptions; + if (searchingChecksumsLeft.Remove(ParseOptions)) + { + Contract.ThrowIfNull(state.ParseOptions, "We should not be trying to serialize a project with no compilation options; RemoteSupportedLanguages.IsSupported should have filtered it out."); + result[ParseOptions] = state.ParseOptions; + } + + ChecksumCollection.Find(state.ProjectReferences, ProjectReferences, searchingChecksumsLeft, result, cancellationToken); + ChecksumCollection.Find(state.MetadataReferences, MetadataReferences, searchingChecksumsLeft, result, cancellationToken); + ChecksumCollection.Find(state.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); } - ChecksumCollection.Find(state.ProjectReferences, ProjectReferences, searchingChecksumsLeft, result, cancellationToken); - ChecksumCollection.Find(state.MetadataReferences, MetadataReferences, searchingChecksumsLeft, result, cancellationToken); - ChecksumCollection.Find(state.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); + if (assetPath.IncludeDocuments) + { + var hintDocument = assetPath.DocumentId; - await ChecksumCollection.FindAsync(state.DocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); - await ChecksumCollection.FindAsync(state.AdditionalDocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); - await ChecksumCollection.FindAsync(state.AnalyzerConfigDocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await ChecksumCollection.FindAsync(state.DocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await ChecksumCollection.FindAsync(state.AdditionalDocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + await ChecksumCollection.FindAsync(state.AnalyzerConfigDocumentStates, hintDocument, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + } } } From f30f0aee40cf57c05629a6083f038a7052232b24 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 09:37:25 -0700 Subject: [PATCH 29/40] Feedback --- .../Portable/Workspace/Solution/AssetHint.cs | 33 +++++++++--- .../Workspace/Solution/StateChecksums.cs | 54 +++++++++---------- 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index e091595257404..db979e0c821e8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -4,6 +4,7 @@ using System; using System.Runtime.Serialization; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Serialization; @@ -18,19 +19,19 @@ internal readonly struct AssetPath /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static readonly AssetPath SolutionOnly = new(kind: AssetPathKind.Solution); + public static readonly AssetPath SolutionOnly = new(kind: AssetPathKind.Solution, forTesting: false); /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects, forTesting: false); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents); + public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents, forTesting: true); [DataMember(Order = 0)] private readonly AssetPathKind _kind; @@ -39,11 +40,30 @@ internal readonly struct AssetPath [DataMember(Order = 2)] public readonly DocumentId? DocumentId; - private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? documentId = null) + private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = null, DocumentId? documentId = null) { _kind = kind; ProjectId = projectId; DocumentId = documentId; + + if (forTesting) + { + // Only tests are allowed to search everything. And, in that case, they don't pass any doc/project along. + Contract.ThrowIfTrue(projectId != null); + Contract.ThrowIfTrue(documentId != null); + } + else + { + // Otherwise, if not in testing, if we say we're searching projects or documents, we have to supply those IDs as well. + if (IncludeProjects) + { + Contract.ThrowIfNull(projectId); + } + else if (IncludeDocuments) + { + Contract.ThrowIfNull(documentId); + } + } } public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution; @@ -51,9 +71,8 @@ private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? d public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects; public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents; - - public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Projects, projectId, documentId: null); - public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.Documents, documentId.ProjectId, documentId); + public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Projects, forTesting: false, projectId, documentId: null); + public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); [Flags] private enum AssetPathKind diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs index b322780a1695c..641eeba2963d1 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs @@ -120,33 +120,35 @@ public async Task FindAsync( if (searchingChecksumsLeft.Count == 0) return; - // verify input - if (searchingChecksumsLeft.Remove(this.Checksum)) - result[this.Checksum] = this; + if (assetPath.IncludeSolution) + { + if (searchingChecksumsLeft.Remove(this.Checksum)) + result[this.Checksum] = this; - if (searchingChecksumsLeft.Remove(this.SourceGeneratorExecutionVersionMap)) - result[this.SourceGeneratorExecutionVersionMap] = compilationState.SourceGeneratorExecutionVersionMap; + if (searchingChecksumsLeft.Remove(this.SourceGeneratorExecutionVersionMap)) + result[this.SourceGeneratorExecutionVersionMap] = compilationState.SourceGeneratorExecutionVersionMap; - if (searchingChecksumsLeft.Count == 0) - return; + if (searchingChecksumsLeft.Count == 0) + return; - if (compilationState.FrozenSourceGeneratedDocumentStates != null) - { - Contract.ThrowIfFalse(FrozenSourceGeneratedDocumentIdentities.HasValue); + if (compilationState.FrozenSourceGeneratedDocumentStates != null) + { + Contract.ThrowIfFalse(FrozenSourceGeneratedDocumentIdentities.HasValue); - // This could either be the checksum for the text (which we'll use our regular helper for first)... - await ChecksumCollection.FindAsync(compilationState.FrozenSourceGeneratedDocumentStates, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); + // This could either be the checksum for the text (which we'll use our regular helper for first)... + await ChecksumCollection.FindAsync(compilationState.FrozenSourceGeneratedDocumentStates, hintDocument: null, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); - // ... or one of the identities. In this case, we'll use the fact that there's a 1:1 correspondence between the - // two collections we hold onto. - for (var i = 0; i < FrozenSourceGeneratedDocumentIdentities.Value.Count; i++) - { - var identityChecksum = FrozenSourceGeneratedDocumentIdentities.Value[0]; - if (searchingChecksumsLeft.Remove(identityChecksum)) + // ... or one of the identities. In this case, we'll use the fact that there's a 1:1 correspondence between the + // two collections we hold onto. + for (var i = 0; i < FrozenSourceGeneratedDocumentIdentities.Value.Count; i++) { - var id = FrozenSourceGeneratedDocuments!.Value.Ids[i]; - Contract.ThrowIfFalse(compilationState.FrozenSourceGeneratedDocumentStates.TryGetState(id, out var state)); - result[identityChecksum] = state.Identity; + var identityChecksum = FrozenSourceGeneratedDocumentIdentities.Value[0]; + if (searchingChecksumsLeft.Remove(identityChecksum)) + { + var id = FrozenSourceGeneratedDocuments!.Value.Ids[i]; + Contract.ThrowIfFalse(compilationState.FrozenSourceGeneratedDocumentStates.TryGetState(id, out var state)); + result[identityChecksum] = state.Identity; + } } } } @@ -253,9 +255,6 @@ public async Task FindAsync( result[Attributes] = solution.SolutionAttributes; ChecksumCollection.Find(solution.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken); - - if (searchingChecksumsLeft.Count == 0) - return; } if (assetPath.IncludeTopLevelProjects) @@ -277,11 +276,11 @@ public async Task FindAsync( result[projectStateChecksums.Checksum] = projectStateChecksums; } } - - if (searchingChecksumsLeft.Count == 0) - return; } + if (searchingChecksumsLeft.Count == 0) + return; + if (assetPath.IncludeProjects || assetPath.IncludeDocuments) { if (assetPath.ProjectId is not null) @@ -316,7 +315,6 @@ public async Task FindAsync( if (projectState.TryGetStateChecksums(out var projectStateChecksums)) await projectStateChecksums.FindAsync(projectState, assetPath, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false); } - } } } From 11e6ec2f631011f84b84a6811f28db5039ca6f63 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 09:44:18 -0700 Subject: [PATCH 30/40] shift left --- .../Core/Portable/Workspace/Solution/AssetHint.cs | 9 +++++---- src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index db979e0c821e8..93f560dc31e10 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -61,6 +61,7 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu } else if (IncludeDocuments) { + Contract.ThrowIfNull(projectId); Contract.ThrowIfNull(documentId); } } @@ -77,9 +78,9 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu [Flags] private enum AssetPathKind { - Solution = 1, - TopLevelProjects = 1 >> 1, - Projects = 1 >> 2, - Documents = 1 >> 3, + Solution = 1 << 0, + TopLevelProjects = 1 << 1, + Projects = 1 << 2, + Documents = 1 << 3, } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index f9a05245d6529..dc8a5d6a09553 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -236,7 +236,7 @@ private static async Task AppendAssetMapAsync(this Project project, Dictionary Date: Sun, 7 Apr 2024 09:53:13 -0700 Subject: [PATCH 31/40] IN progress --- .../Core/Portable/Workspace/Solution/AssetHint.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 93f560dc31e10..f7a175175f139 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -35,14 +35,19 @@ internal readonly struct AssetPath [DataMember(Order = 0)] private readonly AssetPathKind _kind; +#pragma warning disable IDE0052 // Remove unread private members [DataMember(Order = 1)] - public readonly ProjectId? ProjectId; + private readonly bool _forTesting; +#pragma warning restore IDE0052 // Remove unread private members [DataMember(Order = 2)] + public readonly ProjectId? ProjectId; + [DataMember(Order = 3)] public readonly DocumentId? DocumentId; private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = null, DocumentId? documentId = null) { _kind = kind; + _forTesting = forTesting; ProjectId = projectId; DocumentId = documentId; @@ -72,7 +77,7 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects; public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents; - public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Projects, forTesting: false, projectId, documentId: null); + public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId, documentId: null); public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); [Flags] From 0e1ef0a1ce06c7ec22f22763e2637dd88f2654cf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:09:37 -0700 Subject: [PATCH 32/40] fixups --- .../Portable/Workspace/Solution/AssetHint.cs | 31 +++++++++++-------- .../ServiceHub/Host/ChecksumSynchronizer.cs | 16 +++++----- .../Host/RemoteWorkspace.SolutionCreator.cs | 2 +- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index f7a175175f139..ee2e0159fa350 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -19,19 +19,19 @@ internal readonly struct AssetPath /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static readonly AssetPath SolutionOnly = new(kind: AssetPathKind.Solution, forTesting: false); + public static readonly AssetPath SolutionOnly = new(AssetPathKind.Solution, forTesting: false); /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects, forTesting: false); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects, forTesting: false); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetPath FullLookupForTesting = new(kind: AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents, forTesting: true); + public static readonly AssetPath FullLookupForTesting = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents, forTesting: true); [DataMember(Order = 0)] private readonly AssetPathKind _kind; @@ -60,15 +60,8 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu else { // Otherwise, if not in testing, if we say we're searching projects or documents, we have to supply those IDs as well. - if (IncludeProjects) - { + if (IncludeProjects || IncludeDocuments) Contract.ThrowIfNull(projectId); - } - else if (IncludeDocuments) - { - Contract.ThrowIfNull(projectId); - Contract.ThrowIfNull(documentId); - } } } @@ -77,8 +70,20 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects; public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents; - public static implicit operator AssetPath(ProjectId projectId) => new(kind: AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId, documentId: null); - public static implicit operator AssetPath(DocumentId documentId) => new(kind: AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); + public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId, documentId: null); + + /// + /// Searches only for information about this document. + /// + public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); + + /// + /// Searches the requested project, and all documents underneath it. + /// + /// + /// + public static AssetPath ProjectAndDocuments(ProjectId projectId) + => new(AssetPathKind.Projects | AssetPathKind.Documents, forTesting: false, projectId); [Flags] private enum AssetPathKind diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index c43f62b20d2f6..8690801be9b38 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -93,26 +93,26 @@ private async ValueTask SynchronizeProjectAssets_NoLockAsync(ProjectStateChecksu // First synchronize all the top-level info about this project. await _assetProvider.SynchronizeAssetsAsync( - assetPath: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.ProjectAndDocuments(projectChecksum.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false); checksums.Clear(); // Then synchronize the info about all the documents within. - await CollectChecksumChildrenAsync(this, projectChecksum.Documents.Checksums).ConfigureAwait(false); - await CollectChecksumChildrenAsync(this, projectChecksum.AdditionalDocuments.Checksums).ConfigureAwait(false); - await CollectChecksumChildrenAsync(this, projectChecksum.AnalyzerConfigDocuments.Checksums).ConfigureAwait(false); + await CollectChecksumChildrenAsync(this, projectChecksum.Documents).ConfigureAwait(false); + await CollectChecksumChildrenAsync(this, projectChecksum.AdditionalDocuments).ConfigureAwait(false); + await CollectChecksumChildrenAsync(this, projectChecksum.AnalyzerConfigDocuments).ConfigureAwait(false); await _assetProvider.SynchronizeAssetsAsync( - assetPath: projectChecksum.ProjectId, checksums, results: null, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.ProjectAndDocuments(projectChecksum.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false); - async ValueTask CollectChecksumChildrenAsync(ChecksumSynchronizer @this, ChecksumCollection collection) + async ValueTask CollectChecksumChildrenAsync(ChecksumSynchronizer @this, ChecksumsAndIds collection) { - foreach (var checksum in collection) + foreach (var (checksum, documentId) in collection) { // These GetAssetAsync calls should be fast since they were just retrieved above. There's a small // chance the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. var checksumObject = await @this._assetProvider.GetAssetAsync( - assetPath: projectChecksum.ProjectId, checksum, cancellationToken).ConfigureAwait(false); + assetPath: documentId, checksum, cancellationToken).ConfigureAwait(false); checksums.Add(checksumObject.Info); checksums.Add(checksumObject.Text); } diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index f4a6468b84379..44ebedb736917 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -504,7 +504,7 @@ private async Task UpdateDocumentsAsync( newChecksumsToSync.AddRange(newDocumentIdToChecksum.Values); var documentStateChecksums = await _assetProvider.GetAssetsAsync( - assetPath: project.Id, newChecksumsToSync, cancellationToken).ConfigureAwait(false); + assetPath: AssetPath.ProjectAndDocuments(project.Id), newChecksumsToSync, cancellationToken).ConfigureAwait(false); foreach (var (checksum, documentStateChecksum) in documentStateChecksums) { From 241a51dd2343247fb3a3a855bac23e3b3f3f4edb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:14:10 -0700 Subject: [PATCH 33/40] Cleanup --- .../Core/Portable/Workspace/Solution/AssetHint.cs | 14 +++++++++++++- src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index ee2e0159fa350..e0021fc8a7b59 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -70,13 +70,25 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects; public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents; - public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId, documentId: null); + /// + /// Searches only for information about this project. + /// + public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Projects, forTesting: false, projectId, documentId: null); /// /// Searches only for information about this document. /// public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); + /// + /// Searches the requested project, and all documents underneath it. + /// + /// + /// + public static AssetPath SolutionAndProject(ProjectId projectId) + => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId); + + /// /// Searches the requested project, and all documents underneath it. /// diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index dc8a5d6a09553..f0c57e10cb18d 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -215,11 +215,11 @@ public static async Task AppendAssetMapAsync( else { var (compilationChecksums, projectCone) = await solution.CompilationState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone, assetPath: projectId, Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone, AssetPath.SolutionAndProject(projectId), Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectCone.Equals(solutionChecksums.ProjectCone)); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, assetPath: projectId, Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, AssetPath.SolutionAndProject(projectId), Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); var project = solution.GetRequiredProject(projectId); await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); From e2682007fa133700e5ae7458813139056864b82f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:15:36 -0700 Subject: [PATCH 34/40] Docs --- .../Core/Portable/Workspace/Solution/AssetHint.cs | 10 +++++----- src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index e0021fc8a7b59..5176981376a60 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -81,16 +81,16 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); /// - /// Searches the requested project, and all documents underneath it. + /// Searches the requested project, and all documents underneath it. Used only in tests. /// /// /// - public static AssetPath SolutionAndProject(ProjectId projectId) - => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: false, projectId); - + public static AssetPath SolutionAndProjectForTesting(ProjectId projectId) + => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: true, projectId); /// - /// Searches the requested project, and all documents underneath it. + /// Searches the requested project, and all documents underneath it. used during normal sync when bulk syncing a + /// project. /// /// /// diff --git a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs index f0c57e10cb18d..d12f4b38a7eeb 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/TestUtils.cs @@ -215,11 +215,11 @@ public static async Task AppendAssetMapAsync( else { var (compilationChecksums, projectCone) = await solution.CompilationState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); - await compilationChecksums.FindAsync(solution.CompilationState, projectCone, AssetPath.SolutionAndProject(projectId), Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); + await compilationChecksums.FindAsync(solution.CompilationState, projectCone, AssetPath.SolutionAndProjectForTesting(projectId), Flatten(compilationChecksums), map, cancellationToken).ConfigureAwait(false); var solutionChecksums = await solution.CompilationState.SolutionState.GetStateChecksumsAsync(projectId, cancellationToken).ConfigureAwait(false); Contract.ThrowIfFalse(projectCone.Equals(solutionChecksums.ProjectCone)); - await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, AssetPath.SolutionAndProject(projectId), Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); + await solutionChecksums.FindAsync(solution.CompilationState.SolutionState, projectCone, AssetPath.SolutionAndProjectForTesting(projectId), Flatten(solutionChecksums), map, cancellationToken).ConfigureAwait(false); var project = solution.GetRequiredProject(projectId); await project.AppendAssetMapAsync(map, cancellationToken).ConfigureAwait(false); From 6b1802d334f60f53304492e72f322c3bc5bea893 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:17:25 -0700 Subject: [PATCH 35/40] Docs --- .../Portable/Workspace/Solution/AssetHint.cs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 5176981376a60..43035f3b247cd 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -36,6 +36,10 @@ internal readonly struct AssetPath [DataMember(Order = 0)] private readonly AssetPathKind _kind; #pragma warning disable IDE0052 // Remove unread private members + /// + /// Indicates if this was a full-search done by a test utility. Useful when debugging tests to know what sort of + /// caller was doing the search. + /// [DataMember(Order = 1)] private readonly bool _forTesting; #pragma warning restore IDE0052 // Remove unread private members @@ -50,19 +54,6 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu _forTesting = forTesting; ProjectId = projectId; DocumentId = documentId; - - if (forTesting) - { - // Only tests are allowed to search everything. And, in that case, they don't pass any doc/project along. - Contract.ThrowIfTrue(projectId != null); - Contract.ThrowIfTrue(documentId != null); - } - else - { - // Otherwise, if not in testing, if we say we're searching projects or documents, we have to supply those IDs as well. - if (IncludeProjects || IncludeDocuments) - Contract.ThrowIfNull(projectId); - } } public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution; From d63de5af844a2581183867c8f0610b31036cadda Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:20:17 -0700 Subject: [PATCH 36/40] Add docs --- .../Portable/Workspace/Solution/AssetHint.cs | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 43035f3b247cd..648221f20f070 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -19,39 +19,30 @@ internal readonly struct AssetPath /// /// Instance that will only look up solution-level data when searching for checksums. /// - public static readonly AssetPath SolutionOnly = new(AssetPathKind.Solution, forTesting: false); + public static readonly AssetPath SolutionOnly = new(AssetPathKind.Solution); /// /// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for /// checksums. It will not descend into projects. /// - public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects, forTesting: false); + public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects); /// /// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum /// tree. Should not be used in normal release-mode product code. /// - public static readonly AssetPath FullLookupForTesting = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents, forTesting: true); + public static readonly AssetPath FullLookupForTesting = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents | AssetPathKind.Testing); [DataMember(Order = 0)] private readonly AssetPathKind _kind; -#pragma warning disable IDE0052 // Remove unread private members - /// - /// Indicates if this was a full-search done by a test utility. Useful when debugging tests to know what sort of - /// caller was doing the search. - /// [DataMember(Order = 1)] - private readonly bool _forTesting; -#pragma warning restore IDE0052 // Remove unread private members - [DataMember(Order = 2)] public readonly ProjectId? ProjectId; - [DataMember(Order = 3)] + [DataMember(Order = 2)] public readonly DocumentId? DocumentId; - private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = null, DocumentId? documentId = null) + private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? documentId = null) { _kind = kind; - _forTesting = forTesting; ProjectId = projectId; DocumentId = documentId; } @@ -64,12 +55,12 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu /// /// Searches only for information about this project. /// - public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Projects, forTesting: false, projectId, documentId: null); + public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Projects, projectId, documentId: null); /// /// Searches only for information about this document. /// - public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, forTesting: false, documentId.ProjectId, documentId); + public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, documentId.ProjectId, documentId); /// /// Searches the requested project, and all documents underneath it. Used only in tests. @@ -77,7 +68,7 @@ private AssetPath(AssetPathKind kind, bool forTesting, ProjectId? projectId = nu /// /// public static AssetPath SolutionAndProjectForTesting(ProjectId projectId) - => new(AssetPathKind.Solution | AssetPathKind.Projects, forTesting: true, projectId); + => new(AssetPathKind.Solution | AssetPathKind.Projects | AssetPathKind.Testing, projectId); /// /// Searches the requested project, and all documents underneath it. used during normal sync when bulk syncing a @@ -86,14 +77,36 @@ public static AssetPath SolutionAndProjectForTesting(ProjectId projectId) /// /// public static AssetPath ProjectAndDocuments(ProjectId projectId) - => new(AssetPathKind.Projects | AssetPathKind.Documents, forTesting: false, projectId); + => new(AssetPathKind.Projects | AssetPathKind.Documents, projectId); [Flags] private enum AssetPathKind { + /// + /// Search solution-level information. + /// Solution = 1 << 0, + + /// + /// Search projects, with descending into them. In effect, only finding direct ProjectStateChecksum children of + /// the solution. + /// TopLevelProjects = 1 << 1, + + /// + /// Search projects for results. + /// Projects = 1 << 2, + + /// + /// Search documents for results. + /// Documents = 1 << 3, + + /// + /// Indicates that this is a special search performed during testing. These searches are allowed to search + /// everything for expediency purposes. + /// + Testing = 1 << 4, } } From ab2ba2e95cf156489ce20f1e7b1f0060c393cfbf Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:36:50 -0700 Subject: [PATCH 37/40] Bulk sync --- .../Remote/SerializationValidator.cs | 21 +++++++++++----- .../Test.Next/Services/AssetProviderTests.cs | 2 +- .../Portable/Workspace/Solution/AssetHint.cs | 13 ++++++++-- .../Remote/Core/AbstractAssetProvider.cs | 24 +++++++++---------- .../Remote/ServiceHub/Host/AssetProvider.cs | 2 +- .../Host/RemoteWorkspace.SolutionCreator.cs | 8 +++---- 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs b/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs index 7851c17671124..7f6957f670a6a 100644 --- a/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs +++ b/src/VisualStudio/Core/Test.Next/Remote/SerializationValidator.cs @@ -11,6 +11,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Serialization; using Microsoft.VisualStudio.PlatformUI; using Roslyn.Test.Utilities; @@ -21,15 +22,23 @@ namespace Microsoft.CodeAnalysis.Remote.UnitTests { internal sealed class SerializationValidator { - private sealed class AssetProvider : AbstractAssetProvider + private sealed class AssetProvider(SerializationValidator validator) : AbstractAssetProvider { - private readonly SerializationValidator _validator; + public override async ValueTask GetAssetAsync(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken) + => await validator.GetValueAsync(checksum).ConfigureAwait(false); - public AssetProvider(SerializationValidator validator) - => _validator = validator; + public override async ValueTask> GetAssetsAsync(AssetPath assetPath, HashSet checksums, CancellationToken cancellationToken) + { + using var _ = ArrayBuilder<(Checksum checksum, T asset)>.GetInstance(out var result); - public override async ValueTask GetAssetAsync(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken) - => await _validator.GetValueAsync(checksum).ConfigureAwait(false); + foreach (var checksum in checksums) + { + var value = await GetAssetAsync(assetPath, checksum, cancellationToken).ConfigureAwait(false); + result.Add((checksum, value)); + } + + return result.ToImmutable(); + } } internal sealed class ChecksumObjectCollection : IEnumerable diff --git a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs index e06e64efbf1ee..6338cb8e0bdde 100644 --- a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs @@ -56,7 +56,7 @@ private static async Task TestAssetAsync(object data) var stored = await provider.GetAssetAsync(AssetPath.FullLookupForTesting, checksum, CancellationToken.None); Assert.Equal(data, stored); - var stored2 = await provider.GetAssetsAsync(AssetPath.FullLookupForTesting, [checksum], CancellationToken.None); + var stored2 = await provider.GetAssetsAsync(AssetPath.FullLookupForTesting, new HashSet { checksum }, CancellationToken.None); Assert.Equal(1, stored2.Length); Assert.Equal(checksum, stored2[0].Item1); diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs index 648221f20f070..24fbfecb4bc38 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs @@ -45,6 +45,15 @@ private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? d _kind = kind; ProjectId = projectId; DocumentId = documentId; + + // If this isn't a test lookup, and we're searching into projects or documents, then we must have at least a + // projectId to limit the search. If we don't, that risks very expensive searches where we look into *every* + // project in the solution for matches. + if ((kind & AssetPathKind.Testing) == 0) + { + if (IncludeProjects || IncludeDocuments) + Contract.ThrowIfNull(projectId); + } } public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution; @@ -88,8 +97,8 @@ private enum AssetPathKind Solution = 1 << 0, /// - /// Search projects, with descending into them. In effect, only finding direct ProjectStateChecksum children of - /// the solution. + /// Search projects, without descending into them. In effect, only finding direct ProjectStateChecksum children + /// of the solution. /// TopLevelProjects = 1 << 1, diff --git a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs index 01882d93eda60..5feb89710575b 100644 --- a/src/Workspaces/Remote/Core/AbstractAssetProvider.cs +++ b/src/Workspaces/Remote/Core/AbstractAssetProvider.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; @@ -21,6 +22,7 @@ internal abstract class AbstractAssetProvider /// return data of type T whose checksum is the given checksum /// public abstract ValueTask GetAssetAsync(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken); + public abstract ValueTask> GetAssetsAsync(AssetPath assetPath, HashSet checksums, CancellationToken cancellationToken); public async Task CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken) { @@ -34,7 +36,7 @@ public async Task CreateSolutionInfoAsync(Checksum solutionChecksu foreach (var (projectChecksum, projectId) in solutionChecksums.Projects) projects.Add(await CreateProjectInfoAsync(projectId, projectChecksum, cancellationToken).ConfigureAwait(false)); - var analyzerReferences = await CreateCollectionAsync(AssetPath.SolutionOnly, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); + var analyzerReferences = await GetAssetsAsync(AssetPath.SolutionOnly, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); return SolutionInfo.Create( solutionAttributes.Id, solutionAttributes.Version, solutionAttributes.FilePath, projects.ToImmutableAndClear(), analyzerReferences).WithTelemetryId(solutionAttributes.TelemetryId); @@ -52,9 +54,9 @@ public async Task CreateProjectInfoAsync(ProjectId projectId, Check await GetAssetAsync(assetPath: projectId, projectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false)); var parseOptions = await GetAssetAsync(assetPath: projectId, projectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false); - var projectReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false); - var metadataReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false); - var analyzerReferences = await CreateCollectionAsync(assetPath: projectId, projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); + var projectReferences = await GetAssetsAsync(assetPath: projectId, projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false); + var metadataReferences = await GetAssetsAsync(assetPath: projectId, projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false); + var analyzerReferences = await GetAssetsAsync(assetPath: projectId, projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false); var documentInfos = await CreateDocumentInfosAsync(projectChecksums.Documents).ConfigureAwait(false); var additionalDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AdditionalDocuments).ConfigureAwait(false); @@ -102,17 +104,13 @@ public async Task CreateDocumentInfoAsync( return new DocumentInfo(attributes, textLoader, documentServiceProvider: null); } - public async Task> CreateCollectionAsync( + public async Task> GetAssetsAsync( AssetPath assetPath, ChecksumCollection checksums, CancellationToken cancellationToken) where T : class { - using var _ = ArrayBuilder.GetInstance(checksums.Count, out var assets); + using var _ = PooledHashSet.GetInstance(out var checksumSet); + checksumSet.AddAll(checksums.Children); - foreach (var checksum in checksums) - { - cancellationToken.ThrowIfCancellationRequested(); - assets.Add(await GetAssetAsync(assetPath, checksum, cancellationToken).ConfigureAwait(false)); - } - - return assets.ToImmutableAndClear(); + var results = await this.GetAssetsAsync(assetPath, checksumSet, cancellationToken).ConfigureAwait(false); + return results.SelectAsArray(static t => t.asset); } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index 3ecd0415ec421..f65ad2cf20f08 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -46,7 +46,7 @@ public override async ValueTask GetAssetAsync( return (T)results[checksum]; } - public async ValueTask> GetAssetsAsync( + public override async ValueTask> GetAssetsAsync( AssetPath assetPath, HashSet checksums, CancellationToken cancellationToken) { using var _ = PooledDictionary.GetInstance(out var results); diff --git a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs index 44ebedb736917..e8fadaeeeab01 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs @@ -82,7 +82,7 @@ public async Task CreateSolutionAsync(Checksum newSolutionChecksum, Ca if (oldSolutionChecksums.AnalyzerReferences.Checksum != newSolutionChecksums.AnalyzerReferences.Checksum) { - solution = solution.WithAnalyzerReferences(await _assetProvider.CreateCollectionAsync( + solution = solution.WithAnalyzerReferences(await _assetProvider.GetAssetsAsync( assetPath: AssetPath.SolutionOnly, newSolutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); } @@ -323,21 +323,21 @@ await _assetProvider.GetAssetAsync( // changed project references if (oldProjectChecksums.ProjectReferences.Checksum != newProjectChecksums.ProjectReferences.Checksum) { - project = project.WithProjectReferences(await _assetProvider.CreateCollectionAsync( + project = project.WithProjectReferences(await _assetProvider.GetAssetsAsync( assetPath: project.Id, newProjectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false)); } // changed metadata references if (oldProjectChecksums.MetadataReferences.Checksum != newProjectChecksums.MetadataReferences.Checksum) { - project = project.WithMetadataReferences(await _assetProvider.CreateCollectionAsync( + project = project.WithMetadataReferences(await _assetProvider.GetAssetsAsync( assetPath: project.Id, newProjectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false)); } // changed analyzer references if (oldProjectChecksums.AnalyzerReferences.Checksum != newProjectChecksums.AnalyzerReferences.Checksum) { - project = project.WithAnalyzerReferences(await _assetProvider.CreateCollectionAsync( + project = project.WithAnalyzerReferences(await _assetProvider.GetAssetsAsync( assetPath: project.Id, newProjectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false)); } From 6d4249b70e9cfd107218668fbb8d84ec0e6c42fe Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:39:32 -0700 Subject: [PATCH 38/40] Simplify --- .../Remote/ServiceHub/Host/AssetProvider.cs | 4 +--- .../Remote/ServiceHub/Host/ChecksumSynchronizer.cs | 14 +++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index f65ad2cf20f08..6fe15044796ee 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -51,9 +51,7 @@ public override async ValueTask GetAssetAsync( { using var _ = PooledDictionary.GetInstance(out var results); - // bulk synchronize checksums first - var syncer = new ChecksumSynchronizer(this); - await syncer.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); + await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); var result = new (Checksum checksum, T asset)[checksums.Count]; var index = 0; diff --git a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs index 8690801be9b38..e6122d3009322 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/ChecksumSynchronizer.cs @@ -107,14 +107,14 @@ await _assetProvider.SynchronizeAssetsAsync( async ValueTask CollectChecksumChildrenAsync(ChecksumSynchronizer @this, ChecksumsAndIds collection) { - foreach (var (checksum, documentId) in collection) + // This GetAssetsAsync call should be fast since they were just retrieved above. There's a small chance + // the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. + var allDocChecksums = await @this._assetProvider.GetAssetsAsync( + AssetPath.ProjectAndDocuments(projectChecksum.ProjectId), collection.Checksums, cancellationToken).ConfigureAwait(false); + foreach (var docChecksums in allDocChecksums) { - // These GetAssetAsync calls should be fast since they were just retrieved above. There's a small - // chance the asset-cache GC pass may have cleaned them up, but that should be exceedingly rare. - var checksumObject = await @this._assetProvider.GetAssetAsync( - assetPath: documentId, checksum, cancellationToken).ConfigureAwait(false); - checksums.Add(checksumObject.Info); - checksums.Add(checksumObject.Text); + checksums.Add(docChecksums.Info); + checksums.Add(docChecksums.Text); } } } From 00e083110775856ddd79d054668d198632d17080 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:50:35 -0700 Subject: [PATCH 39/40] Doc --- .../DocumentBasedFixAllProviderHelpers.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index 32450aa15116f..a9a9712921841 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -56,6 +56,10 @@ internal static class DocumentBasedFixAllProviderHelpers // TODO: consider computing this in parallel. var singleContextDocIdToNewRootOrText = await getFixedDocumentsAsync(fixAllContext, progressTracker).ConfigureAwait(false); + + // Note: it is safe to blindly add the dictionary for a particular context to the full dictionary. Each + // dictionary will only update documents within that context, and each context represents a distinct + // project, so these should all be distinct without collisions. allContextsDocIdToNewRootOrText.AddRange(singleContextDocIdToNewRootOrText); } } From 47c491ffbf7b29cbcb29f56b849c29d1b8acd1ef Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 10:52:35 -0700 Subject: [PATCH 40/40] Doc --- .../DocumentBasedFixAllProviderHelpers.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs index a9a9712921841..37248a1e7031f 100644 --- a/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs +++ b/src/Workspaces/Core/Portable/CodeFixesAndRefactorings/DocumentBasedFixAllProviderHelpers.cs @@ -59,8 +59,10 @@ internal static class DocumentBasedFixAllProviderHelpers // Note: it is safe to blindly add the dictionary for a particular context to the full dictionary. Each // dictionary will only update documents within that context, and each context represents a distinct - // project, so these should all be distinct without collisions. - allContextsDocIdToNewRootOrText.AddRange(singleContextDocIdToNewRootOrText); + // project, so these should all be distinct without collisions. However, to be very safe, we use an + // overwriting policy here to ensure nothing causes any problems here. + foreach (var kvp in singleContextDocIdToNewRootOrText) + allContextsDocIdToNewRootOrText[kvp.Key] = kvp.Value; } }