Switch to a callback style in the asset finding code #73017
Conversation
| @@ -58,7 +58,7 @@ public void AddAllTo(HashSet<Checksum> checksums) | |||
| AssetPath assetPath, | |||
| TextDocumentStates<TState> documentStates, | |||
| HashSet<Checksum> searchingChecksumsLeft, | |||
| Dictionary<Checksum, object> result, | |||
| Action<Checksum, object> onAssetFound, | |||
There was a problem hiding this comment.
instead of passing in dictionaries, we pass in callbacks to invoke when the item is found. In this Pr, this will still end up populating a dictionary. However in #73013 this changes to populating a channel that then allows another task to be reading from the channle when items are found, writing the assets out to our pipe between oop and the host.
| await FindAssetsAsync(assetPath, checksumsToFind, assetMap, cancellationToken).ConfigureAwait(false); | ||
| await FindAssetsAsync( | ||
| assetPath, checksumsToFind, | ||
| (checksum, asset) => assetMap[checksum] = asset, |
There was a problem hiding this comment.
Containing PR gets rid of the closure allocation (or will once #73013 (comment) is addressed)
| using var _ = Creator.CreateResultMap(out var resultPool); | ||
|
|
||
| await scope.FindAssetsAsync(AssetPath.FullLookupForTesting, checksumPool.Object, resultPool, cancellationToken).ConfigureAwait(false); | ||
| var checksums = new ReadOnlyMemory<Checksum>([checksum]); | ||
| await scope.AddAssetsAsync(AssetPath.FullLookupForTesting, checksums, resultPool, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
It's a test call and was cleaner. Just goes through the to level path without having to make a set and make a lambda.
| @@ -97,10 +100,10 @@ public async ValueTask<object> GetAssetAsync(Checksum checksum, CancellationToke | |||
| { | |||
| Contract.ThrowIfTrue(checksum == Checksum.Null); | |||
|
|
|||
| using var checksumPool = Creator.CreateChecksumSet(checksum); | |||
|
Nice, I like where this is headed! |
|
@jasonmalinowski For review when you get back. |
Extracted out of #73013.