Skip to content

Remove dead code from the class generator: the unreachable DOMJIT paths - #43840

Merged
Jarred-Sumner merged 2 commits into
mainfrom
robobun/9a0817f5/dead-code-sweep
Sep 24, 2026
Merged

Jarred-Sumner merged 2 commits into
mainfrom
robobun/9a0817f5/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • Delete the emitter, the option type, the strip in define(), the five ignored blocks, and the stale notes next to them: 6 files, 273 lines removed.
  • The generated output is the same except for 94 empty #if BUN_DEBUG/#endif pairs in ZigGeneratedClasses.h and three DOMJIT-only #includes in ZigGeneratedClasses.cpp. generated_classes.rs is byte-identical.
  • Verified: the generator run before and after, bun bd, tsc -p src, and the tests in the Notes.
  • Self-reviewed: 13 concerns raised, 13 addressed (Notes).

Background

Downsides

  • To turn generated DOMJIT on again, a person must restore these paths from git history and write the Rust fast paths again.
  • No runtime cost found. Checked: the generated .cpp, .h and .rs differ only as Fix says.
Notes

Removed

  • generate-classes.ts: DOMJITName, argTypeName, DOMJITType, DOMJITFunctionDeclaration, DOMJITFunctionDefinition, domJITTypeCheckFields, RustDOMJITArgType. Also the DOMJIT branches in zigExportName, propRow, renderDecls, the expectedResultType asserts in the host-function wrapper, both Rust thunk loops, and the DOMJITAbstractHeap.h, FrameTracers.h, DFGAbstractHeap.h includes of the generated prologue. The destructures that named DOMJIT also lose the unused cache and value bindings.
  • class-definitions.ts: the DOMJIT?: option type and the two .map() calls in define() that erased it.
  • Ignored live blocks: Crypto.randomUUID, Crypto.timingSafeEqual (crypto.classes.ts), ServerWebSocket.publishText, publishBinary (server.classes.ts), TextDecoder.decode (encoding.classes.ts).
  • Stale notes: the commented-out // DOMJIT: { blocks with their crash notes on sendText, sendBinary (2023) and getRandomValues (Disable DOMJIT for crypto.getRandomValues() #13470, 2024-08), and three orphan "DOMJIT fast path" comments in src/runtime/webcore/Crypto.rs whose functions Remove ~39k lines of dead Rust across the workspace #35002 deleted.

Self-review, and what changed because of it

History

Kept on purpose

  • The hand-written DomCall path for bun:ffi (src/jsc/host_fn.rs, src/runtime/ffi), the C++ DOMJIT signatures in JSBuffer.cpp, JSPerformance.cpp, NodeVM.cpp, JSSQLStatement.cpp, and test/js/bun/jsc/domjit.test.ts.

Tests (debug build)

  • test/js/web/encoding/text-decoder.test.js 127 pass, test/js/web/web-globals.test.js 23 pass, test/js/bun/util/randomUUIDv5.test.ts 40 pass, test/js/bun/websocket/websocket-server.test.ts -t sendBinary 5 pass.
  • websocket-server.test.ts -t "publish|send": 44 pass, 4 time out near 19 s under debug+ASAN. A debug binary built from main fails the same 4.
  • test/js/bun/jsc/domjit.test.ts: 40 pass, 10 time out at the 100k-iteration sizes. A debug binary built from main gives the same 40/10.

The rest of this sweep

Verified and held for the next run (branch robobun/9a0817f5/dead-code-scopes-fields, 25 files, 81 lines removed)

Follow-up candidates, not verified dead

  • Parser Options.preserve_unused_imports_ts is never true. tsconfig importsNotUsedAsValues is parsed into preserve_imports_not_used_as_values but never reaches the parser, in the released binary too. This looks like a missing feature.
  • completions/bun-cli.json (4,513 lines) and misctools/generate-cli-completions.ts (728 lines): nothing in the repo reads the JSON, but feature PRs still edit it by hand.
  • bench/snippets/runner-entrypoint.js (244 lines): no reference, first line says "this isn't done yet", last real change 2023-05.
  • Ten impl_timer_owner! accessors have no caller because dispatch.rs recovers the owner with its own owner! macro. Which mechanism stays is a design call.
  • mordant-baseline.toml still counts about 170 unused_pub findings (sys/lib.rs 56, libuv_sys/libuv.rs 41, errno/windows_errno.rs 31). bun run rust:mordant names them.

@robobun

robobun commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

Status: open, waiting for CI and review.

How to check the change:

  • Run bun src/codegen/generate-classes.ts $(bun scripts/glob-sources.ts zigGeneratedClasses) <out> <types> on main and on this branch, then diff -r the two output directories. The only differences are 94 empty #if BUN_DEBUG/#endif pairs in ZigGeneratedClasses.h and three #include lines in ZigGeneratedClasses.cpp.
  • rg -n DOMJIT src/codegen finds nothing on this branch.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 2183b645-256c-40c7-866e-69b3aceab584

📥 Commits

Reviewing files that changed from the base of the PR and between 6d504dd and 4561f47.

📒 Files selected for processing (6)
  • src/codegen/class-definitions.ts
  • src/codegen/generate-classes.ts
  • src/runtime/crypto/crypto.classes.ts
  • src/runtime/server/server.classes.ts
  • src/runtime/webcore/Crypto.rs
  • src/runtime/webcore/encoding.classes.ts
💤 Files with no reviewable changes (4)
  • src/runtime/webcore/encoding.classes.ts
  • src/runtime/server/server.classes.ts
  • src/runtime/webcore/Crypto.rs
  • src/runtime/crypto/crypto.classes.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


Walkthrough

The Field function-field type and runtime class definitions no longer use DOMJIT metadata. Class generation no longer emits DOMJIT-specific wrappers, checks, fields, fast-path thunks, or support includes. timingSafeEqual now declares a function length of 2.

Changes

DOMJIT removal

Layer / File(s) Summary
Remove DOMJIT metadata
src/codegen/class-definitions.ts, src/runtime/crypto/crypto.classes.ts, src/runtime/server/server.classes.ts, src/runtime/webcore/Crypto.rs, src/runtime/webcore/encoding.classes.ts
The Field function-field type and runtime class definitions no longer include DOMJIT metadata. define() still sorts klass and proto entries by key. DOMJIT fast-path comments are removed from Crypto.rs.
Remove DOMJIT-specific generated code
src/codegen/generate-classes.ts
Generated property tables use ordinary native-function entries. Generation no longer emits DOMJIT exports, wrappers, type checks, class fields, fast-path thunks, or DOMJIT support headers.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 4561f

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: removal of unreachable DOMJIT paths from the class generator.
Description check ✅ Passed The description explains the problem, fix, scope, retained behavior, risks, and verification results. It does not use the exact template headings, but it provides the required information in equivalen…

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot 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.

Code review found no issues

No high-confidence issues detected in this change.

@Jarred-Sumner
Jarred-Sumner merged commit 50e3cad into main Sep 24, 2026
11 of 12 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/9a0817f5/dead-code-sweep branch September 24, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants