Skip to content

Commit dbffba8

Browse files
littledivylittledivy
authored
perf(snapshot): guard against lazy modules leaking into eager snapshot (#35332)
## Why #34450 moved the node-polyfill closure (`process` / `stream` / `net` / `tty` / `require` / the ~118 polyfill modules) out of the eager snapshot into `lazy_loaded_esm`, so non-node `deno run` / `eval` no longer deserialize it at startup (~28–31% faster on the common paths). Nothing guards that win. The closure stays lazy only as long as no eager `esm` entry — and nothing statically imported from one — references it. A single such import silently pulls a closure module back into the eager graph and regresses empty/ESM startup, with **no test going red**. It's a quiet, easy-to-reintroduce regression. ## What Pin the exact set of `lazy_loaded_*` specifiers that are legitimately consumed into the eager snapshot. The snapshot builder already reports them as `consumed_lazy_specifiers`; this asserts, at build time, that the consumed set is a subset of `EXPECTED_CONSUMED`. - Any lazy module **newly** reaching the eager graph fails the build, naming the offender at single-module granularity (well before a coarse size/count threshold would notice a partial leak). - Removals — a module *becoming* lazy, i.e. an improvement — are allowed. - The error message points at `ext/node/lib.rs` (the usual cause) and explains how to update the list for an intentional eager addition. Build-time only, zero runtime cost. ## Test - Builds clean on `main` (consumed set == `EXPECTED_CONSUMED`). - Negative test: removing an entry from the allowlist (simulating a leak) fails the build with `lazy module(s) newly pulled into the eager snapshot: ["…"]`.
1 parent fb197b5 commit dbffba8

1 file changed

Lines changed: 112 additions & 0 deletions

File tree

‎cli/snapshot/build.rs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ fn create_cli_snapshot(
4949
.map(String::as_str)
5050
.collect();
5151

52+
assert_consumed_set_unchanged(&consumed);
53+
5254
let out_dir = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap());
5355
let residual_sources_dir = out_dir.join("residual_sources");
5456
std::fs::create_dir_all(&residual_sources_dir).unwrap();
@@ -108,6 +110,116 @@ fn create_cli_snapshot(
108110
write_residual_table(&mut f, &out_dir, "RESIDUAL_LAZY_ESM", &residual_esm);
109111
}
110112

113+
/// The exact set of `lazy_loaded_*` specifiers that are legitimately pulled
114+
/// into the eager snapshot, i.e. statically reachable from an eager `esm`
115+
/// module. Captured at the time PR #34450 landed. The heavy node-polyfill
116+
/// closure (process / stream / net / tty / require / the ~118 polyfill modules)
117+
/// is deliberately *absent* — it is `lazy_loaded_esm`, deserialized only on
118+
/// first `node:*` use, so non-node `deno run` / `eval` never pay for it.
119+
///
120+
/// Anything reaching the eager graph that is NOT in this list regresses
121+
/// empty/ESM startup (each leaked module adds snapshot deserialization the
122+
/// common path can't avoid). `assert_consumed_set_unchanged` enforces it.
123+
///
124+
/// To update: run a build, take the `consumed_lazy_specifiers` reported by
125+
/// `create_runtime_snapshot`, and reconcile here — but first confirm the new
126+
/// entry is *meant* to be eager. A node-closure module showing up almost
127+
/// always means an eager `esm` entry (or a static import from one) is dragging
128+
/// the polyfill closure into startup; fix that in `ext/node/lib.rs` rather than
129+
/// widening this list. See PR #34450.
130+
#[cfg(not(feature = "disable"))]
131+
const EXPECTED_CONSUMED: &[&str] = &[
132+
"ext:deno_crypto/00_crypto.js",
133+
"ext:deno_fetch/22_http_client.js",
134+
"ext:deno_ffi/00_ffi.js",
135+
"ext:deno_fs/30_fs.js",
136+
"ext:deno_io/12_io.js",
137+
"ext:deno_net/01_net.js",
138+
"ext:deno_net/02_tls.js",
139+
"ext:deno_node/02_register_cloneable.js",
140+
"ext:deno_node/_util/os.ts",
141+
"ext:deno_node/internal/async_hooks.ts",
142+
"ext:deno_node/internal/buffer.mjs",
143+
"ext:deno_node/internal/crypto/_keys.ts",
144+
"ext:deno_node/internal/crypto/constants.ts",
145+
"ext:deno_node/internal/error_codes.ts",
146+
"ext:deno_node/internal/errors.ts",
147+
"ext:deno_node/internal/hide_stack_frames.ts",
148+
"ext:deno_node/internal/normalize_encoding.ts",
149+
"ext:deno_node/internal/options.ts",
150+
"ext:deno_node/internal/primordials.mjs",
151+
"ext:deno_node/internal/timers.mjs",
152+
"ext:deno_node/internal/util.mjs",
153+
"ext:deno_node/internal/util/colorize.mjs",
154+
"ext:deno_node/internal/util/inspect.mjs",
155+
"ext:deno_node/internal/util/types.ts",
156+
"ext:deno_node/internal/validators.mjs",
157+
"ext:deno_node/internal_binding/_libuv_winerror.ts",
158+
"ext:deno_node/internal_binding/_node.ts",
159+
"ext:deno_node/internal_binding/_utils.ts",
160+
"ext:deno_node/internal_binding/async_wrap.ts",
161+
"ext:deno_node/internal_binding/buffer.ts",
162+
"ext:deno_node/internal_binding/constants.ts",
163+
"ext:deno_node/internal_binding/node_options.ts",
164+
"ext:deno_node/internal_binding/string_decoder.ts",
165+
"ext:deno_node/internal_binding/symbols.ts",
166+
"ext:deno_node/internal_binding/types.ts",
167+
"ext:deno_node/internal_binding/util.ts",
168+
"ext:deno_node/internal_binding/uv.ts",
169+
"ext:deno_node/timers.ts",
170+
"ext:deno_os/30_os.js",
171+
"ext:deno_os/40_signals.js",
172+
"ext:deno_web/00_infra.js",
173+
"ext:deno_web/00_url.js",
174+
"ext:deno_web/01_console.js",
175+
"ext:deno_web/01_dom_exception.js",
176+
"ext:deno_web/02_event.js",
177+
"ext:deno_web/02_timers.js",
178+
"ext:deno_web/03_abort_signal.js",
179+
"ext:deno_web/04_global_interfaces.js",
180+
"ext:deno_web/05_base64.js",
181+
"ext:deno_web/08_text_encoding.js",
182+
"ext:deno_web/09_file.js",
183+
"ext:deno_web/12_location.js",
184+
"ext:deno_web/13_message_port.js",
185+
"ext:deno_web/15_performance.js",
186+
"ext:deno_webgpu/00_init.js",
187+
"ext:deno_webidl/00_webidl.js",
188+
"ext:runtime/01_errors.js",
189+
"ext:runtime/01_version.ts",
190+
"ext:runtime/06_util.js",
191+
"ext:runtime/10_permissions.js",
192+
"ext:runtime/11_workers.js",
193+
"ext:runtime/40_fs_events.js",
194+
"ext:runtime/40_tty.js",
195+
"node:buffer",
196+
"node:timers",
197+
];
198+
199+
/// Startup guardrail (see PR #34450). Asserts the set of `lazy_loaded_*`
200+
/// modules consumed into the eager snapshot is exactly `EXPECTED_CONSUMED`.
201+
/// Catches a single lazy module silently leaking into the eager graph — the
202+
/// failure mode that regresses empty/ESM startup at fine granularity, well
203+
/// before a coarse count threshold would notice. Removals (a module becoming
204+
/// lazy — an improvement) are allowed; additions fail the build.
205+
#[cfg(not(feature = "disable"))]
206+
fn assert_consumed_set_unchanged(consumed: &std::collections::HashSet<&str>) {
207+
let expected: std::collections::HashSet<&str> =
208+
EXPECTED_CONSUMED.iter().copied().collect();
209+
let mut leaked: Vec<&str> = consumed.difference(&expected).copied().collect();
210+
leaked.sort_unstable();
211+
assert!(
212+
leaked.is_empty(),
213+
"lazy module(s) newly pulled into the eager snapshot: {leaked:?}.\nEach \
214+
adds snapshot deserialization to the common `deno run` / `eval` startup \
215+
path. If a `node:*` / `ext:deno_node/*` module leaked, an eager `esm` \
216+
entry or a static import from an eager module is dragging the node \
217+
polyfill closure into startup — fix it in ext/node/lib.rs. If the entry \
218+
is genuinely meant to be eager, add it to EXPECTED_CONSUMED. See \
219+
PR #34450."
220+
);
221+
}
222+
111223
#[cfg(not(feature = "disable"))]
112224
fn transpile_residual_source(
113225
out_dir: &std::path::Path,

0 commit comments

Comments
 (0)