Skip to content

perf(ext/web): convert all ext/web JS sources to lazy-loaded scripts#33760

Merged
bartlomieju merged 5 commits into
mainfrom
ext-web-lazy-scripts
May 2, 2026
Merged

perf(ext/web): convert all ext/web JS sources to lazy-loaded scripts#33760
bartlomieju merged 5 commits into
mainfrom
ext-web-lazy-scripts

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Summary

  • Converts all 24 ext/web JS files from ESM modules to IIFE-wrapped lazy-loaded scripts using core.loadExtScript(), building on the infrastructure from feat(core): add Deno.core.loadExtScript() for lazy-loaded scripts #33739
  • Updates all external consumers across ext/, runtime/, and cli/ from import to core.loadExtScript()
  • Replaces createLazyLoader() calls targeting ext:deno_web files with loadExtScript() since they are now plain scripts

Notes

  • import.meta.log in 06_streams.js replaced with core.print() as a temporary workaround since import.meta is not available in non-module scripts. A proper core.log equivalent may be needed.

Test plan

  • cargo build succeeds (snapshot creation passes)
  • Smoke tested: console, URL, TextEncoder/TextDecoder, ReadableStream, Blob, File, Performance, BroadcastChannel, MessagePort, CompressionStream, DOMException, timers
  • Full test suite

Convert all remaining ext/web JavaScript files from ESM modules to
IIFE-wrapped lazy-loaded scripts using `core.loadExtScript()`. This
builds on the initial `loadExtScript()` infrastructure from #33739
which converted 3 files.

All 24 ext/web JS files are now in `lazy_loaded_js` instead of `esm`,
avoiding V8's ES module infrastructure overhead (~31% snapshot size,
~4x serialization cost).

External consumers across ext/, runtime/, and cli/ updated from
`import` to `core.loadExtScript()`. All `createLazyLoader()` calls
referencing ext:deno_web files replaced with `loadExtScript()` since
the targets are now plain scripts, not ESM modules.
These files are loaded on-demand at runtime (not during snapshotting),
so they need ESM format to access core via import rather than
globalThis.__bootstrap which is deleted after bootstrap.

Also fixes dlint no-unused-vars in 13_message_port.js.
ops on core.ops are deleted at bootstrap by removeImportedOps().
Destructuring at load time captures the function reference before
deletion, matching how other IIFE scripts handle ops.
@bartlomieju
bartlomieju enabled auto-merge (squash) May 1, 2026 19:28

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Substance is interesting (converting all 24 ext/web JS files from ESM to lazy-loaded scripts on top of #33739) but two real concerns are blocking merge readiness.

Real V8 fatal in unit::canvas_test

test unit debug linux-x86_64 (and the same shard across all 8 debug platforms × release linux-x86_64) crashes the runtime — not just fails an assertion:

offscreenCanvasConstructor ... FAILED (1ms)
#
# Fatal error in v8::Module::GetModuleNamespace
# v8::Module::GetModuleNamespace must be used on an instantiated module
#

Deno should not have died with a signal follows. This is a hard-crash, not a test-output mismatch. The diagnostic points directly at the conversion: something downstream of the OffscreenCanvas constructor (the recently-landed #29357 surface) is calling v8::Module::GetModuleNamespace on what was a real ESM module before this PR but is now a non-module IIFE script. After the conversion, that V8-side namespace lookup has nothing to bind to.

Worth grepping the ext/canvas and ext/webgpu JS surface for any import.meta, core.getModuleNamespace, prepareLoad-style references to ext:deno_web modules — those are the call sites that need migrating in lockstep with the script conversion. The 9 CI failures all cascade from this one V8 abort (the 8 unit-debug shards plus wpt release linux-x86_64 which probably exercises the same OffscreenCanvas path).

Test plan checkbox is unchecked

The PR body's - [ ] Full test suite box is unchecked, and the smoke-test list in the body ("console, URL, TextEncoder/TextDecoder, ReadableStream, Blob, File, Performance, BroadcastChannel, MessagePort, CompressionStream, DOMException, timers") notably does not include OffscreenCanvas / canvas / webgpu — which is exactly where the regression surfaces. Worth running the full unit suite locally before pushing the next iteration; the V8 abort would have shown up immediately.

Minor: import.meta.log workaround acknowledged

The note in the PR body about import.meta.log in 06_streams.js being replaced with core.print() (because import.meta is module-only) is honest, and the suggestion that "a proper core.log equivalent may be needed" is the right framing. core.print writes to stdout/stderr without log-level routing, which is a regression for anything that relied on the implicit import.meta.log formatting. Worth landing the core.log shim in a precursor PR before this one merges, so the workaround can be undone.

Escalation

This is cross-cutting infrastructure — switching the entire ext/web JS surface from ESM to scripts, plus updating all external consumers across ext/, runtime/, and cli/. 79 files, 542+/394-. The performance benefit is real (lazy scripts skip the ESM module-graph instantiation overhead) but the blast radius is large enough that the V8 abort is unlikely to be the only place this breaks — every import.meta.* reference, every core.createLazyLoader / propNonEnumerableLazyLoaded call in the affected files needs auditing. Worth keeping in mind for the next pass.

Not blocking on substance — happy to re-review once the OffscreenCanvas crash is sorted and the full test suite is green.

09_file.js is now a lazy_loaded_js script, not an ESM module,
so it cannot be imported via static import.

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM at 9062c0d. The OffscreenCanvas V8 abort I'd flagged in my prior review is fixed cleanly:

// ext/canvas/01_canvas.js
-import { Blob } from "ext:deno_web/09_file.js";
+const { Blob } = core.loadExtScript("ext:deno_web/09_file.js");

Three-line fix that addresses the exact diagnosis from the prior review — 09_file.js is now a lazy_loaded_js script rather than an ESM module, so the static import was triggering v8::Module::GetModuleNamespace must be used on an instantiated module. Switching to core.loadExtScript with destructuring routes through the script-loading path that does work for the new shape.

Also noted the earlier intermediate fix bb10746 correctly carved out geometry.js and webtransport.js as lazy_loaded_esm rather than scripts, since those are loaded on-demand at runtime (not during snapshot) — that's the right scope decision and worth keeping.

CI 121/123 success at this head, 1 in-progress (canary), 1 skip, 0 fail. All 58 of the relevant lint debug / test unit / test node_compat / wpt shards across 4 platforms × debug+release are SUCCESS, including the previously-crashing unit::canvas_test.

Two follow-up items from my prior review carry forward as non-blocking notes for future iterations:

  • The PR body's - [ ] Full test suite checkbox is still unchecked, but CI is now in fact running it green — stale doc, not a blocker.
  • The import.meta.logcore.print() workaround in 06_streams.js:170 is still in place. The PR-body note ("A proper core.log equivalent may be needed") remains the right framing — worth landing a core.log shim in a follow-up so this workaround can be undone.

@bartlomieju
bartlomieju merged commit ec58158 into main May 2, 2026
389 of 396 checks passed
@bartlomieju
bartlomieju deleted the ext-web-lazy-scripts branch May 2, 2026 07:57
bartlomieju added a commit that referenced this pull request May 2, 2026
…scripts (#33779)

## Summary

Follow-up to #33760. Converts the ESM JS sources in `ext/io`, `ext/os`,
and `ext/net` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`),
eliminating ESM module resolution overhead.

These three extensions have no cross-extension ESM import dependencies
(only `core.loadExtScript` calls to already-converted extensions like
`ext/web`), so no `internals` bridges are needed.

Converting these also unblocks clean conversion of:
- `ext/fs` (depends on `ext/io`)
- `ext/fetch` (depends on `ext/net`)
divybot added a commit to divybot/deno that referenced this pull request May 2, 2026
Adapt enqueuePerformanceEntry/registerExtraEntryType to lazy-loaded
script pattern from denoland#33760: 15_performance.js now returns its exports
from an IIFE; consumers (perf_hooks.js, http2.ts) load via
core.loadExtScript instead of static ESM import.

Co-authored-by: Divy Srivastava <[email protected]>
bartlomieju added a commit that referenced this pull request May 3, 2026
Follow-up to #33760. Converts `ext/ffi/00_ffi.js` from ESM to an
IIFE-wrapped lazy-loaded script (`lazy_loaded_js`).

Signed-off-by: Bartek Iwańczuk <[email protected]>
bartlomieju added a commit that referenced this pull request May 3, 2026
Follow-up to #33760 and #33779. Converts all 8 ESM JS sources in
`ext/fetch` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`).

- `20_headers.js`, `21_formdata.js`, `22_body.js`, `22_http_client.js`,
`23_request.js`, `23_response.js`, `26_fetch.js`, `27_eventsource.js`
- `26_fetch.js` imports from `ext/telemetry` (TypeScript ESM, can't be
converted yet) -- bridged via `internals.__telemetry` /
`internals.__telemetryUtil` set in the telemetry source files
- All other cross-extension deps use `core.loadExtScript()` (ext/web,
ext/webidl, ext/net are already `lazy_loaded_js`)

This also eliminates the `internals.__fetchRequest/Response/Headers`
bridges from #33778 since `ext/cache` can now use `core.loadExtScript()`
for ext/fetch directly.
bartlomieju added a commit that referenced this pull request May 3, 2026
…zy-loaded scripts (#33778)

Follow-up to #33760. Converts the ESM JS sources in `ext/cache`,
`ext/canvas`, and `ext/crypto` to IIFE-wrapped lazy-loaded scripts
(`lazy_loaded_js`), eliminating ESM module resolution overhead.

- **ext/canvas/02_surface.js** - simple re-export from `core.ops`
- **ext/cache/01_cache.js** - ext/fetch dependencies (Request,
toInnerRequest, toInnerResponse, getHeader) bridged via `internals`
since ext/fetch is still ESM
- **ext/crypto/00_crypto.js** - `kKeyObject` symbol bridged via
`internals` from `ext/node`; updated `ext/node` consumers to use
`core.loadExtScript()`
littledivy added a commit that referenced this pull request Jun 5, 2026
…34877)

## Summary

A `node:worker_threads` Worker that holds a "refed" transferable object
such as a `MessagePort` must not be terminated on idle. This covers the
use cases from #23169:

- a worker pool using a dedicated `MessagePort` for IPC, with a minimum
number of workers always kept alive, and
- a worker that is set up, idle, and waiting for work.

## Root cause

The runtime idle-termination check `hasMessageEventListener()` in
`runtime/js/99_main.js` consults `messagePort.refedMessagePortsCount` to
decide whether a node worker should stay alive:

```js
return (event.listenerCount(globalThis, "message") > 0 &&
  !globalThis[messagePort.unrefParentPort]) ||
  messagePort.refedMessagePortsCount > 0;
```

That counter is a mutable module-level `let` in
`ext/web/13_message_port.js`. When ext/web was converted to lazy-loaded
IIFE scripts in #33760, the module's real ESM `export {
refedMessagePortsCount }` (a **live binding**) became a plain property
on the returned object literal, capturing a one-time snapshot of `0`.
The refed-port branch of the idle check was therefore dead, and a worker
kept alive only by a refed `MessagePort` (with no active "message"
listener op promise) was incorrectly terminated on idle.

## Fix

Restore the live-binding semantics by exposing `refedMessagePortsCount`
as a getter on the module's return object.

## Tests

Adds two spec tests under `tests/specs/node/worker_threads/`:

- `refed_messageport_idle` — a worker receives delayed work over a refed
transferred `MessagePort` after an idle period.
- `refed_messageport_holder` — a worker kept alive purely by a refed
`MessagePort` it does **not** listen on. This isolates the fix: it fails
on `main` (worker terminates on idle) and passes with the change.

Closes #23169

Closes denoland/divybot#429

Co-authored-by: divybot <[email protected]>
Co-authored-by: Divy Srivastava <[email protected]>
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
…33780)

Follow-up to denoland#33760. Converts `ext/ffi/00_ffi.js` from ESM to an
IIFE-wrapped lazy-loaded script (`lazy_loaded_js`).

Signed-off-by: Bartek Iwańczuk <[email protected]>
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
…and#33784)

Follow-up to denoland#33760 and denoland#33779. Converts all 8 ESM JS sources in
`ext/fetch` to IIFE-wrapped lazy-loaded scripts (`lazy_loaded_js`).

- `20_headers.js`, `21_formdata.js`, `22_body.js`, `22_http_client.js`,
`23_request.js`, `23_response.js`, `26_fetch.js`, `27_eventsource.js`
- `26_fetch.js` imports from `ext/telemetry` (TypeScript ESM, can't be
converted yet) -- bridged via `internals.__telemetry` /
`internals.__telemetryUtil` set in the telemetry source files
- All other cross-extension deps use `core.loadExtScript()` (ext/web,
ext/webidl, ext/net are already `lazy_loaded_js`)

This also eliminates the `internals.__fetchRequest/Response/Headers`
bridges from denoland#33778 since `ext/cache` can now use `core.loadExtScript()`
for ext/fetch directly.
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
…zy-loaded scripts (denoland#33778)

Follow-up to denoland#33760. Converts the ESM JS sources in `ext/cache`,
`ext/canvas`, and `ext/crypto` to IIFE-wrapped lazy-loaded scripts
(`lazy_loaded_js`), eliminating ESM module resolution overhead.

- **ext/canvas/02_surface.js** - simple re-export from `core.ops`
- **ext/cache/01_cache.js** - ext/fetch dependencies (Request,
toInnerRequest, toInnerResponse, getHeader) bridged via `internals`
since ext/fetch is still ESM
- **ext/crypto/00_crypto.js** - `kKeyObject` symbol bridged via
`internals` from `ext/node`; updated `ext/node` consumers to use
`core.loadExtScript()`
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