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

Include project-id in sync'ed data to allow future calls to directly jump to that project. #70343

Merged
merged 13 commits into from
Oct 14, 2023
Merged

Include project-id in sync'ed data to allow future calls to directly jump to that project. #70343

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6120c62
Pass along project id info to help ensure we can do less work looking…
CyrusNajmabadi Oct 11, 2023
54cd06f
shortcircuit
CyrusNajmabadi Oct 11, 2023
2d6a410
Be consistent
CyrusNajmabadi Oct 11, 2023
90b1647
Merge branch 'checksumCleanup6' into checksumCleanup7
CyrusNajmabadi Oct 12, 2023
ee92866
Merge branch 'checksumCleanup6' into checksumCleanup7
CyrusNajmabadi Oct 12, 2023
6972c42
Merge branch 'checksumCleanup6' into checksumCleanup7
CyrusNajmabadi Oct 12, 2023
93abadc
Merge branch 'main' into checksumCleanup7
CyrusNajmabadi Oct 12, 2023
55b72f6
Simplify
CyrusNajmabadi Oct 12, 2023
a33f025
PR feedback
CyrusNajmabadi Oct 12, 2023
5d6e078
named param
CyrusNajmabadi Oct 12, 2023
c251087
Merge remote-tracking branch 'upstream/main' into checksumCleanup7
CyrusNajmabadi Oct 12, 2023
d5afa2f
Merge remote-tracking branch 'upstream/main' into checksumCleanup7
CyrusNajmabadi Oct 13, 2023
aafc45b
Merge remote-tracking branch 'upstream/main' into checksumCleanup7
CyrusNajmabadi Oct 13, 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
9 changes: 4 additions & 5 deletions src/Workspaces/Core/Portable/Workspace/Solution/ChecksumCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ internal static async Task FindAsync<TState>(
{
foreach (var (_, state) in documentStates.States)
{
cancellationToken.ThrowIfCancellationRequested();
if (searchingChecksumsLeft.Count == 0)
return;

Contract.ThrowIfFalse(state.TryGetStateChecksums(out var stateChecksums));

await stateChecksums.FindAsync(state, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false);
if (searchingChecksumsLeft.Count == 0)
{
return;
}
}
}

Expand All @@ -74,7 +74,6 @@ internal static void Find<T>(
for (var i = 0; i < checksums.Children.Length; i++)
{
cancellationToken.ThrowIfCancellationRequested();

if (searchingChecksumsLeft.Count == 0)
return;

Expand Down
21 changes: 17 additions & 4 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState_Checksum.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Remote;
using Microsoft.CodeAnalysis.Serialization;
using Roslyn.Utilities;
Expand Down Expand Up @@ -128,13 +129,13 @@ private async Task<SolutionStateChecksums> ComputeChecksumsAsync(
// if it's a project that's specifically in the sync'ed cone, include this checksum so that
// this project definitely syncs over.
if (t.mustCompute)
return await t.state.GetChecksumAsync(cancellationToken).ConfigureAwait(false);
return await t.state.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false);

// If it's a project that is not in the cone, still try to get the latest checksum for it if
// we have it. That way we don't send over a checksum *without* that project, causing the
// OOP side to throw that project away (along with all the compilation info stored with it).
if (t.state.TryGetStateChecksums(out var stateChecksums))
return stateChecksums.Checksum;
return stateChecksums;

// We have never computed the checksum for this project. Don't send anything for it.
return null;
Expand All @@ -156,10 +157,22 @@ private async Task<SolutionStateChecksums> ComputeChecksumsAsync(
var analyzerReferenceChecksums = ChecksumCache.GetOrCreate<ChecksumCollection>(AnalyzerReferences,
_ => new ChecksumCollection(AnalyzerReferences.SelectAsArray(r => serializer.CreateChecksum(r, cancellationToken))));

var projectChecksums = await Task.WhenAll(projectChecksumTasks).ConfigureAwait(false);
var allResults = await Task.WhenAll(projectChecksumTasks).ConfigureAwait(false);
using var _1 = ArrayBuilder<ProjectId>.GetInstance(projectChecksumTasks.Length, out var projectIds);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
using var _2 = ArrayBuilder<Checksum>.GetInstance(projectChecksumTasks.Length, out var projectChecksums);
foreach (var projectStateChecksums in allResults)
{
if (projectStateChecksums != null)
{
projectIds.Add(projectStateChecksums.ProjectId);
projectChecksums.Add(projectStateChecksums.Checksum);
}
}

return new SolutionStateChecksums(
attributesChecksum,
new ChecksumCollection(projectChecksums.WhereNotNull().ToImmutableArray()),
projectIds.ToImmutableAndClear(),
Copy link
Member Author

Choose a reason for hiding this comment

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

passing along the project ids, not just their checksums. that way when we jhave an instance of SolutionStateChecksums we can scope requests down to a particular id when trying to retrieve a particular project checksum.

new ChecksumCollection(projectChecksums.ToImmutableAndClear()),
analyzerReferenceChecksums,
frozenSourceGeneratedDocumentIdentityChecksum,
frozenSourceGeneratedDocumentTextChecksum);
Expand Down
123 changes: 78 additions & 45 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 @@ -4,6 +4,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
Expand All @@ -14,27 +15,43 @@

namespace Microsoft.CodeAnalysis.Serialization;

internal sealed class SolutionStateChecksums(
Checksum attributesChecksum,
ChecksumCollection projectChecksums,
ChecksumCollection analyzerReferenceChecksums,
Checksum frozenSourceGeneratedDocumentIdentity,
Checksum frozenSourceGeneratedDocumentText) : IChecksummedObject
internal sealed class SolutionStateChecksums : IChecksummedObject
{
public Checksum Checksum { get; } = Checksum.Create(stackalloc[]
public Checksum Checksum { get; }

public Checksum Attributes { get; }
public ImmutableArray<ProjectId> ProjectIds { get; }
public ChecksumCollection Projects { get; }
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: i merge ProejctIds and Projects into a singular collection type in a later PR to keep these more tightly bound

public ChecksumCollection AnalyzerReferences { get; }
public Checksum FrozenSourceGeneratedDocumentIdentity { get; }
public Checksum FrozenSourceGeneratedDocumentText { get; }

public SolutionStateChecksums(
Checksum attributes,
ImmutableArray<ProjectId> projectIds,
ChecksumCollection projects,
ChecksumCollection analyzerReferences,
Checksum frozenSourceGeneratedDocumentIdentity,
Checksum frozenSourceGeneratedDocumentText)
{
attributesChecksum.Hash,
projectChecksums.Checksum.Hash,
analyzerReferenceChecksums.Checksum.Hash,
frozenSourceGeneratedDocumentIdentity.Hash,
frozenSourceGeneratedDocumentText.Hash,
});
Contract.ThrowIfFalse(projectIds.Length == projects.Children.Length);

public Checksum Attributes => attributesChecksum;
public ChecksumCollection Projects => projectChecksums;
public ChecksumCollection AnalyzerReferences => analyzerReferenceChecksums;
public Checksum FrozenSourceGeneratedDocumentIdentity => frozenSourceGeneratedDocumentIdentity;
public Checksum FrozenSourceGeneratedDocumentText => frozenSourceGeneratedDocumentText;
this.Checksum = Checksum.Create(stackalloc[]
{
attributes.Hash,
projects.Checksum.Hash,
analyzerReferences.Checksum.Hash,
frozenSourceGeneratedDocumentIdentity.Hash,
frozenSourceGeneratedDocumentText.Hash,
});

this.Attributes = attributes;
this.ProjectIds = projectIds;
this.Projects = projects;
this.AnalyzerReferences = analyzerReferences;
this.FrozenSourceGeneratedDocumentIdentity = frozenSourceGeneratedDocumentIdentity;
this.FrozenSourceGeneratedDocumentText = frozenSourceGeneratedDocumentText;
}

public void AddAllTo(HashSet<Checksum> checksums)
{
Expand All @@ -51,6 +68,7 @@ public void Serialize(ObjectWriter writer)
// Writing this is optional, but helps ensure checksums are being computed properly on both the host and oop side.
this.Checksum.WriteTo(writer);
this.Attributes.WriteTo(writer);
writer.WriteArray(this.ProjectIds, static (writer, value) => value.WriteTo(writer));
this.Projects.WriteTo(writer);
this.AnalyzerReferences.WriteTo(writer);
this.FrozenSourceGeneratedDocumentIdentity.WriteTo(writer);
Expand All @@ -61,9 +79,10 @@ public static SolutionStateChecksums Deserialize(ObjectReader reader)
{
var checksum = Checksum.ReadFrom(reader);
var result = new SolutionStateChecksums(
attributesChecksum: Checksum.ReadFrom(reader),
projectChecksums: ChecksumCollection.ReadFrom(reader),
analyzerReferenceChecksums: ChecksumCollection.ReadFrom(reader),
attributes: Checksum.ReadFrom(reader),
reader.ReadArray(ProjectId.ReadFrom),
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
projects: ChecksumCollection.ReadFrom(reader),
analyzerReferences: ChecksumCollection.ReadFrom(reader),
frozenSourceGeneratedDocumentIdentity: Checksum.ReadFrom(reader),
frozenSourceGeneratedDocumentText: Checksum.ReadFrom(reader));
Contract.ThrowIfFalse(result.Checksum == checksum);
Expand Down Expand Up @@ -102,39 +121,48 @@ public async Task FindAsync(

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.
if (searchingChecksumsLeft.Count == 0)
return;

foreach (var (projectId, projectState) in state.ProjectStates)
if (hintProject != null)
{
if (searchingChecksumsLeft.Count == 0)
break;

if (hintProject != null && hintProject != projectId)
continue;

if (projectState.TryGetStateChecksums(out var projectStateChecksums) &&
searchingChecksumsLeft.Remove(projectStateChecksums.Checksum))
var projectState = state.GetProjectState(hintProject);
if (projectState != null &&
projectState.TryGetStateChecksums(out var projectStateChecksums))
{
Copy link
Contributor

Choose a reason for hiding this comment

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

a searchingChecksumsLeft.Remove call isn't required here?

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. this isn't removing. this is just fast-pathing finding the project-state-checksums to dive into it to search the actual checksums.

result[projectStateChecksums.Checksum] = projectStateChecksums;
await projectStateChecksums.FindAsync(projectState, searchingChecksumsLeft, result, cancellationToken).ConfigureAwait(false);
}
}
else
{
// 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.

// Now actually do the depth first search into each project.
foreach (var (_, projectState) in state.ProjectStates)
{
if (searchingChecksumsLeft.Count == 0)
break;

if (projectState.TryGetStateChecksums(out var projectStateChecksums) &&
searchingChecksumsLeft.Remove(projectStateChecksums.Checksum))
{
result[projectStateChecksums.Checksum] = projectStateChecksums;
}
}

foreach (var (projectId, projectState) in state.ProjectStates)
{
if (searchingChecksumsLeft.Count == 0)
break;
// Now actually do the depth first search into each project.

if (hintProject != null && hintProject != projectId)
continue;
foreach (var (_, projectState) in state.ProjectStates)
{
if (searchingChecksumsLeft.Count == 0)
break;

// 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);
// 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);
}
}
}
}
Expand Down Expand Up @@ -256,6 +284,11 @@ public async Task FindAsync(
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;

if (searchingChecksumsLeft.Remove(Info))
{
result[Info] = state.ProjectInfo.Attributes;
Expand Down
11 changes: 6 additions & 5 deletions src/Workspaces/Remote/Core/AbstractAssetProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Remote;

Expand All @@ -27,20 +28,20 @@ public async Task<SolutionInfo> CreateSolutionInfoAsync(Checksum solutionChecksu
var solutionAttributes = await GetAssetAsync<SolutionInfo.SolutionAttributes>(hintProject: null, solutionChecksums.Attributes, cancellationToken).ConfigureAwait(false);

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));
for (int i = 0, n = solutionChecksums.ProjectIds.Length; i < n; i++)
projects.AddIfNotNull(await CreateProjectInfoAsync(solutionChecksums.ProjectIds[i], solutionChecksums.Projects[i], 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 benefit comes here.


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

Choose a reason for hiding this comment

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

now, when asking to retrieve the ProjectStateChecksums we don't need to do a linear walk to find it. we can directly grab it using the known projectId.


var projectId = projectChecksums.ProjectId;
var attributes = await GetAssetAsync<ProjectInfo.ProjectAttributes>(hintProject: projectId, projectChecksums.Info, cancellationToken).ConfigureAwait(false);
if (!RemoteSupportedLanguages.IsSupported(attributes.Language))
{
Expand Down
16 changes: 9 additions & 7 deletions src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.SolutionCreator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public async Task<Solution> CreateSolutionAsync(Checksum newSolutionChecksum, Ca

if (oldSolutionChecksums.Projects.Checksum != newSolutionChecksums.Projects.Checksum)
{
solution = await UpdateProjectsAsync(solution, oldSolutionChecksums.Projects, newSolutionChecksums.Projects, cancellationToken).ConfigureAwait(false);
solution = await UpdateProjectsAsync(
solution, oldSolutionChecksums, newSolutionChecksums, 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.

i preferred this form as just passing around CheckasumCollection instances is unclear as to what they represent.

}

if (oldSolutionChecksums.AnalyzerReferences.Checksum != newSolutionChecksums.AnalyzerReferences.Checksum)
Expand Down Expand Up @@ -104,17 +105,18 @@ public async Task<Solution> CreateSolutionAsync(Checksum newSolutionChecksum, Ca
}
}

private async Task<Solution> UpdateProjectsAsync(Solution solution, ChecksumCollection oldChecksums, ChecksumCollection newChecksums, CancellationToken cancellationToken)
private async Task<Solution> UpdateProjectsAsync(
Solution solution, SolutionStateChecksums oldSolutionChecksums, SolutionStateChecksums newSolutionChecksums, CancellationToken cancellationToken)
{
using var olds = SharedPools.Default<HashSet<Checksum>>().GetPooledObject();
using var news = SharedPools.Default<HashSet<Checksum>>().GetPooledObject();

olds.Object.UnionWith(oldChecksums);
news.Object.UnionWith(newChecksums);
olds.Object.UnionWith(oldSolutionChecksums.Projects);
news.Object.UnionWith(newSolutionChecksums.Projects);

// remove projects that exist in both side
olds.Object.ExceptWith(newChecksums);
news.Object.ExceptWith(oldChecksums);
olds.Object.ExceptWith(newSolutionChecksums.Projects);
news.Object.ExceptWith(oldSolutionChecksums.Projects);

using var _1 = PooledDictionary<ProjectId, ProjectStateChecksums>.GetInstance(out var oldMap);
using var _2 = PooledDictionary<ProjectId, ProjectStateChecksums>.GetInstance(out var newMap);
Expand All @@ -130,7 +132,7 @@ private async Task<Solution> UpdateProjectsAsync(Solution solution, ChecksumColl
{
if (!oldMap.ContainsKey(projectId))
{
var projectInfo = await _assetProvider.CreateProjectInfoAsync(newProjectChecksums.Checksum, cancellationToken).ConfigureAwait(false);
var projectInfo = await _assetProvider.CreateProjectInfoAsync(projectId, newProjectChecksums.Checksum, cancellationToken).ConfigureAwait(false);
if (projectInfo == null)
{
// this project is not supported in OOP
Expand Down
Loading