From fbae44f0faad7e0593966ec8906f5eb01d602cc8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 7 Apr 2024 19:07:10 -0700 Subject: [PATCH 1/4] use generics while syncing --- .../Test.Next/Services/AssetProviderTests.cs | 2 +- .../Fakes/SimpleAssetSource.cs | 6 ++-- .../Core/RemoteHostAssetSerialization.cs | 12 ++++---- .../Remote/ServiceHub/Host/AssetProvider.cs | 28 +++++++++---------- .../Remote/ServiceHub/Host/IAssetSource.cs | 2 +- .../ServiceHub/Host/SolutionAssetSource.cs | 4 +-- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs index 6338cb8e0bdde..bbc1a1c351225 100644 --- a/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs +++ b/src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs @@ -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(AssetPath.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/CoreTestUtilities/Fakes/SimpleAssetSource.cs b/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs index 5482ce1b62a35..d86e63c29f436 100644 --- a/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs +++ b/src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs @@ -18,10 +18,10 @@ namespace Microsoft.CodeAnalysis.Remote.Testing; /// internal sealed class SimpleAssetSource(ISerializerService serializerService, IReadOnlyDictionary map) : IAssetSource { - public ValueTask> GetAssetsAsync( + public ValueTask> GetAssetsAsync( Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, ISerializerService deserializerService, CancellationToken cancellationToken) { - var results = new List(); + var results = new List(); foreach (var checksum in checksums.Span) { @@ -39,7 +39,7 @@ public ValueTask> GetAssetsAsync( using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken); var asset = deserializerService.Deserialize(data.GetWellKnownSynchronizationKind(), reader, cancellationToken); Contract.ThrowIfNull(asset); - results.Add(asset); + results.Add((T)asset); } return ValueTaskFactory.FromResult(results.ToImmutableArray()); diff --git a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs index b7a04d068ce19..fb01f05403286 100644 --- a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs +++ b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs @@ -66,7 +66,7 @@ static void WriteAsset(ObjectWriter writer, ISerializerService serializer, Solut } } - public static ValueTask> ReadDataAsync( + public static ValueTask> ReadDataAsync( PipeReader pipeReader, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken) { // Suppress ExecutionContext flow for asynchronous operations operate on the pipe. In addition to avoiding @@ -78,17 +78,17 @@ public static ValueTask> ReadDataAsync( using var _ = FlowControlHelper.TrySuppressFlow(); return ReadDataSuppressedFlowAsync(pipeReader, solutionChecksum, objectCount, serializerService, cancellationToken); - static async ValueTask> ReadDataSuppressedFlowAsync( + static async ValueTask> ReadDataSuppressedFlowAsync( PipeReader pipeReader, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken) { using var stream = await pipeReader.AsPrebufferedStreamAsync(cancellationToken).ConfigureAwait(false); - return ReadData(stream, solutionChecksum, objectCount, serializerService, cancellationToken); + return ReadData(stream, solutionChecksum, objectCount, serializerService, cancellationToken); } } - public static ImmutableArray ReadData(Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken) + public static ImmutableArray ReadData(Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken) { - using var _ = ArrayBuilder.GetInstance(objectCount, out var results); + using var _ = ArrayBuilder.GetInstance(objectCount, out var results); using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken); @@ -104,7 +104,7 @@ public static ImmutableArray ReadData(Stream stream, Checksum solutionCh // in service hub, cancellation means simply closed stream var result = serializerService.Deserialize(kind, reader, cancellationToken); Contract.ThrowIfNull(result); - results.Add(result); + results.Add((T)result); } return results.ToImmutableAndClear(); diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index 2ab3af2a1324d..7e6e96a75f8e5 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -39,16 +39,16 @@ public override async ValueTask GetAssetAsync( using var _1 = PooledHashSet.GetInstance(out var checksums); checksums.Add(checksum); - using var _2 = PooledDictionary.GetInstance(out var results); + using var _2 = PooledDictionary.GetInstance(out var results); await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); - return (T)results[checksum]; + return results[checksum]; } public override async ValueTask> GetAssetsAsync( AssetPath assetPath, HashSet checksums, CancellationToken cancellationToken) { - using var _ = PooledDictionary.GetInstance(out var results); + using var _ = PooledDictionary.GetInstance(out var results); await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); @@ -56,7 +56,7 @@ public override async ValueTask GetAssetAsync( var index = 0; foreach (var (checksum, assetObject) in results) { - result[index] = (checksum, (T)assetObject); + result[index] = (checksum, assetObject); index++; } @@ -98,7 +98,7 @@ async ValueTask SynchronizeSolutionAssetsWorkerAsync() // second, get direct children of the solution compilation state. compilationStateChecksums.AddAllTo(checksums); - await this.SynchronizeAssetsAsync(assetPath: AssetPath.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); + await this.SynchronizeAssetsAsync(assetPath: AssetPath.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false); // third, get direct children of the solution state. var stateChecksums = await this.GetAssetAsync( @@ -151,7 +151,7 @@ async ValueTask SynchronizeProjectAssetsWorkerAsync() AddAll(checksums, projectChecksums.AnalyzerConfigDocuments.Checksums); // First synchronize all the top-level info about this project. - await this.SynchronizeAssetsAsync( + await this.SynchronizeAssetsAsync( assetPath: AssetPath.ProjectAndDocuments(projectChecksums.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false); checksums.Clear(); @@ -161,7 +161,7 @@ await this.SynchronizeAssetsAsync( await CollectChecksumChildrenAsync(checksums, projectChecksums.AdditionalDocuments).ConfigureAwait(false); await CollectChecksumChildrenAsync(checksums, projectChecksums.AnalyzerConfigDocuments).ConfigureAwait(false); - await this.SynchronizeAssetsAsync( + await this.SynchronizeAssetsAsync( assetPath: AssetPath.ProjectAndDocuments(projectChecksums.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false); } @@ -185,8 +185,8 @@ static void AddAll(HashSet checksums, ChecksumCollection checksumColle } } - public async ValueTask SynchronizeAssetsAsync( - AssetPath assetPath, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) + public async ValueTask SynchronizeAssetsAsync( + AssetPath assetPath, HashSet checksums, Dictionary? results, CancellationToken cancellationToken) { Contract.ThrowIfTrue(checksums.Contains(Checksum.Null)); if (checksums.Count == 0) @@ -209,7 +209,7 @@ public async ValueTask SynchronizeAssetsAsync( missingChecksumsCount = 0; foreach (var checksum in checksums) { - if (_assetCache.TryGetAsset(checksum, out var existing)) + if (_assetCache.TryGetAsset(checksum, out var existing)) { AddResult(checksum, existing); } @@ -238,7 +238,7 @@ public async ValueTask SynchronizeAssetsAsync( if (missingChecksumsCount > 0) { var missingChecksumsMemory = new ReadOnlyMemory(missingChecksums, 0, missingChecksumsCount); - var missingAssets = await RequestAssetsAsync(assetPath, missingChecksumsMemory, cancellationToken).ConfigureAwait(false); + var missingAssets = await RequestAssetsAsync(assetPath, missingChecksumsMemory, cancellationToken).ConfigureAwait(false); Contract.ThrowIfTrue(missingChecksumsMemory.Length != missingAssets.Length); @@ -258,14 +258,14 @@ public async ValueTask SynchronizeAssetsAsync( return; - void AddResult(Checksum checksum, object result) + void AddResult(Checksum checksum, T result) { if (results != null) results[checksum] = result; } } - private async ValueTask> RequestAssetsAsync( + private async ValueTask> RequestAssetsAsync( AssetPath assetPath, ReadOnlyMemory checksums, CancellationToken cancellationToken) { #if NETCOREAPP @@ -277,6 +277,6 @@ private async ValueTask> RequestAssetsAsync( if (checksums.Length == 0) return []; - return await _assetSource.GetAssetsAsync(_solutionChecksum, assetPath, checksums, _serializerService, cancellationToken).ConfigureAwait(false); + return await _assetSource.GetAssetsAsync(_solutionChecksum, assetPath, checksums, _serializerService, cancellationToken).ConfigureAwait(false); } } diff --git a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs index d7fcab7984e13..cbc79a32ac787 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs @@ -15,6 +15,6 @@ namespace Microsoft.CodeAnalysis.Remote; /// internal interface IAssetSource { - ValueTask> GetAssetsAsync( + ValueTask> GetAssetsAsync( Checksum solutionChecksum, AssetPath assetPath, 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 d81767fbddda4..66a465b721823 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs @@ -16,7 +16,7 @@ internal sealed class SolutionAssetSource(ServiceBrokerClient client) : IAssetSo { private readonly ServiceBrokerClient _client = client; - public async ValueTask> GetAssetsAsync( + public async ValueTask> GetAssetsAsync( Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory checksums, @@ -31,7 +31,7 @@ public async ValueTask> GetAssetsAsync( SolutionAssetProvider.ServiceDescriptor, (callback, cancellationToken) => callback.InvokeAsync( (proxy, pipeWriter, cancellationToken) => proxy.WriteAssetsAsync(pipeWriter, solutionChecksum, assetPath, checksums, cancellationToken), - (pipeReader, cancellationToken) => RemoteHostAssetSerialization.ReadDataAsync(pipeReader, solutionChecksum, checksums.Length, serializerService, cancellationToken), + (pipeReader, cancellationToken) => RemoteHostAssetSerialization.ReadDataAsync(pipeReader, solutionChecksum, checksums.Length, serializerService, cancellationToken), cancellationToken), cancellationToken).ConfigureAwait(false); } From 4f41b242a911f9792cc500fc8a1ab7695f2bb229 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 8 Apr 2024 00:17:34 -0700 Subject: [PATCH 2/4] lint --- src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs index 7e6e96a75f8e5..0eb800bb64fe2 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs @@ -248,7 +248,7 @@ public async ValueTask SynchronizeAssetsAsync( var missingAsset = missingAssets[i]; AddResult(missingChecksum, missingAsset); - _assetCache.GetOrAdd(missingChecksum, missingAsset); + _assetCache.GetOrAdd(missingChecksum, missingAsset!); } } From 3f0a815746a1adc735c5f7cabd96e2f4763efce7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 8 Apr 2024 00:18:07 -0700 Subject: [PATCH 3/4] Add assert --- src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs index 9d7ddc0d3a3b7..9b5bae5bfb29f 100644 --- a/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs +++ b/src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs @@ -83,6 +83,7 @@ public SolutionAssetCache(RemoteWorkspace? remoteWorkspace, TimeSpan cleanupInte public object GetOrAdd(Checksum checksum, object value) { + Contract.ThrowIfNull(value); UpdateLastActivityTime(); var entry = _assets.GetOrAdd(checksum, new Entry(value)); From 58bad7efd396a8c5fe762d0c3ebba23e4112c11b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 8 Apr 2024 13:55:03 -0700 Subject: [PATCH 4/4] Direct array --- src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs index fb01f05403286..296505712f54d 100644 --- a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs +++ b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.IO; using System.IO.Pipelines; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.PooledObjects; @@ -88,7 +89,7 @@ static async ValueTask> ReadDataSuppressedFlowAsync( public static ImmutableArray ReadData(Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken) { - using var _ = ArrayBuilder.GetInstance(objectCount, out var results); + var results = new T[objectCount]; using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken); @@ -104,10 +105,10 @@ public static ImmutableArray ReadData(Stream stream, Checksum solutionChec // in service hub, cancellation means simply closed stream var result = serializerService.Deserialize(kind, reader, cancellationToken); Contract.ThrowIfNull(result); - results.Add((T)result); + results[i] = (T)result; } - return results.ToImmutableAndClear(); + return ImmutableCollectionsMarshal.AsImmutableArray(results); } } }