lib: load fewer builtins when bootstrapping without a snapshot - #65329
lib: load fewer builtins when bootstrapping without a snapshot#65329codebytere wants to merge 1 commit into
Conversation
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65329 +/- ##
==========================================
- Coverage 90.13% 90.09% -0.05%
==========================================
Files 752 752
Lines 251568 252348 +780
Branches 47270 47462 +192
==========================================
+ Hits 226759 227355 +596
- Misses 16168 16292 +124
- Partials 8641 8701 +60
🚀 New features to boost your workflow:
|
5d49228 to
088f769
Compare
|
Benchmark GHA (misc / startup-core): https://github.com/nodejs/node/actions/runs/32009241408 Results
Benchmark results:
|
|
The modified test is failing on Windows, indicating this PR is adding flakiness. Can you have a look? |
088f769 to
0d3b8a7
Compare
|
@aduh95 fixed! |
0d3b8a7 to
f95b3c7
Compare
|
@mcollina mind re-reviewing? |
Contexts that are not deserialized from the built-in snapshot -- worker threads, and the main context of embedders that create their own isolate or of `node --no-node-snapshot` -- compile (with the code cache at best) every builtin the bootstrap touches, so each eagerly required builtin is startup time (~0.15-0.4 ms apiece). A number of them are only required eagerly so that they end up in the snapshot, or for features the bootstrap path never uses. Load lazily what those paths do not need: - is_main_thread.js: preload util, url, the ESM loader (translators, resolver, module_job/map, source maps, node:module, vm modules, mime, data_url, the TypeScript stripper), internal/blob and internal/dns/utils only while building a snapshot; they load on first use otherwise. - fs: internal/blob (+ internal/encoding and its tables) is only used by fs.openAsBlob(). - internal/url: internal/data_url (+ internal/mime) is only used by the Buffer-returning file URL helpers. - internal/process/execution, the CommonJS loader, esm/translators and esm/load: the TypeScript stripper and data: URL helpers are only needed for TypeScript sources / data: URLs. - pre_execution: internal/dns/utils (+ internal/net) is only needed up front to validate an explicit --dns-result-order or to register the resolver's snapshot serializer; the default order becomes the variable's initializer. - internal/worker: event_loop_utilization and error_serdes are only needed once a sub-worker's ELU is read or it reports an error. - worker_threads: `locks` is defined lazily, like util's lazy exports. Main-thread startup with the snapshot is unchanged (the same modules are preloaded into it; the bootstrap-modules test lists are adjusted). A bare worker compiles 95 -> 83 builtins (cold start -5%); without the snapshot an empty CommonJS entry point compiles 76 -> 59 builtins and an empty ES module entry point 76 -> 69. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
f95b3c7 to
0cc532a
Compare
|
I am in favor of "for snapshot less mode, try not to eager load that much" in general, although some of the removals seem somewhat too conservative (e.g. the url and util modules are commonly used by most applications) and may only favor the startup of empty scripts but penalise the startup of a real application that actually does something. So I think we should check the performance impact of the scripts that does something beyond being empty as well. If the number Comes from that scenario then LGTM, otherwise can you run the benchmark across the startup-core and startup-cli-version data sets? |
Worker startup gets ~6 % faster and a snapshot-less main-thread bootstrap (embedders that create their own isolate,
--no-node-snapshot) ~10 % faster, by not eagerly loading builtins those paths never use. Startup with the snapshot is unchanged.Builtins compiled: bare worker 95 → 83;
--no-node-snapshotempty CJS entry 76 → 59, empty ESM entry 76 → 69.Contexts that aren't deserialized from the snapshot compile every builtin the bootstrap touches, so each eager
requirethere is startup time (~0.15–0.4 ms apiece). Several are eager only so that they land in the snapshot, or for features the bootstrap doesn't use. This makes them lazy without changing the snapshot's contents:is_main_thread.js:util,url, the ESM loader chain,internal/blobandinternal/dns/utilsare preloaded onlyif (isBuildingSnapshot()); otherwise they load on first use.fs→internal/blob: only forfs.openAsBlob().internal/url→internal/data_url: only for the Buffer-returning file-URL helpers.execution, cjs loader,esm/translators,esm/load→ TypeScript stripper /data:helpers: only for those inputs.pre_execution→internal/dns/utils: only for an explicit--dns-result-order(still validated at startup) or a snapshot build;'verbatim'becomes the variable's initializer, so a snapshot-timesetDefaultResultOrder()still survives deserialization.internal/worker→ ELU /error_serdeson demand;worker_threads.locksviadefineLazyProperties(asutildoes).An intermediate version that didn't re-add these to the snapshot regressed
node empty.mjsby 2–4 %, which is why theisBuildingSnapshot()block lists them explicitly.test-bootstrap-modulesis adjusted for the worker-side list.Tests:
test-bootstrap-modulesplus worker, url, fs, dns, process, cli, vm, snapshot, blob, esm, inspector, module, util, test-runner and single-executable suites pass.Disclosure: the code, test, measurements and this description were written by Claude Code, directed and reviewed by @codebytere.