Skip to content

worker: preload streams into startup snapshot - #66311

Open
mcollina wants to merge 3 commits into
nodejs:mainfrom
mcollina:worker-snapshot-streams
Open

mcollina wants to merge 3 commits into
nodejs:mainfrom
mcollina:worker-snapshot-streams

Conversation

@mcollina

Copy link
Copy Markdown
Member

Worker stdio loads stream after restoring the main-thread context, initializing its constructors and prototype methods on every startup. Preload stream during snapshot construction so workers can restore that initialized state instead. Worker-specific stdio instances and message ports are still created at runtime.

Add a concurrent startup benchmark and verify snapshot membership, stdio methods, and isolation from parent-thread stream mutations.


AI generated, reviewed by me.

Worker stdio loads stream after restoring the main-thread context,
initializing its constructors and prototype methods on every startup.
Preload stream during snapshot construction so workers can restore that
initialized state instead. Worker-specific stdio instances and message
ports are still created at runtime.

Add a concurrent startup benchmark and verify snapshot membership,
stdio methods, and isolation from parent-thread stream mutations.

Assisted-by: pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 26, 2026
Keep Readable and Writable eagerly initialized for worker stdio, but
load operators, pipeline, compose, promises, and the other stream
constructors on demand. This keeps their implementation state out of
the startup snapshot when it is not needed.

Preserve export identity, prototype descriptors, and promisification.
Install subclass methods before linking prototype chains so lazy loading
also works when a base constructor or prototype has been frozen.

Update the bootstrap inventory and add lazy-loading compatibility tests.

Assisted-by: pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina

Copy link
Copy Markdown
Member Author

Added lazy stream initialization in 1a9691a. Readable and Writable remain eagerly initialized for worker stdio; operators, pipeline, compose, promises, and the other stream constructors load on demand.

Compared the combined changes at 1a9691a against the original baseline, 4889fb0, on an Intel i7-7700 (4 cores / 8 hardware threads), using release builds with identical configuration. Percentages are changes in mean throughput; positive means faster.

For benchmark/worker/startup.js, 100 samples per binary/configuration, followed by another 100 with binary order reversed:

Concurrent workers Throughput change Reversed-order confirmation
1 +12.40% ±1.82% +12.86%
4 +10.70% ±3.47% +8.21%
8 +7.73% ±1.59% +7.28%
16 +8.34% ±1.22% +9.34%

This measures time until all workers are online, keeping workers alive until then so teardown does not compete with startup. The ± values are the harness's unadjusted 95% accuracy estimates. All four gains survive Holm correction at 5%, in both orders.

Full-lifecycle results from benchmark/misc/startup-core.js:

Workload Single process Worker
Empty CommonJS −0.41% ±0.68% (not significant) +10.45% ±0.59%
Empty ESM −1.61% ±0.71% +10.47% ±0.40%
Require builtins +1.09% ±0.86% +4.22% ±0.76%
Import builtins +1.03% ±0.84% +1.90% ±0.71%
Immediate pipeline() plus .map() +10.88% ±0.53% +23.70% ±0.48%

Empty and immediate-stream-use cases used 60 samples per binary/mode, with 30 starts per sample. Builtin-loading cases used 40 samples with 10 starts per sample. All differences except empty CommonJS process startup survive Holm correction across the 14 primary comparisons, including the worker-online cases.

There is a small regression for empty ESM processes: about 0.45 ms per lifecycle. A separate 100-sample run with reversed binary order reproduced it at −1.49%. The measured builtin-loading and stream-use workloads improved, but these results do not establish a universal no-regression claim.

The immediate-stream-use fixture was:

'use strict';
const { Readable, Writable, pipeline } = require('node:stream');
pipeline(
  Readable.from([1, 2, 3]).map((value) => value * 2),
  new Writable({ objectMode: true, write(value, encoding, callback) { callback(); } }),
  (error) => { if (error) throw error; },
);

Worker-online comparison command, with binaries built at the two commits:

./node benchmark/compare.js --old /path/to/baseline --new /path/to/candidate \
  --runs 100 --filter startup --no-progress --analyze worker

@mcollina
mcollina marked this pull request as ready for review September 26, 2026 15:18
@mcollina
mcollina requested a review from codebytere September 26, 2026 15:18
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.45455% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (4889fb0) to head (a57a1e2).
⚠️ Report is 96 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/bootstrap/switches/is_main_thread.js 0.00% 3 Missing ⚠️
src/node_util.cc 87.50% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66311      +/-   ##
==========================================
- Coverage   90.37%   90.36%   -0.02%     
==========================================
  Files         790      792       +2     
  Lines      274497   275599    +1102     
  Branches    52562    52830     +268     
==========================================
+ Hits       248077   249040     +963     
- Misses      16921    16987      +66     
- Partials     9499     9572      +73     
Files with missing lines Coverage Δ
lib/internal/streams/duplex.js 100.00% <100.00%> (ø)
lib/internal/streams/operators.js 95.77% <100.00%> (+0.37%) ⬆️
lib/internal/streams/passthrough.js 100.00% <100.00%> (ø)
lib/internal/streams/pipeline.js 95.24% <100.00%> (+0.12%) ⬆️
lib/internal/streams/transform.js 98.54% <100.00%> (+0.01%) ⬆️
lib/stream.js 100.00% <100.00%> (ø)
lib/stream/promises.js 100.00% <100.00%> (ø)
src/node_util.cc 82.40% <87.50%> (+0.43%) ⬆️
lib/internal/bootstrap/switches/is_main_thread.js 89.85% <0.00%> (-0.82%) ⬇️

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

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 29, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 30, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 30, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Allow defineLazyProperties to map property names directly to module IDs
so stream can expose function exports without a separate lazy module.
Keep only module IDs in native callback data for snapshot serialization.

Set the mapping object's prototype to null after creation to retain
fast properties without inheriting from Object.prototype.

Assisted-by: pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants