stream: allow infinite concurrency #48588
Conversation
|
Review requested:
|
|
the macOS failed test is unrelated: |
| { | ||
| // Allow Infinite concurrency | ||
| const stream = Readable.from([1, 2]).map(async (item, { signal }) => { | ||
| await setTimeout(10 - item, { signal }); |
There was a problem hiding this comment.
I am not sure this asserts concurrency works
There was a problem hiding this comment.
The test meant to make sure that concurrency: Infinity does not throw an error, but you are right, it does not test that concurrency works, fixed it
There was a problem hiding this comment.
Why do we need a timer here? Can't this just be something like:
let count = 0; // or t.mock
Readable.from(Array(1000).fill()).map(() => {
count++;
return new Promise(() => {});
}, { concurrency: Infinity }).toArray().catch(common.mustNotCall());
await setImmediate(); // from timers/promises
strictEqual(count, 1000);
|
@ronag I'm not sure about this wdyt? |
|
How is it different than allowing Number.MAX_SAFE_INTEGER? |
@mcollina we can log a warning in case the array reaches some size like what's happening when having a lot of listeners to event emitter |
then why add this option in the first place? |
adding this so we can use it in:
which is needed as we don't want to wait for the prev tests to finish before starting a new one (same issue as in #46132)
This has a pitfall in using
concurrency: Infinityon infinite stream