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

Allow server to pass along a specific project-scope it wants to sync data for. #70342

Merged
merged 13 commits into from
Oct 12, 2023
Merged

Allow server to pass along a specific project-scope it wants to sync data for. #70342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2b6b842
Add project id hint
CyrusNajmabadi Oct 10, 2023
f72b32e
Add project id hint
CyrusNajmabadi Oct 10, 2023
1fab9f7
Cleanup
CyrusNajmabadi Oct 10, 2023
c2b8de8
Fixes
CyrusNajmabadi Oct 10, 2023
528a01b
Fix test
CyrusNajmabadi Oct 10, 2023
eb5f197
Fix
CyrusNajmabadi Oct 10, 2023
ab6e43d
Fix
CyrusNajmabadi Oct 10, 2023
bf4badd
Fix
CyrusNajmabadi Oct 10, 2023
afe7f7a
Merge branch 'main' into checksumCleanup6
CyrusNajmabadi Oct 11, 2023
0103f81
Merge branch 'checksumCleanup5' into checksumCleanup6
CyrusNajmabadi Oct 12, 2023
66dc991
Merge branch 'checksumCleanup5' into checksumCleanup6
CyrusNajmabadi Oct 12, 2023
d174bb8
Merge branch 'checksumCleanup5' into checksumCleanup6
CyrusNajmabadi Oct 12, 2023
4b619d7
Merge remote-tracking branch 'upstream/main' into checksumCleanup6
CyrusNajmabadi Oct 12, 2023
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/Remote/SerializationValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ private sealed class AssetProvider : AbstractAssetProvider
public AssetProvider(SerializationValidator validator)
=> _validator = validator;

public override async ValueTask<T> GetAssetAsync<T>(Checksum checksum, CancellationToken cancellationToken)
public override async ValueTask<T> GetAssetAsync<T>(ProjectId? hintProject, Checksum checksum, CancellationToken cancellationToken)
Copy link
Member Author

Choose a reason for hiding this comment

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

can pass this 'hint' along to tell the host where to search, isntead of searching everything.

Note: in a followup PR thsi becomes more generic, so we can also scope down to the document level as well (so it's trivial to get data for things like a document, without having to do a full solution sweep).

=> await _validator.GetValueAsync<T>(checksum).ConfigureAwait(false);
}

Expand Down
6 changes: 3 additions & 3 deletions 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 @@ -53,10 +53,10 @@ private static async Task TestAssetAsync(object data)
var assetSource = new SimpleAssetSource(workspace.Services.GetService<ISerializerService>(), new Dictionary<Checksum, object>() { { checksum, data } });

var provider = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService<ISerializerService>());
var stored = await provider.GetAssetAsync<object>(checksum, CancellationToken.None);
var stored = await provider.GetAssetAsync<object>(hintProject: null, checksum, CancellationToken.None);
Assert.Equal(data, stored);

var stored2 = await provider.GetAssetsAsync<object>(new HashSet<Checksum> { checksum }, CancellationToken.None);
var stored2 = await provider.GetAssetsAsync<object>(hintProject: null, new HashSet<Checksum> { checksum }, CancellationToken.None);
Assert.Equal(1, stored2.Length);

Assert.Equal(checksum, stored2[0].Item1);
Expand All @@ -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(new HashSet<Checksum>(map.Keys), CancellationToken.None);
await service.SynchronizeAssetsAsync(hintProject: null, new HashSet<Checksum>(map.Keys), CancellationToken.None);

foreach (var kv in map)
{
Expand Down
31 changes: 28 additions & 3 deletions src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public static SolutionStateChecksums Deserialize(ObjectReader reader)

public async Task FindAsync(
SolutionState state,
ProjectId? hintProject,
HashSet<Checksum> searchingChecksumsLeft,
Dictionary<Checksum, object> result,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -99,18 +100,42 @@ public async Task FindAsync(
result[FrozenSourceGeneratedDocumentText] = await SerializableSourceText.FromTextDocumentStateAsync(state.FrozenSourceGeneratedDocumentState, cancellationToken).ConfigureAwait(false);
}

foreach (var (_, projectState) in state.ProjectStates)
ChecksumCollection.Find(state.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken);

// 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 state.ProjectStates)
Copy link
Contributor

Choose a reason for hiding this comment

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

state.ProjectStates

I wonder if the hintProject case would be better by just creating an ImmutableDictionary with the one entry in it and enumerating over it in the two loops here, rather than iterating over all the project states

Copy link
Member Author

Choose a reason for hiding this comment

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

this changes in a followup pr. i use the hint project to just index directly into state.ProjectStates.

{
if (searchingChecksumsLeft.Count == 0)
break;

if (hintProject != null && hintProject != 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 state.ProjectStates)
{
if (searchingChecksumsLeft.Count == 0)
break;

if (hintProject != null && hintProject != 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, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false);
}

ChecksumCollection.Find(state.AnalyzerReferences, AnalyzerReferences, searchingChecksumsLeft, result, cancellationToken);
Copy link
Member Author

Choose a reason for hiding this comment

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

by moving this above the projects, we don't bother searching into documents when someone is just trying to sync the solution-checksum-info in isolation.

}
}

Expand Down
33 changes: 14 additions & 19 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,32 +18,27 @@ namespace Microsoft.CodeAnalysis.Remote.Testing;
internal sealed class SimpleAssetSource(ISerializerService serializerService, IReadOnlyDictionary<Checksum, object> map) : IAssetSource
{
public ValueTask<ImmutableArray<object>> GetAssetsAsync(
Checksum solutionChecksum, ImmutableArray<Checksum> checksums, ISerializerService deserializerService, CancellationToken cancellationToken)
Checksum solutionChecksum, ProjectId? hintProject, ImmutableArray<Checksum> checksums, ISerializerService deserializerService, CancellationToken cancellationToken)
{
var results = new List<object>();

foreach (var checksum in checksums)
{
if (map.TryGetValue(checksum, out var data))
{
using var stream = new MemoryStream();
using var context = new SolutionReplicationContext();

using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
{
serializerService.Serialize(data, writer, context, cancellationToken);
}

stream.Position = 0;
using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken);
var asset = deserializerService.Deserialize<object>(data.GetWellKnownSynchronizationKind(), reader, cancellationToken);
Contract.ThrowIfNull(asset);
results.Add(asset);
}
else
Contract.ThrowIfFalse(map.TryGetValue(checksum, out var data));

using var stream = new MemoryStream();
using var context = new SolutionReplicationContext();

using (var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken))
{
throw ExceptionUtilities.UnexpectedValue(checksum);
serializerService.Serialize(data, writer, context, cancellationToken);
}

stream.Position = 0;
using var reader = ObjectReader.GetReader(stream, leaveOpen: true, cancellationToken);
var asset = deserializerService.Deserialize<object>(data.GetWellKnownSynchronizationKind(), reader, cancellationToken);
Contract.ThrowIfNull(asset);
results.Add(asset);
}

return ValueTaskFactory.FromResult(results.ToImmutableArray());
Expand Down
70 changes: 36 additions & 34 deletions src/Workspaces/Remote/Core/AbstractAssetProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,29 @@ internal abstract class AbstractAssetProvider
/// <summary>
/// return data of type T whose checksum is the given checksum
/// </summary>
public abstract ValueTask<T> GetAssetAsync<T>(Checksum checksum, CancellationToken cancellationToken);
public abstract ValueTask<T> GetAssetAsync<T>(ProjectId? hintProject, Checksum checksum, CancellationToken cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

ProjectId? hintProject,

If you see other hint data coming down the line, would it make sense for this to be a struct so everywhere piping it through wouldn't need to change?

Copy link
Member Author

Choose a reason for hiding this comment

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

that happens in the followup :)


public async Task<SolutionInfo> CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken)
{
var solutionChecksums = await GetAssetAsync<SolutionStateChecksums>(solutionChecksum, cancellationToken).ConfigureAwait(false);
var solutionAttributes = await GetAssetAsync<SolutionInfo.SolutionAttributes>(solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false);
var solutionChecksums = await GetAssetAsync<SolutionStateChecksums>(hintProject: null, solutionChecksum, cancellationToken).ConfigureAwait(false);
var solutionAttributes = await GetAssetAsync<SolutionInfo.SolutionAttributes>(hintProject: null, solutionChecksums.Attributes, 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.

no hints here (because this is solution info, not project info). However, with teh reordering we did in the solution-finding code, this will all be cheap. that's because we search the direct (non-project-child) info of the solution first, before recursing. So we'll always find this data in O(1) time, instead of O(solution) time.


using var _ = ArrayBuilder<ProjectInfo>.GetInstance(solutionChecksums.Projects.Count, out var projects);
foreach (var projectChecksum in solutionChecksums.Projects)
projects.AddIfNotNull(await CreateProjectInfoAsync(projectChecksum, cancellationToken).ConfigureAwait(false));

var analyzerReferences = await CreateCollectionAsync<AnalyzerReference>(solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false);
var analyzerReferences = await CreateCollectionAsync<AnalyzerReference>(hintProject: null, solutionChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false);

return SolutionInfo.Create(
solutionAttributes.Id, solutionAttributes.Version, solutionAttributes.FilePath, projects.ToImmutableAndClear(), analyzerReferences).WithTelemetryId(solutionAttributes.TelemetryId);
}

public async Task<ProjectInfo?> CreateProjectInfoAsync(Checksum projectChecksum, CancellationToken cancellationToken)
{
var projectChecksums = await GetAssetAsync<ProjectStateChecksums>(projectChecksum, cancellationToken).ConfigureAwait(false);
var projectChecksums = await GetAssetAsync<ProjectStateChecksums>(hintProject: null, projectChecksum, 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.

note: this is a place where passing a project-id would be better. a followup PR accomplishes this.


var attributes = await GetAssetAsync<ProjectInfo.ProjectAttributes>(projectChecksums.Info, cancellationToken).ConfigureAwait(false);
var projectId = projectChecksums.ProjectId;
var attributes = await GetAssetAsync<ProjectInfo.ProjectAttributes>(hintProject: projectId, projectChecksums.Info, 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.

in this PR, we don't know the projectID without this roundtrip. A future PR fixes that, so we have that info up front.

if (!RemoteSupportedLanguages.IsSupported(attributes.Language))
{
// only add project our workspace supports.
Expand All @@ -49,17 +50,16 @@ public async Task<SolutionInfo> CreateSolutionInfoAsync(Checksum solutionChecksu
}

var compilationOptions = attributes.FixUpCompilationOptions(
await GetAssetAsync<CompilationOptions>(projectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false));
await GetAssetAsync<CompilationOptions>(hintProject: projectId, projectChecksums.CompilationOptions, cancellationToken).ConfigureAwait(false));
var parseOptions = await GetAssetAsync<ParseOptions>(hintProject: projectId, projectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false);

var parseOptions = await GetAssetAsync<ParseOptions>(projectChecksums.ParseOptions, cancellationToken).ConfigureAwait(false);
var projectReferences = await CreateCollectionAsync<ProjectReference>(hintProject: projectId, projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false);
var metadataReferences = await CreateCollectionAsync<MetadataReference>(hintProject: projectId, projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false);
var analyzerReferences = await CreateCollectionAsync<AnalyzerReference>(hintProject: projectId, projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false);

var projectReferences = await CreateCollectionAsync<ProjectReference>(projectChecksums.ProjectReferences, cancellationToken).ConfigureAwait(false);
var metadataReferences = await CreateCollectionAsync<MetadataReference>(projectChecksums.MetadataReferences, cancellationToken).ConfigureAwait(false);
var analyzerReferences = await CreateCollectionAsync<AnalyzerReference>(projectChecksums.AnalyzerReferences, cancellationToken).ConfigureAwait(false);

var documentInfos = await CreateDocumentInfosAsync(projectChecksums.Documents, cancellationToken).ConfigureAwait(false);
var additionalDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AdditionalDocuments, cancellationToken).ConfigureAwait(false);
var analyzerConfigDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AnalyzerConfigDocuments, cancellationToken).ConfigureAwait(false);
var documentInfos = await CreateDocumentInfosAsync(projectChecksums.Documents).ConfigureAwait(false);
var additionalDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AdditionalDocuments).ConfigureAwait(false);
var analyzerConfigDocumentInfos = await CreateDocumentInfosAsync(projectChecksums.AnalyzerConfigDocuments).ConfigureAwait(false);

return ProjectInfo.Create(
attributes,
Expand All @@ -72,13 +72,27 @@ public async Task<SolutionInfo> CreateSolutionInfoAsync(Checksum solutionChecksu
additionalDocumentInfos,
analyzerConfigDocumentInfos,
hostObjectType: null); // TODO: https://github.com/dotnet/roslyn/issues/62804

async Task<ImmutableArray<DocumentInfo>> CreateDocumentInfosAsync(ChecksumCollection documentChecksums)
{
using var _ = ArrayBuilder<DocumentInfo>.GetInstance(documentChecksums.Count, out var documentInfos);

foreach (var documentChecksum in documentChecksums)
{
cancellationToken.ThrowIfCancellationRequested();
documentInfos.Add(await CreateDocumentInfoAsync(projectId, documentChecksum, cancellationToken).ConfigureAwait(false));
}

return documentInfos.ToImmutableAndClear();
}
}

public async Task<DocumentInfo> CreateDocumentInfoAsync(Checksum documentChecksum, CancellationToken cancellationToken)
public async Task<DocumentInfo> CreateDocumentInfoAsync(
ProjectId projectId, Checksum documentChecksum, CancellationToken cancellationToken)
{
var documentSnapshot = await GetAssetAsync<DocumentStateChecksums>(documentChecksum, cancellationToken).ConfigureAwait(false);
var attributes = await GetAssetAsync<DocumentInfo.DocumentAttributes>(documentSnapshot.Info, cancellationToken).ConfigureAwait(false);
var serializableSourceText = await GetAssetAsync<SerializableSourceText>(documentSnapshot.Text, cancellationToken).ConfigureAwait(false);
var documentSnapshot = await GetAssetAsync<DocumentStateChecksums>(hintProject: projectId, documentChecksum, cancellationToken).ConfigureAwait(false);
var attributes = await GetAssetAsync<DocumentInfo.DocumentAttributes>(hintProject: projectId, documentSnapshot.Info, cancellationToken).ConfigureAwait(false);
var serializableSourceText = await GetAssetAsync<SerializableSourceText>(hintProject: projectId, documentSnapshot.Text, 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.

this scopes us down to a particular project to find this data. but we still need to search the whole project to find the document info being asked for. a followup PR addresses this by allowing the doc-id to be passed along as well to narrow the search even further.


var text = await serializableSourceText.GetTextAsync(cancellationToken).ConfigureAwait(false);
var textLoader = TextLoader.From(TextAndVersion.Create(text, VersionStamp.Create(), attributes.FilePath));
Expand All @@ -87,27 +101,15 @@ public async Task<DocumentInfo> CreateDocumentInfoAsync(Checksum documentChecksu
return new DocumentInfo(attributes, textLoader, documentServiceProvider: null);
}

private async Task<ImmutableArray<DocumentInfo>> CreateDocumentInfosAsync(ChecksumCollection documentChecksums, CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<DocumentInfo>.GetInstance(documentChecksums.Count, out var documentInfos);

foreach (var documentChecksum in documentChecksums)
{
cancellationToken.ThrowIfCancellationRequested();
documentInfos.Add(await CreateDocumentInfoAsync(documentChecksum, cancellationToken).ConfigureAwait(false));
}

return documentInfos.ToImmutableAndClear();
}

public async Task<ImmutableArray<T>> CreateCollectionAsync<T>(ChecksumCollection checksums, CancellationToken cancellationToken) where T : class
public async Task<ImmutableArray<T>> CreateCollectionAsync<T>(
ProjectId? hintProject, ChecksumCollection checksums, CancellationToken cancellationToken) where T : class
{
using var _ = ArrayBuilder<T>.GetInstance(checksums.Count, out var assets);

foreach (var checksum in checksums)
{
cancellationToken.ThrowIfCancellationRequested();
assets.Add(await GetAssetAsync<T>(checksum, cancellationToken).ConfigureAwait(false));
assets.Add(await GetAssetAsync<T>(hintProject, checksum, cancellationToken).ConfigureAwait(false));
}

return assets.ToImmutableAndClear();
Expand Down
28 changes: 15 additions & 13 deletions src/Workspaces/Remote/Core/ISolutionAssetProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,22 @@
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.CodeAnalysis.Remote
namespace Microsoft.CodeAnalysis.Remote;

/// <summary>
/// Brokered service.
/// </summary>
internal interface ISolutionAssetProvider
{
/// <summary>
/// Brokered service.
/// Streams serialized assets into the given stream. Assets will be serialized in the exact same order
/// corresponding to the checksum index in <paramref name="checksums"/>.
/// </summary>
internal interface ISolutionAssetProvider
{
/// <summary>
/// Streams serialized assets into the given stream. Assets will be serialized in the exact same order
/// corresponding to the checksum index in <paramref name="checksums"/>.
/// </summary>
/// <param name="pipeWriter">The writer to write the assets into. Implementations of this method must call<see
/// cref="PipeWriter.Complete"/> 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 <see cref="PipeReader"/> side of this.</param>
ValueTask WriteAssetsAsync(PipeWriter pipeWriter, Checksum solutionChecksum, ImmutableArray<Checksum> checksums, CancellationToken cancellationToken);
}
/// <param name="pipeWriter">The writer to write the assets into. Implementations of this method must call<see
/// cref="PipeWriter.Complete"/> 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 <see cref="PipeReader"/> side of this.</param>
/// <param name="hintProject">Optional project id 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.</param>
ValueTask WriteAssetsAsync(PipeWriter pipeWriter, Checksum solutionChecksum, ProjectId? hintProject, ImmutableArray<Checksum> checksums, CancellationToken cancellationToken);
}
Loading