Skip to content

fix(core): externalize lazy loaded sources#34936

Merged
nathanwhit merged 2 commits into
denoland:mainfrom
nathanwhit:fix-dirty-heap
Jun 5, 2026
Merged

fix(core): externalize lazy loaded sources#34936
nathanwhit merged 2 commits into
denoland:mainfrom
nathanwhit:fix-dirty-heap

Conversation

@nathanwhit

Copy link
Copy Markdown
Member

Problem

lazy_loaded_js extension scripts (the ones loaded via loadExtScript / compile_function) were being baked
into 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:

  • the isolate blob carried a full copy of each lazy script's source text, and
  • deserialization had to materialize and fix up those string objects on every startup.

This is wasteful: the source text is immutable and already lives in the binary as a &'static string (via
include_str!). It never needs to be a heap-allocated, snapshot-serialized V8 string.

Fix

Externalize lazy_loaded_js sources so they stay off the V8 heap, backed directly by the static string in
the binary:

  1. Pre-wrap at build time. loadExtScript evaluates each source as the body of a compile_function call
    ("use strict"; return (<IIFE>);). That wrapping is now done once at snapshot-build time
    (wrap_lazy_ext_script in cli/snapshot/build.rs + extension_set.rs) so the stored source is the exact
    &'static string V8 compiles — no per-script owned copy. map.rs detects the wrapped prefix and hands V8 the
    external string directly; sources that arrive unwrapped (e.g. consumed during snapshot creation) still get
    wrapped at runtime as a fallback.

  2. Externalize only when building the snapshot. externalize_sources now externalizes lazy_loaded_js
    (in addition to js + esm + lazy_esm) only under will_snapshot. At runtime these stay as static
    registrations; consumed ones re-link to the snapshot's external backing. The source/external-reference counts
    in externalize_sources and jsruntime.rs are kept exactly in sync — a mismatch leaves uninitialized
    external-reference slots that shift snapshot indices and miscompile JIT'd code that references them.

  3. 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

  • ~46% smaller isolate blob (the dirty source copies are gone).
  • ~0.6 ms faster cold startup (less deserialization fixup), measured locally with
    --v8-flags=--profile-deserialization.
  • No behavior change: lazy scripts compile and run identically (strict-mode body, IIFE completion value
    preserved).

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Careful, well-documented change — the hard invariants all check out:

  • Count sync between externalize_sources (sources_count) and jsruntime.rs (source_count): both now add lazy_js.len() under the same will_snapshot guard.
  • Trailing-order assumption in snapshot() (lazy_js_start = original_sources.len() - lazy_js_specifiers.len()) holds because you iterate an explicit js -> esm -> lazy_esm -> lazy_js chain and collect lazy_js_specifiers in that same order; no underflow when there are zero lazy scripts.
  • Prefix detection in map.rs won't false-positive on unwrapped IIFE sources (those start "use strict";\n(function, never "use strict"; return ().

Blocking

  • lint debug windows-x86_64 is 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

  1. 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 one fn externalized_source_count(&LoadedSources, externalize_lazy_js) used by both call sites would make the invariant unbreakable instead of comment-enforced.
  2. Share the wrap prefix. wrap_lazy_ext_script's format!("\"use strict\"; return (...)") and the starts_with("\"use strict\"; return (") detector are coupled by a copy-pasted literal — a shared const LAZY_WRAP_PREFIX: &str referenced by both removes the silent-drift risk.
  3. 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.

@nathanwhit
nathanwhit merged commit 24df47a into denoland:main Jun 5, 2026
267 of 270 checks passed
@nathanwhit
nathanwhit deleted the fix-dirty-heap branch June 5, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants