Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use generics in syncing code. #72929

Merged
merged 6 commits into from
Apr 8, 2024
Merged

Use generics in syncing code. #72929

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fbae44f
use generics while syncing
CyrusNajmabadi Apr 8, 2024
4f41b24
lint
CyrusNajmabadi Apr 8, 2024
3f0a815
Add assert
CyrusNajmabadi Apr 8, 2024
d8d01db
Merge branch 'main' into generics
CyrusNajmabadi Apr 8, 2024
d9b8df6
Merge remote-tracking branch 'upstream/main' into generics
CyrusNajmabadi Apr 8, 2024
58bad7e
Direct array
CyrusNajmabadi Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/VisualStudio/Core/Test.Next/Services/AssetProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public async Task TestAssetSynchronization()
var assetSource = new SimpleAssetSource(workspace.Services.GetService<ISerializerService>(), map);

var service = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService<ISerializerService>());
await service.SynchronizeAssetsAsync(AssetPath.FullLookupForTesting, new HashSet<Checksum>(map.Keys), results: null, CancellationToken.None);
await service.SynchronizeAssetsAsync<object>(AssetPath.FullLookupForTesting, new HashSet<Checksum>(map.Keys), results: null, CancellationToken.None);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<object> is used when the request is genuinely asking for heterogenous data.


foreach (var kv in map)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Workspaces/CoreTestUtilities/Fakes/SimpleAssetSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace Microsoft.CodeAnalysis.Remote.Testing;
/// </summary>
internal sealed class SimpleAssetSource(ISerializerService serializerService, IReadOnlyDictionary<Checksum, object> map) : IAssetSource
{
public ValueTask<ImmutableArray<object>> GetAssetsAsync(
public ValueTask<ImmutableArray<T>> GetAssetsAsync<T>(
Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory<Checksum> checksums, ISerializerService deserializerService, CancellationToken cancellationToken)
{
var results = new List<object>();
var results = new List<T>();

foreach (var checksum in checksums.Span)
{
Expand All @@ -39,7 +39,7 @@ public ValueTask<ImmutableArray<object>> 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());
Expand Down
12 changes: 6 additions & 6 deletions src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static void WriteAsset(ObjectWriter writer, ISerializerService serializer, Solut
}
}

public static ValueTask<ImmutableArray<object>> ReadDataAsync(
public static ValueTask<ImmutableArray<T>> ReadDataAsync<T>(
PipeReader pipeReader, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken)
{
// Suppress ExecutionContext flow for asynchronous operations operate on the pipe. In addition to avoiding
Expand All @@ -78,17 +78,17 @@ public static ValueTask<ImmutableArray<object>> ReadDataAsync(
using var _ = FlowControlHelper.TrySuppressFlow();
return ReadDataSuppressedFlowAsync(pipeReader, solutionChecksum, objectCount, serializerService, cancellationToken);

static async ValueTask<ImmutableArray<object>> ReadDataSuppressedFlowAsync(
static async ValueTask<ImmutableArray<T>> 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<T>(stream, solutionChecksum, objectCount, serializerService, cancellationToken);
}
}

public static ImmutableArray<object> ReadData(Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken)
public static ImmutableArray<T> ReadData<T>(Stream stream, Checksum solutionChecksum, int objectCount, ISerializerService serializerService, CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<object>.GetInstance(objectCount, out var results);
using var _ = ArrayBuilder<T>.GetInstance(objectCount, out var results);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken);

Expand All @@ -104,7 +104,7 @@ public static ImmutableArray<object> 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();
Expand Down
30 changes: 15 additions & 15 deletions src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,24 @@ public override async ValueTask<T> GetAssetAsync<T>(
using var _1 = PooledHashSet<Checksum>.GetInstance(out var checksums);
checksums.Add(checksum);

using var _2 = PooledDictionary<Checksum, object>.GetInstance(out var results);
using var _2 = PooledDictionary<Checksum, T>.GetInstance(out var results);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method this is in is already generic. so this now allows that generic information to flow into SynchronizeAssetsAsync

await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false);

return (T)results[checksum];
return results[checksum];
}

public override async ValueTask<ImmutableArray<(Checksum checksum, T asset)>> GetAssetsAsync<T>(
AssetPath assetPath, HashSet<Checksum> checksums, CancellationToken cancellationToken)
{
using var _ = PooledDictionary<Checksum, object>.GetInstance(out var results);
using var _ = PooledDictionary<Checksum, T>.GetInstance(out var results);

await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method this is in is already generic. so this now allows that generic information to flow into SynchronizeAssetsAsync


var result = new (Checksum checksum, T asset)[checksums.Count];
var index = 0;
foreach (var (checksum, assetObject) in results)
{
result[index] = (checksum, (T)assetObject);
result[index] = (checksum, assetObject);
index++;
}

Expand Down Expand Up @@ -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<object>(assetPath: AssetPath.SolutionOnly, checksums, results: null, cancellationToken).ConfigureAwait(false);

// third, get direct children of the solution state.
var stateChecksums = await this.GetAssetAsync<SolutionStateChecksums>(
Expand Down Expand Up @@ -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<object>(
assetPath: AssetPath.ProjectAndDocuments(projectChecksums.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false);

checksums.Clear();
Expand All @@ -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<object>(
assetPath: AssetPath.ProjectAndDocuments(projectChecksums.ProjectId), checksums, results: null, cancellationToken).ConfigureAwait(false);
}

Expand All @@ -185,8 +185,8 @@ static void AddAll(HashSet<Checksum> checksums, ChecksumCollection checksumColle
}
}

public async ValueTask SynchronizeAssetsAsync(
AssetPath assetPath, HashSet<Checksum> checksums, Dictionary<Checksum, object>? results, CancellationToken cancellationToken)
public async ValueTask SynchronizeAssetsAsync<T>(
AssetPath assetPath, HashSet<Checksum> checksums, Dictionary<Checksum, T>? results, CancellationToken cancellationToken)
{
Contract.ThrowIfTrue(checksums.Contains(Checksum.Null));
if (checksums.Count == 0)
Expand All @@ -209,7 +209,7 @@ public async ValueTask SynchronizeAssetsAsync(
missingChecksumsCount = 0;
foreach (var checksum in checksums)
{
if (_assetCache.TryGetAsset<object>(checksum, out var existing))
if (_assetCache.TryGetAsset<T>(checksum, out var existing))
{
AddResult(checksum, existing);
}
Expand Down Expand Up @@ -238,7 +238,7 @@ public async ValueTask SynchronizeAssetsAsync(
if (missingChecksumsCount > 0)
{
var missingChecksumsMemory = new ReadOnlyMemory<Checksum>(missingChecksums, 0, missingChecksumsCount);
var missingAssets = await RequestAssetsAsync(assetPath, missingChecksumsMemory, cancellationToken).ConfigureAwait(false);
var missingAssets = await RequestAssetsAsync<T>(assetPath, missingChecksumsMemory, cancellationToken).ConfigureAwait(false);

Contract.ThrowIfTrue(missingChecksumsMemory.Length != missingAssets.Length);

Expand All @@ -248,7 +248,7 @@ public async ValueTask SynchronizeAssetsAsync(
var missingAsset = missingAssets[i];

AddResult(missingChecksum, missingAsset);
_assetCache.GetOrAdd(missingChecksum, missingAsset);
_assetCache.GetOrAdd(missingChecksum, missingAsset!);
}
}

Expand All @@ -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<ImmutableArray<object>> RequestAssetsAsync(
private async ValueTask<ImmutableArray<T>> RequestAssetsAsync<T>(
AssetPath assetPath, ReadOnlyMemory<Checksum> checksums, CancellationToken cancellationToken)
{
#if NETCOREAPP
Expand All @@ -277,6 +277,6 @@ private async ValueTask<ImmutableArray<object>> RequestAssetsAsync(
if (checksums.Length == 0)
return [];

return await _assetSource.GetAssetsAsync(_solutionChecksum, assetPath, checksums, _serializerService, cancellationToken).ConfigureAwait(false);
return await _assetSource.GetAssetsAsync<T>(_solutionChecksum, assetPath, checksums, _serializerService, cancellationToken).ConfigureAwait(false);
}
}
2 changes: 1 addition & 1 deletion src/Workspaces/Remote/ServiceHub/Host/IAssetSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ namespace Microsoft.CodeAnalysis.Remote;
/// </summary>
internal interface IAssetSource
{
ValueTask<ImmutableArray<object>> GetAssetsAsync(
ValueTask<ImmutableArray<T>> GetAssetsAsync<T>(
Checksum solutionChecksum, AssetPath assetPath, ReadOnlyMemory<Checksum> checksums, ISerializerService serializerService, CancellationToken cancellationToken);
}
1 change: 1 addition & 0 deletions src/Workspaces/Remote/ServiceHub/Host/SolutionAssetCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal sealed class SolutionAssetSource(ServiceBrokerClient client) : IAssetSo
{
private readonly ServiceBrokerClient _client = client;

public async ValueTask<ImmutableArray<object>> GetAssetsAsync(
public async ValueTask<ImmutableArray<T>> GetAssetsAsync<T>(
Checksum solutionChecksum,
AssetPath assetPath,
ReadOnlyMemory<Checksum> checksums,
Expand All @@ -31,7 +31,7 @@ public async ValueTask<ImmutableArray<object>> 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<T>(pipeReader, solutionChecksum, checksums.Length, serializerService, cancellationToken),
cancellationToken),
cancellationToken).ConfigureAwait(false);
}
Expand Down
Loading