Skip to content

Sync project level info in parallel.#72976

Merged
CyrusNajmabadi merged 58 commits intodotnet:mainfrom
CyrusNajmabadi:syncStripesParallel
Apr 12, 2024
Merged

Sync project level info in parallel.#72976
CyrusNajmabadi merged 58 commits intodotnet:mainfrom
CyrusNajmabadi:syncStripesParallel

Conversation

@CyrusNajmabadi
Copy link
Contributor

Followup to #72975

Shaves off 5 seconds locally when syncing (from 15s to 10s).

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 10, 2024 22:46
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 10, 2024

if (searchingChecksumsLeft.Remove(projectStateChecksums.CompilationOptions))
result[projectStateChecksums.CompilationOptions] = projectState.CompilationOptions!;
await projectStateChecksums.FindAsync(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code was duplicating exactly what projectStateChecksums.FindAsync already does. We just want to be able to search projects, just without a particular project-id provided. so this now defers to that helper to do that. thanks the enum values this is easy.

: AbstractAssetProvider
{
private const int PooledChecksumArraySize = 256;
private const int PooledChecksumArraySize = 1024;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i upped this as i was gonig over 256 a fair bit (easy when we have 900 projects in roslyn alone).

}

checksums.Clear();
async Task SynchronizeProjectAssetCollectionAsync<TAsset>(Func<ProjectStateChecksums, ChecksumCollection> getChecksums, CancellationToken cancellationToken)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is almost the same as the prior method. just the primary method says "we have one checksum of a particular type per project" whereas thsi method says "we have many checksums of a particular type per project".

Note; lots of checksums will be duplicated across projects (like metadata and analyzer references) so thsi hashset dedupes a lot most of the time.


// Then sync each project's documents in parallel with each other.
foreach (var projectChecksums in allProjectChecksums)
tasks.Add(SynchronizeProjectDocumentsAsync(projectChecksums, cancellationToken));
Copy link
Contributor

Choose a reason for hiding this comment

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

cancellationToken

Any concern about a potential large increase in cancellation exceptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can cross that bridge if it happens :-)

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

// looked at.
await this.SynchronizeAssetsAsync<object, Dictionary<Checksum, object>>(
assetPath: AssetPath.SolutionAndTopLevelProjectsOnly,
assetPath: new(AssetPathKind.Solution | AssetPathKind.ProjectStateChecksums),
Copy link
Contributor

Choose a reason for hiding this comment

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

new

nit: could use implicit ctor

/// <summary>
/// Instance that will only look up solution-level data when searching for checksums.
/// </summary>
public static readonly AssetPath SolutionOnly = AssetPathKind.Solution;
Copy link
Contributor

Choose a reason for hiding this comment

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

SolutionOnly

nit: doesn't seem necessary. Almost all other callers just specify the AssetPathKind explicitly.


ChecksumCollection? frozenSourceGeneratedDocumentIdentities = null;
ChecksumsAndIds<DocumentId>? frozenSourceGeneratedDocuments = null;
ChecksumsAndIds<DocumentId>? frozenSourceGeneratedDocumentTexts = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this got simplified to avoid an extra indirection and roundtrip. previously we would store the DocumentStateCheckums for the frozen SG docs. Then, OOP would ask for these, just to get the text checksum, just to call back into the host to get this. This changes to now just store the text checksums only.

@CyrusNajmabadi
Copy link
Contributor Author

@jasonmalinowski For review when you get back.

@dibarbet dibarbet modified the milestones: Next, 17.11 P1 Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants