Implementation of Readable iteration helpers that does not rely on Async Iteration

[lukiano]

Hi, I'd like to offer an alternative implementation of Readable iteration helpers like find() that don't rely on for await (...) to consume items from the stream. This results in improved performance when the data is already available, such as when creating a Readable from an array, as shown in the attached screenshot (executed on a MacBook M2 Max).

The main con is increased duplication and code complexity. My understanding is that these helpers are still in the experimental phase, which I hope makes a change like this easier to accept.

There are many commits in the branch, but I'll squash them before merging.
Edit: I merged them to fix CI issues

The affected helpers are

• map()
• filter()
• reduce()
• find()
• toArray()
• some()
• every()
• drop()

Unaffected helpers:

• flatmap()
• take()

There's also a change to the from() method that increases the buffer watermark when the source is an array of data.
At first, I coded the changes manually, but eventually I had assistance from AI to keep backward compatibility. Still, there's a small break as can be seen in the updated test in test/parallel/test-stream-reduce.js.

The file benchmark/streams/operator-throughput.js allows interested parties to run the benchmarks on their computers or modify them to try other scenarios. I can remove it before merging.

[nodejs-github-bot]

Review requested:

• @nodejs/performance
• @nodejs/streams

[codecov]

Codecov Report

❌ Patch coverage is 92.99065% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.15%. Comparing base (9ad097b) to head (df55497).
⚠️ Report is 249 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/streams/operators.js	92.90%	43 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64429      +/-   ##
==========================================
- Coverage   90.33%   90.15%   -0.19%     
==========================================
  Files         751      751              
  Lines      250341   254051    +3710     
  Branches    47322    47897     +575     
==========================================
+ Hits       226145   229037    +2892     
- Misses      15575    16276     +701     
- Partials     8621     8738     +117     

Files with missing lines	Coverage Δ
lib/internal/streams/from.js	91.55% <100.00%> (+4.79%)	⬆️
lib/internal/streams/operators.js	94.11% <92.90%> (-2.04%)	⬇️

... and 160 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[lukiano]

I'm seeing #64447, which may change the measured performance of the current implementation.

[lukiano]

macbook-stream-operator-throughput-report.pdf
wsl2-stream-operator-throughput-report.pdf
linux-server-stream-operator-throughput-report.pdf
I've run the performance comparisons again with the latest changes from the main branch. macbook is the same M2 Max MacBook as before, linux-server is a 10900KF Intel CPU running on Debian 13, and wsl2 is a Windows machine with an AMD 9900x processor, and the tests ran within a WSL2 with Ubuntu 24.04

[lukiano]

@nodejs/streams I know you guys have probably 20 million PRs to review but if you could add this one to the queue I'd really appreciate it :)

[ovflowd]

The changes on streams/operators are quite significant, making the review of the code much harder. It'd be helpful if you could reorganize your changes in a way that minimizes diff. Or decouple current behavior from the added behavior of "data already being available"

My main concern is due to the nature of the diff, it is hard to see/visualize what actually was changed in terms of behavior/logic versus what is just moving code around. I'm not an expert on the streams implementation within node, but possibly my peers would argue the same.

Do you believe there any improvements you can do to the current diff?

[lukiano]

The changes on streams/operators are quite significant, making the review of the code much harder. It'd be helpful if you could reorganize your changes in a way that minimizes diff. Or decouple current behavior from the added behavior of "data already being available"

My main concern is due to the nature of the diff, it is hard to see/visualize what actually was changed in terms of behavior/logic versus what is just moving code around. I'm not an expert on the streams implementation within node, but possibly my peers would argue the same.

Do you believe there any improvements you can do to the current diff?

@ovflowd thanks for taking a look at the code. I could split the changes in from.js and the updated tests into a different PR, if that would help minimize the amount of code changes in this one.

In terms of the logic inside operators.js, it's not easy to view this as a diff because the previous logic almost entirely relied on createAsyncIterator which is in a different file readable.js and the asynchronous iteration engine that v8 provides. The new proposal is an implementation from scratch that keeps the same compatibility evaluated by the tests, but with improved performance. It's better to see both as black boxes implementations rather than trying to diff.

I could also split the changes into a few sections (maybe different commits?):

Functions that return a stream:

• changes to drop, map, filter (drop depends on filter and filter depends on map).

I reiterate that flatMap and take have not been changed by this PR.

Functions that return a value:

• changes to forEach, some, every, find. (the first two rely on the last)
• changes to reduce
• changes to toArray