fix(core): externalize lazy loaded sources#34936
Merged
Merged
Conversation
bartlomieju
reviewed
Jun 5, 2026
bartlomieju
left a comment
Member
There was a problem hiding this comment.
Careful, well-documented change — the hard invariants all check out:
- Count sync between
externalize_sources(sources_count) andjsruntime.rs(source_count): both now addlazy_js.len()under the samewill_snapshotguard. - Trailing-order assumption in
snapshot()(lazy_js_start = original_sources.len() - lazy_js_specifiers.len()) holds because you iterate an explicitjs -> esm -> lazy_esm -> lazy_jschain and collectlazy_js_specifiersin that same order; no underflow when there are zero lazy scripts. - Prefix detection in
map.rswon't false-positive on unwrapped IIFE sources (those start"use strict";\n(function, never"use strict"; return ().
Blocking
lint debug windows-x86_64is failing (exit 1; run still in progress so full logs aren't up yet). It passed on linux and nothing in the diff is Windows-specific, so it may be flaky/infra — but it needs to be green before merge. Can you check the job and re-run if transient?
Non-blocking suggestions
- Unify the duplicated count formula. The
js + esm + lazy_esm + (lazy_js if will_snapshot)count now lives in two files and must stay byte-identical or the snapshot miscompiles. Extracting onefn externalized_source_count(&LoadedSources, externalize_lazy_js)used by both call sites would make the invariant unbreakable instead of comment-enforced. - Share the wrap prefix.
wrap_lazy_ext_script'sformat!("\"use strict\"; return (...)")and thestarts_with("\"use strict\"; return (")detector are coupled by a copy-pasted literal — a sharedconst LAZY_WRAP_PREFIX: &strreferenced by both removes the silent-drift risk. - Tests. No test added; correctness currently rests on existing integration tests (node polyfills exercise lazy scripts) catching a miscompile. That's real but indirect — a
debug_assert_eq!tying the two counts together (folds into #1) would be a cheap explicit guard.
Implementation looks ready pending green lint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
lazy_loaded_jsextension scripts (the ones loaded vialoadExtScript/compile_function) were being bakedinto the V8 startup snapshot as owned source-string copies on the V8 heap. Every one of these sources
landed in mutable ("dirty") snapshot heap pages, which means:
This is wasteful: the source text is immutable and already lives in the binary as a
&'staticstring (viainclude_str!). It never needs to be a heap-allocated, snapshot-serialized V8 string.Fix
Externalize
lazy_loaded_jssources so they stay off the V8 heap, backed directly by the static string inthe binary:
Pre-wrap at build time.
loadExtScriptevaluates each source as the body of acompile_functioncall(
"use strict"; return (<IIFE>);). That wrapping is now done once at snapshot-build time(
wrap_lazy_ext_scriptincli/snapshot/build.rs+extension_set.rs) so the stored source is the exact&'staticstring V8 compiles — no per-script owned copy.map.rsdetects the wrapped prefix and hands V8 theexternal string directly; sources that arrive unwrapped (e.g. consumed during snapshot creation) still get
wrapped at runtime as a fallback.
Externalize only when building the snapshot.
externalize_sourcesnow externalizeslazy_loaded_js(in addition to
js + esm + lazy_esm) only underwill_snapshot. At runtime these stay as staticregistrations; consumed ones re-link to the snapshot's external backing. The source/external-reference counts
in
externalize_sourcesandjsruntime.rsare kept exactly in sync — a mismatch leaves uninitializedexternal-reference slots that shift snapshot indices and miscompile JIT'd code that references them.
Don't duplicate residual sources. Only consumed lazy scripts are actually referenced by snapshotted
code. Non-consumed (residual) scripts are already shipped via the residual table, so persisting their bytes in
the snapshot sidecar would duplicate them. At serialize time we store empty bytes for non-consumed
lazy-script slots (keeping the external-reference index for alignment, since nothing references it).
Impact
--v8-flags=--profile-deserialization.preserved).