Skip to content

stream: replace bind with arrow function for onwrite callback#62087

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
thisalihassan:stream-writable-arrow-onwrite
Mar 11, 2026
Merged

stream: replace bind with arrow function for onwrite callback#62087
nodejs-github-bot merged 1 commit intonodejs:mainfrom
thisalihassan:stream-writable-arrow-onwrite

Conversation

@thisalihassan
Copy link
Contributor

Replace onwrite.bind(undefined, stream) with an arrow function.

Writable creation +29.92%
Duplex creation +19.91%

streams/creation.js kind='duplex' n=50000000 *** 19.91 % ±0.92% ±1.23% ±1.60%
streams/creation.js kind='readable' n=50000000 -0.53 % ±1.14% ±1.53% ±2.01%
streams/creation.js kind='transform' n=50000000 *** 5.06 % ±1.03% ±1.37% ±1.79%
streams/creation.js kind='writable' n=50000000 *** 29.92 % ±1.16% ±1.55% ±2.01%
streams/pipe-object-mode.js n=5000000 0.12 % ±1.16% ±1.55% ±2.04%
streams/pipe.js n=5000000 -0.12 % ±0.80% ±1.07% ±1.39%
streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=100000 1.82 % ±2.31% ±3.10% ±4.10%
streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=100000 -0.77 % ±1.88% ±2.50% ±3.26%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=100000 0.33 % ±1.24% ±1.66% ±2.17%
streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=100000 0.48 % ±1.48% ±1.97% ±2.56%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=100000 0.08 % ±0.73% ±0.97% ±1.26%
streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=100000 -2.24 % ±4.73% ±6.36% ±8.42%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=100000 -1.10 % ±1.66% ±2.20% ±2.87%
streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=100000 -0.78 % ±1.58% ±2.11% ±2.75%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=100000 -0.12 % ±0.83% ±1.11% ±1.45%
streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=100000 -0.61 % ±1.88% ±2.51% ±3.26%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=100000 -0.12 % ±0.68% ±0.91% ±1.18%
streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=100000 -1.37 % ±4.54% ±6.10% ±8.04%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=100000 3.08 % ±4.28% ±5.76% ±7.63%
streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=100000 0.18 % ±1.71% ±2.28% ±2.97%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=100000 1.83 % ±3.21% ±4.32% ±5.72%
streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=100000 0.00 % ±1.64% ±2.19% ±2.86%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 22 comparisons, you can thus
expect the following amount of false-positive results:
1.10 false positives, when considering a 5% risk acceptance (*, **, *),
0.22 false positives, when considering a 1% risk acceptance (
, ),
0.02 false positives, when considering a 0.1% risk acceptance (
)

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Mar 3, 2026
@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.65%. Comparing base (acb79bc) to head (fdc7712).
⚠️ Report is 70 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62087      +/-   ##
==========================================
- Coverage   89.65%   89.65%   -0.01%     
==========================================
  Files         676      676              
  Lines      206312   206325      +13     
  Branches    39522    39523       +1     
==========================================
+ Hits       184972   184977       +5     
- Misses      13469    13472       +3     
- Partials     7871     7876       +5     
Files with missing lines Coverage Δ
lib/internal/streams/writable.js 96.64% <100.00%> (ø)

... and 31 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.

@atlowChemi atlowChemi added the needs-benchmark-ci PR that need a benchmark CI run. label Mar 3, 2026
@MattiasBuelens MattiasBuelens added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 7, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 7, 2026
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has break potential, but very little.

@ronag @mcollina are you aware of cases where this can be problematic? (e.g. use as constructor)

@mcollina
Copy link
Member

mcollina commented Mar 8, 2026

zero

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@thisalihassan
Copy link
Contributor Author

should we merge this @mcollina @MattiasBuelens @benjamingr all CI have passed

@MattiasBuelens MattiasBuelens added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Mar 11, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 11, 2026
@nodejs-github-bot nodejs-github-bot merged commit e47df44 into nodejs:main Mar 11, 2026
83 of 84 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in e47df44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-benchmark-ci PR that need a benchmark CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants