-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Move to a callback-style approach to deserializing objects (part2) #72944
Conversation
|
||
await this.SynchronizeAssetsAsync(assetPath, checksums, results, cancellationToken).ConfigureAwait(false); | ||
|
||
var result = new (Checksum checksum, T asset)[checksums.Count]; |
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 intermediary alloc!
@@ -218,14 +218,15 @@ static SourceGeneratorExecutionVersionMap FilterToProjectCone(SourceGeneratorExe | |||
using var _5 = PooledHashSet<Checksum>.GetInstance(out var newChecksumsToSync); | |||
newChecksumsToSync.AddRange(newProjectIdToChecksum.Values); | |||
|
|||
var newProjectStateChecksums = await _assetProvider.GetAssetsAsync<ProjectStateChecksums>( | |||
assetPath: AssetPath.SolutionAndTopLevelProjectsOnly, newChecksumsToSync, 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.
instead of having to allocate into this array, just to copy those all out and put into a dictionary below, we can now just read things right into the dictionary itself :)
@@ -503,14 +504,15 @@ private async Task<Project> UpdateProjectInfoAsync(Project project, Checksum inf | |||
using var _5 = PooledHashSet<Checksum>.GetInstance(out var newChecksumsToSync); | |||
newChecksumsToSync.AddRange(newDocumentIdToChecksum.Values); | |||
|
|||
var documentStateChecksums = await _assetProvider.GetAssetsAsync<DocumentStateChecksums>( |
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.
same here.
@ToddGrun ptal :) |
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Runtime.InteropServices; |
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.
not needed
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.
@jasonmalinowski For review when you get back. |
This moves to removing even more intermediary allocs, as well as allowing our lambdas to be static through the use of an
TArg
value passed along.