Pool-bound forEach for Stream and Iterable #6765
Conversation
isoos
commented
Jun 15, 2023
isoos
commented
- replaced most Pool use in app/
- also limiting the number of pending operations in the pool
| await Future.wait(futures); | ||
| futures.clear(); | ||
| await recentlyUpdated.entries.boundedForEach(concurrency, (e) async { | ||
| await updatePackage(e.key, e.value); |
There was a problem hiding this comment.
| await updatePackage(e.key, e.value); | |
| if (!claim.valid) { | |
| return; | |
| } | |
| await updatePackage(e.key, e.value); |
| (package) async { | ||
| if (package.isNotVisible) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| if (!claim.valid) { | |
| return; | |
| } |
| // early exit on the first error | ||
| if (firstError != null) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Are you sure we want to do this?
All of the code you're replacing follows a variant of running a loop with:
final f = pool.withResource(() => /* do something... */);
futures.add(f);And then doing Future.wait(futures).
You existing loops won't exit early. If we want to change semantics in every place where we do a loop like this, I'm down for hearing the arguments. But I think it might be different on a case by case basis.
In package:async I'm leaning towards onError which defaults to a function that throws the error.
But then all our invocations would be like:
await page!.items.boundedForEach(concurrency ?? 1, (entry) async {
final deleted = await deleteFromBucket(bucket, entry.name);
if (deleted) count++;
}, onError: (e, st) => null);Which gets formatted something like:
await page!.items.boundedForEach(
concurrency ?? 1,
(entry) async {
final deleted = await deleteFromBucket(bucket, entry.name);
if (deleted) count++;
},
onError: (e, st) => null,
);with trailing comma.
Where you currently have:
await page!.items.boundedForEach(concurrency ?? 1, (entry) async {
final deleted = await deleteFromBucket(bucket, entry.name);
if (deleted) count++;
});But this assumes we default to not exit early on errors (assuming we want to preserve semantics).
|
I think we could close this in favor of dart-lang/async#249 |
|
@isoos yes, but feel free to just copy paste from dart-lang/async#249, it might be a while before it lands :D |
