-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
Conversation
return new SolutionStateChecksums( | ||
attributesChecksum, | ||
new ChecksumCollection(projectChecksums.WhereNotNull().ToImmutableArray()), | ||
projectIds.ToImmutableAndClear(), |
There was a problem hiding this comment.
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.
|
||
public Checksum Attributes { get; } | ||
public ImmutableArray<ProjectId> ProjectIds { get; } | ||
public ChecksumCollection Projects { get; } |
There was a problem hiding this comment.
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
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)); |
There was a problem hiding this comment.
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 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); |
There was a problem hiding this comment.
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.
@@ -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); |
There was a problem hiding this comment.
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.
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
searchingChecksumsLeft.Remove(projectStateChecksums.Checksum)) | ||
var projectState = state.GetProjectState(hintProject); | ||
if (projectState != null && | ||
projectState.TryGetStateChecksums(out var projectStateChecksums)) | ||
{ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/Workspaces/Core/Portable/Workspace/Solution/SolutionState_Checksum.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.