Skip to content

Remove dead code from C++ bindings, bindgen glue, ast, and orphaned scripts - #39581

Merged
Jarred-Sumner merged 1 commit into
mainfrom
claude/farm/99b45785/dead-code-cpp-scripts-ast
Aug 23, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
claude/farm/99b45785/dead-code-cpp-scripts-ast

Conversation

@robobun

@robobun robobun commented Aug 18, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Reopens #37149 against current main, as requested there. It deletes dead code whose last references are already gone. Every removed symbol was verified again on current main: zero references across src/, scripts/, packages/, and freshly regenerated codegen output.

C++ (src/jsc/bindings):

  • ncrypto.cpp/h: 33 uncalled methods from the ncrypto port (bytesToKey, FIPS toggles, secure-heap helpers, X509 time/usages getters, unused AES CTR/GCM/KW factories, BIO/Bignum constructors), the 5 named Digest factories orphaned by the bytesToKey removal (only Digest::FromName has callers), and the DataPointer secure_ field plus constructor parameters that SecureAlloc was the sole producer for
  • DOMJIT IDL helper headers (DOMJITIDLTypeFilter.h, DOMJITIDLType.h, DOMJITIDLConvert.h, DOMJITHelpers.h/.cpp): zero instantiations of any template they define; include sites now pull FrameTracers.h directly where JITOperationPrologueCallFrameTracer is used
  • 13 empty stub files (MessagePortChannel*, MessagePortIdentifier.h, BroadcastChannelRegistry.h, JSDOMConvertSerializedScriptValue.h, JSDOMBuiltinConstructorBase.*) kept by an earlier cleanup only for its own verification mechanics
  • ZigGlobalObject: 19 accessors with zero callers (8 *SinkPrototype() getters, worldIsNormal, bunStdin/bunStderr/bunStdout, KeyObjectStructure, JSBufferStructure, JSCryptoKeyStructure, builtinInternalFunctions, performMicrotaskVariadicFunction, globalProxyStructure), the backing fields and initLater closures this orphaned, and jsFunctionPerformMicrotaskVariadic
  • BunString isCrossThreadShareable, NodeValidator validateArrayBufferView, JSDOMConvertBase convertResult, JSDOMConvertSequences reserveEstimated, EventNames isGamepadEventType (declaration with no definition), ExceptionDetails EnumTraits, ResourceTiming overrideInitiatorType, WebSocket hasNativeCallbacks, PerformanceObserverCallback hasCallback (never-invoked virtual pair), ScriptExecutionContext isDocument/isWorkerGlobalScope
  • io_darwin.cpp: io_darwin_close_machport (both cfg arms); unlike its siblings it has no Rust-side declaration and no caller anywhere
  • WorkerMessagingProxy.cpp gains a direct #include <JavaScriptCore/JSPromise.h>. It got the header transitively through a unified-source neighbor that this PR deletes.

Rust:

  • jsc/bindgen.rs: BindgenOptionalRepr trait, BindgenOptionalCustom, BindgenExternalShared and their impls; the bindgen codegen emitters never name them. Also the Optional::adopt helper and ExternalSharedOptional<T> (bun_core/external_shared.rs plus its two re-exports) that this orphaned.
  • ast/binding.rs: BindingInit trait and Binding::init; all construction goes through Binding::alloc or literal struct init. Two doc comments that pointed at Binding::init are updated.
  • ast/runtime.rs: write-only ImportsIteratorEntry.value field and two commented-out leftover fields
  • runtime/webcore.rs: two unused flat re-exports
  • ini/lib.rs: commented-out Zig code from the port

Scripts (invoker .github/workflows/labeled.yml.disabled was deleted in #36778, or superseded):

  • label-issue.ts, read-issue.ts, handle-crash-patterns.ts, is-outdated.ts, associate-issue-with-sentry.ts
  • nav2readme.ts (imports docs/nav, deleted in Replace old docs with new docs repo #24201; cannot run)
  • buildkite-slow-tests.js (superseded by ci:slowest and update-test-durations)
  • check-node.sh, check-node-all.sh (superseded by bun run node:test)

Differences from #37149: the port resolves one conflict in ZigGlobalObject.h (main replaced JSDOMStructureMap m_structures with the m_domStructures array; m_worldIsNormal is still dead and stays deleted), adds the WorkerMessagingProxy include, and drops the dead-symbols source-lint test per test/internal/source-lints/CLAUDE.md.

Rebase notes (ab55966): main moved the ZigGlobalObject lazy-property registration from per-field initLater calls into OBJECT_OFFSETOF tables. The rebase re-applies the deletions in that shape: the table entries for m_cachedGlobalProxyStructure, m_JSCryptoKey, and m_performMicrotaskVariadicFunction are removed with their fields. One deletion from #37149 is dropped: m_bunStdin/m_bunStderr/m_bunStdout are live again (BunObject.cpp reads the fields directly), so the fields and their initializers stay. Only their unused accessor methods remain deleted.

Rebase notes (72ec6e2): parallel dead-code sweeps on main deleted neighbors of lines this PR deletes. Resolved by checking each symbol on current main: BIOPointer::NewFile and X509View::enumUsages still have zero callers, so their removal stands, next to main's removal of NewFp, ifRsa, and ifEc. The performMicrotaskVariadicFunction() accessor removal stands next to main's removal of performanceObject().

How did you verify your code works?

  • Full debug build (bun bd) on this branch at current main (6948a12)
  • Repo-wide greps on current main for each removed symbol, the deleted script names, and the deleted header names return only the deletion sites
  • bun bd test test/js/node/crypto/crypto.key-objects.test.ts: 95 pass, 0 fail
  • bun bd test test/js/web/workers/worker.test.ts: 4 debug-ASAN timeout failures, identical on a main debug build, release bun passes 37/37

This PR adds no test files on purpose. Every hunk deletes unreferenced code or keeps behavior identical (the ini hunk keeps the same expression and drops commented-out code, the iterator hunk drops a write-only field, the codegen hunk swaps deleted includes for FrameTracers.h). No test can distinguish the tree before and after this change, and test/internal/source-lints/CLAUDE.md forbids dead-symbol pin tests. The build is the check.

@coderabbitai

coderabbitai Bot commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

  • Run review for free

On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file.

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 56 seconds), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8b2b8093-1459-423a-919a-2768d636693b

📥 Commits

Reviewing files that changed from the base of the PR and between 72ec6e2 and ed53499.

📒 Files selected for processing (55)
  • scripts/associate-issue-with-sentry.ts
  • scripts/buildkite-slow-tests.js
  • scripts/check-node-all.sh
  • scripts/check-node.sh
  • scripts/handle-crash-patterns.ts
  • scripts/is-outdated.ts
  • scripts/label-issue.ts
  • scripts/nav2readme.ts
  • scripts/read-issue.ts
  • src/ast/binding.rs
  • src/ast/runtime.rs
  • src/bun_core/external_shared.rs
  • src/bun_core/lib.rs
  • src/codegen/generate-classes.ts
  • src/ini/lib.rs
  • src/io/io_darwin.cpp
  • src/js_parser/p.rs
  • src/js_printer/lib.rs
  • src/jsc/Strong.rs
  • src/jsc/bindgen.rs
  • src/jsc/bindings/BunObject.cpp
  • src/jsc/bindings/BunString.cpp
  • src/jsc/bindings/BunString.h
  • src/jsc/bindings/JSBuffer.cpp
  • src/jsc/bindings/JSFFIFunction.cpp
  • src/jsc/bindings/NodeValidator.cpp
  • src/jsc/bindings/NodeValidator.h
  • src/jsc/bindings/ScriptExecutionContext.h
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/jsc/bindings/ZigGlobalObject.h
  • src/jsc/bindings/ncrypto.cpp
  • src/jsc/bindings/ncrypto.h
  • src/jsc/bindings/sqlite/JSSQLStatement.cpp
  • src/jsc/bindings/webcore/DOMJITHelpers.cpp
  • src/jsc/bindings/webcore/DOMJITHelpers.h
  • src/jsc/bindings/webcore/DOMJITIDLConvert.h
  • src/jsc/bindings/webcore/DOMJITIDLType.h
  • src/jsc/bindings/webcore/DOMJITIDLTypeFilter.h
  • src/jsc/bindings/webcore/EventNames.h
  • src/jsc/bindings/webcore/ExceptionDetails.h
  • src/jsc/bindings/webcore/JSDOMBuiltinConstructorBase.cpp
  • src/jsc/bindings/webcore/JSDOMConvertBase.h
  • src/jsc/bindings/webcore/JSDOMConvertSequences.h
  • src/jsc/bindings/webcore/JSPerformance.cpp
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.h
  • src/jsc/bindings/webcore/MessagePortChannel.cpp
  • src/jsc/bindings/webcore/MessagePortChannelProvider.cpp
  • src/jsc/bindings/webcore/MessagePortChannelProviderImpl.cpp
  • src/jsc/bindings/webcore/MessagePortChannelRegistry.cpp
  • src/jsc/bindings/webcore/PerformanceObserverCallback.h
  • src/jsc/bindings/webcore/ResourceTiming.h
  • src/jsc/bindings/webcore/WebSocket.h
  • src/jsc/bindings/webcore/WorkerMessagingProxy.cpp
  • src/ptr/lib.rs
  • src/runtime/webcore.rs

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.

I reviewed this PR and didn't find any bugs. Because it's a broad deletion sweep across 63 files — including ZigGlobalObject, 33 ncrypto methods plus the DataPointer::secure_ field, the bindgen adapter layer, and 9 scripts whose supersession claims reference workflow history — a maintainer look to confirm scope would still be worthwhile.

What was reviewed:

  • Grepped every removed C++/Rust symbol (m_worldIsNormal/offsetOfWorldIsNormal, m_JSCryptoKey, performMicrotaskVariadic, m_cachedGlobalProxyStructure, io_darwin_close_machport, BindgenOptionalRepr/BindgenOptionalCustom/BindgenExternalShared/ExternalSharedOptional, reserveEstimated, Binding::init, ImportsIteratorEntry.value) — zero remaining references in src/ or codegen.
  • Deleted headers/.cpp files (DOMJIT*, MessagePortChannel*, JSDOMBuiltinConstructorBase, BroadcastChannelRegistry.h, JSDOMConvertSerializedScriptValue.h) have no remaining #include sites and no CMake/.cmake listings; the BunBroadcastChannelRegistry hits are a separate class.
  • DataPointer::operator= uses placement-new via the move ctor, so dropping secure_ there is complete; reset() no longer branches on it.
  • The #include "webcrypto/JSCryptoKey.h" in ZigGlobalObject.cpp was flagged as potentially orphaned but ruled out.
Extended reasoning...

Overview

This PR is a reopen of #37149: pure dead-code deletion across 63 files with no behavioral changes intended. It removes ~1,800 lines spanning C++ JSC bindings (ncrypto, ZigGlobalObject, DOMJIT IDL headers, 13 empty stub files), Rust bindgen/ast/ptr glue, and 9 orphaned scripts. The only additions are a direct #include <JavaScriptCore/JSPromise.h> in WorkerMessagingProxy.cpp (previously transitive via a deleted unified-source neighbor) and <JavaScriptCore/FrameTracers.h> at the three sites that used to get it via DOMJITHelpers.h.

Security risks

None introduced. The ncrypto deletions remove uncalled wrappers (FIPS toggles, secure-heap, unused cipher factories) but do not touch any live crypto path; DataPointer::reset() still OPENSSL_clear_frees. No auth, permissions, or input-validation code is weakened.

Level of scrutiny

High, for scope rather than correctness-per-line. Each deletion is compiler-verified (a stale reference would fail the debug build the author ran), and I spot-checked the highest-risk items — the OBJECT_OFFSETOF(m_worldIsNormal) (potential JIT consumer), the bindgen adapter types (potential codegen emitter), and the deleted .cpp files (potential CMake listing) — all confirmed dead. But the decision that each of 33 ncrypto methods, 19 ZigGlobalObject accessors, and 9 scripts is intentionally-abandoned rather than kept-for-upcoming-work is a maintainer judgment call this review can't make. ZigGlobalObject.h's own header warns it is included nearly everywhere.

Other factors

The PR reopens #37149 "as requested there," implying prior maintainer engagement on scope; that context isn't visible here. The ncrypto file is a port from Node's ncrypto and stripping it diverges from upstream — reasonable, but worth a maintainer nod. The nine deleted scripts reference workflow/PR history (#36778, #24201, ci:slowest, bun run node:test) that a human familiar with the tooling can confirm in seconds. No tests are added, which is standard for dead-code removal, and no existing tests are weakened.

Jarred-Sumner pushed a commit that referenced this pull request Aug 21, 2026
)

### Problem
- 92 hand-written trait impls (`Default`, `From`, `Clone`, `PartialEq`)
across 29 crates have no caller. rustc's `dead_code` lint exempts trait
impls, so the workspace lints never report them.
- A few items existed only for those impls: two tables in
`src/resolver/options.rs`, `Flags::DEFAULTS` in `src/md/types.rs`, and
five error variants that only a removed `From` impl constructed.

### Fix
- Delete the impls and the items that became unused. 80 files, 1064
lines removed. The one added line is a doc comment with its last
sentence cut.
- The candidates come from the linker: a relink of the debug build with
`--gc-sections --print-gc-sections` lists the trait methods dropped from
every codegen unit. Impls that a bound or a supertrait still needs
(`Deref` under `DerefMut`, `PartialEq` under `Ord`, `From<RGBA> for
SRGB`) do not compile without the impl, so they stay.
- Verified: `bun run rust:check-all` (11 targets), `bun run
rust:clippy`, `cargo build --tests` for the 21 touched crates with unit
tests, `cargo miri test` for the touched miri crates, `bun bd`, and 2112
tests across css, md, shell, env, resolve, node:fs, console.table,
archive, http2, build errors, install, and compile sourcemaps.

### Background
- To rustc a trait impl is always reachable: generic code could call it
through the trait. Only a whole-program view shows which impls nothing
calls.
- The debug build uses `-ffunction-sections` at opt-level 0, so a
function that nothing references is its own section and is not inlined
away. `--gc-sections` drops exactly those sections. A method that
survives in any codegen unit (an `#[inline]` copy, for example) counts
as live.
- The removed impls are listed below.

<details><summary>Removed impls by crate</summary>

- `src/runtime` (19): `Default for Chmod`; `Default for ExecCfg`;
`Default for FChmod`; `Default for FetchOptions`; `Default for
FrameHeader`; `Default for Framework`; `Default for FromJSOptions`;
`Default for GzipOptions`; `Default for ID`; `Default for
InitOptions<'a>`; `Default for IntoArray`; `Default for JSS3Error`;
`Default for MkdirTemp`; `Default for Open`; `Default for
ReactFastRefresh`; `Default for RmDir`; `Default for Stat`;
`From<CreatePtyError> for crate::Error`; `From<InitError> for
crate::Error`
- `src/react_compiler` (10): `Clone for GlobalRegistry`; `Clone for
ShapeRegistry`; `Default for Environment`; `Default for
ExhaustiveEffectDepsMode`; `Default for ScopeBlockTraversal`;
`From<&str> for JsString`; `From<String> for JsString`; `From<f64> for
FloatValue`; `PartialEq<&str> for JsString`; `PartialEq<str> for
JsString`
- `src/ast` (8): `Default for ClassStaticBlock`; `Default for
CommonJSNamedExport`; `Default for Dependency`; `Default for
MetadataResolve`; `Default for NamedImport`; `Default for Query`;
`From<SetError> for crate::Error`; `PartialEq for Data`
- `src/jsc` (6): `Default for BuildMessage`; `Default for
CPUProfilerConfig`; `Default for Column`; `Default for ResolveMessage`;
`Default for RuntimeTranspilerCache`; `Default for Store`
- `src/resolver` (6): `Clone for DependencyMap`; `Default for
BundleOptions`; `Default for EntryCache`; `Default for ExtensionOrder`;
`Default for ExtensionOrderGroup`; `Default for Package<'_>`
- `src/bun_core` (4): `Clone for SmolStr`; `Default for CtorOptions`;
`Default for DeserOpts`; `From<crate::Error> for JsError`
- `src/install` (4): `Default for Entry`; `Default for
MoreInstructions<'_>`; `Default for Scratch`; `PartialEq<crate::Error>
for ForManifestError`
- `src/css` (3): `PartialEq for CustomPropertyName`; `PartialEq for
KeyframesName`; `PartialEq for LayerName`
- `src/md` (3): `Default for BlockHeader`; `Default for Flags`; `Default
for Theme<'a>`
- `src/parsers` (3): `Default for JSONOptions`; `From<AddToLogError> for
crate::Error`; `From<bun_alloc::AllocError> for PErr`
- `src/uws_sys` (3): `Default for ListenConfig`; `Default for VTable`;
`Default for us_socket_stream_buffer_t`
- `src/bun_alloc` (2): `Default for MaxHeapAllocator`; `Default for
Mutex`
- `src/bundler` (2): `Clone for OutputFile`; `Default for BunLogOptions`
- `src/install_types` (2): `Default for TarballInfo`;
`From<CreateMatcherError> for FromExprError`
- `src/js_parser` (2): `Default for ReactFastRefresh`; `Default for
ThenCatchChain`
- `src/sourcemap` (2): `Default for Chunk`; `Default for Mapping`
- `src/base64` (1): `Default for VLQ`
- `src/collections` (1): `Default for StringMap`
- `src/dns` (1): `Default for Backend`
- `src/dotenv` (1): `Default for Map`
- `src/io` (1): `Default for FilePoll`
- `src/paths` (1): `From<Error> for crate::Error`
- `src/picohttp` (1): `Default for Response<'a>`
- `src/ptr` (1): `Default for WeakPtrData`
- `src/resolve_builtins` (1): `Default for Alias`
- `src/s3_signing` (1): `Default for SignQueryOptions`
- `src/sql_jsc` (1): `From<FlushQueueError> for crate::Error`
- `src/sys` (1): `Default for Address`
- `src/threading` (1): `Default for ThreadPool`

Follow-up removals that became unused with them:

- `src/resolver/options.rs`: the `EXTENSION_ORDER` /
`MODULE_EXTENSION_ORDER` copies and the `TargetMainFields` /
`DEFAULT_MAIN_FIELDS_*` tables (only `Default for BundleOptions` read
them; the bundler projects these options itself).
- `src/md/types.rs`: `Flags::DEFAULTS` (only `Default for Flags` read
it; `root.rs` builds `Flags` field by field).
- `src/parsers/xml.rs`: the `PErr::Oom` variant and its match arm (only
the removed `From<AllocError>` produced it).
- `src/css/rules/keyframes.rs`, `src/css/rules/layer.rs`: the `Hash` and
`Eq` impls that went with the removed `PartialEq` impls, and the two
comments that described them. The `ShapeRegistry` and `GlobalRegistry`
docs in `src/react_compiler/hir/` lose the sentence about cloning for
the same reason. Neither type is used as a map key; `LayerName`
comparisons go through the inherent `eql`.
- `src/ast/stmt.rs`: `impl Eq for Data`.
- Error variants whose only constructor was a removed `From` impl, with
their `name()` arms: `bun_runtime::Error::TerminalInit`,
`bun_ast::Error::Clobber`, `bun_paths::Error::MaxPathExceeded`,
`bun_sql_jsc::Error::AuthenticationFailed`. The inner error types
(`InitError`, `SetError`, `path_options::Error`, `FlushQueueError`) are
still live on their own.
</details>

<details><summary>Notes</summary>

- `bun_core::strings::rsplit_once` has no caller, but `clippy.toml`
names it as the replacement for `str::rsplit_once`, `str::rsplitn` and
`bstr::ByteSlice::rsplit_once_str`, so it stays. Policy files are one
kind of reference the linker does not see. No other removed item is
named in `clippy.toml`, `hawk.toml`, or the docs.
- Three impls came back during verification and are not in the diff:
`Semaphore: Default` (used by the macOS `fs_events.rs`, found by
`rust:check-all`), `clap::Help: Default` and `braces::Token: PartialEq`
(used only by unit tests, found by `cargo build --tests`). The
cross-target and test builds are needed for this kind of sweep: the
linux binary alone does not see them.
Scope of this sweep and what came back clean, so the next run can skip
it:

- Seven dead-code PRs are open right now (#37181, #37659, #39581,
#39582, #39618, #39697, #39732). This PR deletes no line that any of
them deletes (checked by diffing the removed lines).
`src/jsc/headergen/sizegen.cpp` is dead too, but #39618 already removes
it.
- Flipping every `pub` to `pub(crate)` in `bun_runtime` (349k lines) and
letting rustc report leaves 53 items, nearly all Windows-only or FFI
enum tags. The same two-round flip over the other 95 crates found only
macro-generated tables and items that are used from other platforms or
from macro bodies.
- The gc-sections list for C++ is almost entirely covered by the open
PRs. What is left is small: 15 `JSFoo::toWrapped` stubs, a few `ncrypto`
helpers, a handful of unused overloads.
- `src/js/internal/**` (111 files) and the remaining `src/js/node`,
`builtins`, `bun`, `thirdparty` files (86 files) were scanned export by
export: nothing dead beyond a few single lines
(`WASI.prototype.getState/setState`, three unused WASI constants).
- No orphan files: every `.rs` is mounted, every `.cpp` is compiled,
every `.h` is included. No stale `cfg` names (the build emits no
`unexpected_cfgs` warnings).
- Left alone on purpose: `src/runtime/api/bun/h2/` (in-progress rewrite,
`#![allow(dead_code)]` by design),
`src/react_compiler/compile_result.rs` (marked as not yet wired), the
MySQL `CharacterSet` collation table (222 unused constants, a code table
like the ones `hawk.toml` keeps), `Sys`/`Alloc` error variants that
follow the crate `Error` convention, and the `JsPoster` clone vtable
slot.
- Possible larger follow-up for a maintainer decision:
`PerformanceResourceTiming::create` and the `ResourceTiming` /
`NetworkLoadMetrics` / `PerformanceServerTiming` constructors are
unreferenced, so no such entry can ever exist. The JS classes stay
reachable as globals, which is why about 1.8k lines of
`src/jsc/bindings/webcore/*Timing*` survive. Removing them changes the
global surface, so it is not in this PR.
</details>

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@robobun
robobun force-pushed the claude/farm/99b45785/dead-code-cpp-scripts-ast branch from f349ece to 1c66d5a Compare August 21, 2026 01:16
@robobun

robobun commented Aug 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:53 PM PT - Aug 20th, 2026

❌ @robobun, your commit ed53499 has 2 failures in Build #102376 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 39581

That installs a local version of the PR into your bun-39581 executable, so you can run:

bun-39581 --bun

Comment thread src/ast/binding.rs Resolved
Comment thread src/js_parser/p.rs Resolved
Comment thread src/jsc/bindgen.rs Resolved
@robobun

robobun commented Aug 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Current head is ed53499, rebased onto main, full debug build passes. The red CI on the Windows 2019 lane comes from two failures that reproduce on unmodified main and are reported for triage: the bake/deinitialization.test.ts segfault (4/10 on a main debug build on the same Windows version) and bun-server.test.ts (tagged pre-existing by the CI scraper). No failure in the run touches this diff.

Comment thread src/jsc/bindings/ZigGlobalObject.cpp Resolved
@robobun
robobun force-pushed the claude/farm/99b45785/dead-code-cpp-scripts-ast branch 2 times, most recently from f365cfa to 09db9f8 Compare August 21, 2026 05:30

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

Thanks for addressing the JSGlobalProxyInlines.h include — confirmed removed. Correcting my earlier sibling claim: webcrypto/JSCryptoKey.h is not orphaned (line ~1182 WEBCORE_GENERATED_CONSTRUCTOR_GETTER(CryptoKey) still consumes it), so it should stay. The remaining inline nit on JSBuffer.cpp is the only new item this pass.

Extended reasoning...

The prior review flagged two orphaned includes in ZigGlobalObject.cpp. Re-checking against the current diff: JSGlobalProxyInlines.h is now deleted (addressed), and JSCryptoKey.h turns out to still have a consumer — WEBCORE_GENERATED_CONSTRUCTOR_GETTER(CryptoKey) — so that half of the earlier comment was incorrect and the include should remain. Verifier agents independently reached the same conclusion this run. Noting this so the author does not act on the stale suggestion.

Comment thread src/jsc/bindings/JSBuffer.cpp Resolved
…cripts

Every deleted symbol was verified to have zero references across src/,
scripts/, packages/, and freshly regenerated build/debug/codegen/ output,
then validated with a full debug build.

C++ (src/jsc/bindings):
- ncrypto.cpp/h: 33 uncalled methods from the ncrypto port (bytesToKey,
  FIPS toggles, secure-heap helpers, X509 time/usages getters, unused
  AES CTR/GCM/KW factories, BIO/Bignum constructors), the 5 named Digest
  factories orphaned by the bytesToKey removal (only Digest::FromName
  has callers), and the DataPointer secure_ field plus constructor
  parameters that SecureAlloc was the sole producer for
- DOMJIT IDL helper headers (DOMJITIDLTypeFilter.h, DOMJITIDLType.h,
  DOMJITIDLConvert.h, DOMJITHelpers.h/.cpp): zero instantiations of any
  template they define; include sites now pull FrameTracers.h directly
  where JITOperationPrologueCallFrameTracer is used
- 13 empty stub files (MessagePortChannel*, MessagePortIdentifier.h,
  BroadcastChannelRegistry.h, JSDOMConvertSerializedScriptValue.h,
  JSDOMBuiltinConstructorBase.*) kept by an earlier cleanup only for
  its own verification mechanics
- ZigGlobalObject: 19 accessors with zero callers (8 *SinkPrototype()
  getters, worldIsNormal, bunStdin/bunStderr/bunStdout, KeyObjectStructure,
  JSBufferStructure, JSCryptoKeyStructure, builtinInternalFunctions,
  performMicrotaskVariadicFunction, globalProxyStructure), the backing
  fields and initLater closures this orphaned, and
  jsFunctionPerformMicrotaskVariadic
- BunString isCrossThreadShareable, NodeValidator validateArrayBufferView,
  JSDOMConvertBase convertResult, JSDOMConvertSequences reserveEstimated,
  EventNames isGamepadEventType (declaration with no definition),
  ExceptionDetails EnumTraits, ResourceTiming overrideInitiatorType,
  WebSocket hasNativeCallbacks, PerformanceObserverCallback hasCallback
  (never-invoked virtual pair), ScriptExecutionContext isDocument/
  isWorkerGlobalScope
- io_darwin.cpp: io_darwin_close_machport (both cfg arms); unlike its
  siblings it has no Rust-side declaration and no caller anywhere

Rust:
- jsc/bindgen.rs: BindgenOptionalRepr trait, BindgenOptionalCustom,
  BindgenExternalShared and their impls; the bindgen codegen emitters
  never name them. Also the Optional::adopt helper and the
  ExternalSharedOptional type (bun_core/external_shared.rs plus its two
  re-exports) that this orphaned.
- ast/binding.rs: BindingInit trait and Binding::init; all construction
  goes through Binding::alloc or literal struct init. Two doc comments
  that pointed at Binding::init are updated.
- ast/runtime.rs: write-only ImportsIteratorEntry.value field and two
  commented-out leftover fields
- runtime/webcore.rs: two unused flat re-exports
- ini/lib.rs: commented-out Zig code from the port

Scripts (invoker .github/workflows/labeled.yml.disabled was deleted in
 #36778, or superseded):
- label-issue.ts, read-issue.ts, handle-crash-patterns.ts,
  is-outdated.ts, associate-issue-with-sentry.ts
- nav2readme.ts (imports docs/nav, deleted in #24201; cannot run)
- buildkite-slow-tests.js (superseded by ci:slowest and
  update-test-durations)
- check-node.sh, check-node-all.sh (superseded by bun run node:test)
@robobun
robobun force-pushed the claude/farm/99b45785/dead-code-cpp-scripts-ast branch from 09db9f8 to ed53499 Compare August 21, 2026 06:02

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

Both earlier inline notes (orphaned JSGlobalProxyInlines.h include, unnecessary FrameTracers.h in JSBuffer.cpp) are now addressed, and this pass found no further issues. Given the breadth — 55 files including ZigGlobalObject.{h,cpp} lazy-property tables, the DataPointer constructor signature change in ncrypto, and bindgen trait removal — a maintainer sign-off is still worthwhile.

What was reviewed:

  • Confirmed webcrypto/JSCryptoKey.h include is still live (WEBCORE_GENERATED_CONSTRUCTOR_GETTER(CryptoKey)), so leaving it was correct.
  • Confirmed kMinCrossThreadShareableLength remains used by toCrossThreadShareable after isCrossThreadShareable removal.
  • Checked that each removed FOR_EACH_GLOBALOBJECT_GC_MEMBER entry has its matching field, accessor, and initLater table entry removed together.
Extended reasoning...

Overview

This PR deletes dead code across 55 files: ~33 uncalled ncrypto methods and the DataPointer secure_ field/ctor param, 5 DOMJIT IDL helper headers, 13 empty stub .cpp files, ~19 unused ZigGlobalObject accessors plus 3 backing lazy-property fields (m_cachedGlobalProxyStructure, m_JSCryptoKey, m_performMicrotaskVariadicFunction) and jsFunctionPerformMicrotaskVariadic, assorted single-function removals in webcore headers, Rust bindgen adapter traits (BindgenOptionalRepr/BindgenOptionalCustom/BindgenExternalShared) and the ExternalSharedOptional type they orphaned, an unused AST BindingInit trait, a write-only iterator field, and 9 orphaned scripts whose invoking workflow was deleted.

Security risks

None identified. The ncrypto deletions remove uncalled wrapper methods (FIPS toggles, secure-heap helpers, unused cipher factories) — no live crypto path changes behavior. DataPointer::reset now unconditionally calls OPENSSL_clear_free since the only producer of secure_=true (SecureAlloc) is deleted; under BoringSSL both branches were already equivalent.

Level of scrutiny

High, because of surface area rather than logic complexity. Each hunk is a mechanical deletion whose correctness reduces to "zero callers on current main" plus "still builds", but the deletions span ZigGlobalObject's GC-visited member table, the class-codegen include block, and FFI-adjacent Rust traits — files where a missed reference fails at link time or worse. The author states a full debug build passes and repo-wide greps return only deletion sites; CI build #102376 is the ground truth.

Other factors

I left two inline nits on earlier revisions; both are addressed in the current diff (JSGlobalProxyInlines.h include dropped; FrameTracers.h no longer added to JSBuffer.cpp). One candidate this run — JSCryptoKey.h orphaned — was verified false: WEBCORE_GENERATED_CONSTRUCTOR_GETTER(CryptoKey) at ZigGlobalObject.cpp:1182 still consumes it. The comment-cop bot flags on binding.rs/p.rs/bindgen.rs were resolved as false positives (section-banner comments being shortened, not workaround justifications). No tests are added by design; this is deletion-only and the source-lints doc forbids dead-symbol pin tests. Given the file count and that this touches the GlobalObject GC-member macro, deferring to a maintainer rather than auto-approving.

Jarred-Sumner pushed a commit that referenced this pull request Aug 23, 2026
…JS, the bake dev server assets, the shell, and the SQL crates (#40066)

Scheduled dead-code sweep. Net -3275 lines (78 files, +112 / -3387; the
source side is +32 / -3387, the rest is the test pin). No behavior
change: every item below has zero references on `main` in `src/`,
`packages/`, `scripts/`, `test/` and the regenerated
`build/debug/codegen/`, and the build passes without it.

About 2,350 of these lines carry over the removals from #37659, which
was closed only for merge conflicts. Each of those was re-verified
against current `main` (nothing gained a caller), and the diff applies
to current `main` with no conflicts. The rest comes from fresh scans of
the shell, SQL, bake and built-in JS areas, which no open dead-code PR
covers.

### bun-uws (`packages/bun-uws/src`, -1168)

Bun reaches uWS only through the C shim in `src/uws_sys/` and eight
bindings files, so those includers are the complete set of callers.
Checked against every one of them:

* Whole headers nothing includes: `ClientApp.h`, `HttpError.h`
(superseded by `HttpErrors.h`), `Multipart.h` and `MessageParser.h` (its
only includer), `ProxyParser.h` (only reachable under `UWS_WITH_PROXY`,
which no build defines; `grep UWS_ build/debug/compile_commands.json` is
empty), plus every `#ifdef UWS_WITH_PROXY` block and the `reserved` /
`proxyParser` parameters that only carried the proxy parser into
`getHeaders()`.
* `WebSocketBehavior::subscription` and
`WebSocketContextData::subscriptionHandler`: the only place a
`WebSocketBehavior` is built (`uws_ws()` in the shim) never sets it, so
every dispatch site in `WebSocket::end/subscribe/unsubscribe` and
`WebSocketContext::onClose` was unreachable. `TopicTree::unsubscribe`
drops the `newCount` only those sites read.
* `HttpRequest`: the per-route `std::map` of parameter offsets
(`setParameterOffsets`, `getParameter(name)`; every caller uses the
index overload), `getQuery()` with no key, `isShortRead`,
`notFieldNameWord` / `hasMore` / `hasBetween`.
* `HttpContext::HTTP_IDLE_TIMEOUT_S`, `HttpFlags::isAuthorized` (the
live flag is the per-socket one in `AsyncSocketData`),
`HttpResponse::getHttpResponseDataS`, `HttpResponse::endWithoutBody`,
`HttpResponse::overrideWriteOffset` (the shim calls `setWriteOffset`).
* `App::publish(unsigned char)`, two `App::listen` overloads, the
`UWS_NO_ZLIB` block; two `H3App::listen` overloads;
`Http3Request::isAncient` / `getCaseSensitiveMethod`; nine
`Http3Response` members whose `uws_h3_res_*` shims are stubs; the
write-only `Http3ResponseData::totalSize`; `Loop::integrate`,
`Loop::setSilent` and `LoopData::noMark`; the `UWS_NO_ZLIB` /
`UWS_MOCK_ZLIB` mock streams and `UWS_USE_LIBDEFLATE` guards in
`PerMessageDeflate.h` (the macro is defined unconditionally three lines
above its first use).
* `HttpContext::layoutAssert()` and `WebSocketContext::layoutAssert()`
were never called, and as static members of class templates their
`static_assert`s never ran. The `HttpContext` one guarded nothing. The
`WebSocketContext` one guards the layout `WebSocket::getContextData()`
depends on (`group` first, `data` right after), so it becomes a
`RELEASE_ASSERT` in `create()`: `offsetof` cannot express it because the
struct is not standard-layout, and the layout can differ per ABI, so the
check runs once per context in every build.
* `misc/` (upstream README assets, demo `cert.pem` / `key.pem`), and the
`.gitattributes`, `.dockerignore`, `.cursorignore` entries for a
`fuzzing/` directory that no longer exists.

### Native `JSBufferList` (`src/jsc/bindings`, -641)

`JSBufferList.cpp` / `.h`, the lazy class structure on `GlobalObject`,
its three accessors, and the `DOMIsoSubspaces` / `DOMClientIsoSubspaces`
slots. The JS streams implementation replaced it; the only remaining
mention of "BufferList" in the tree is a comment in
`internal/streams/readable.ts`. The sources are globbed, so no
build-script change is needed.

### Built-in JS (`src/js`, -508)

* `internal/util/inspect.d.ts` (ambient types for
`node-inspect-extracted`, which nothing imports) and `src/js/.gitignore`
(ignores `src/js/out`, which codegen has not written since the build
moved under `build/`).
* `private.d.ts`: the `BunFS` / `BunFSWatcher` types (`Bun.fs()` no
longer exists), `Bun.TOML`, `Bun.tty`.
* `QuicStream.prototype[kSendHeaders]` and its symbol, the unused `get
[kVerifyPeer]` (the setter stays; `session[kVerifyPeer] = …` writes it),
`colors.ts` `yellow` / `gray` / `clear` / `reset`, the runtime `SSLMode`
export of `internal/sql/shared.ts` (only imported as a type), the
`REPL_MODE_*` re-exports of `internal/repl/utils.js` (`repl.js` takes
them from `internal/repl/mode`), `Dequeue#clear()`, the write-only
`listeningId` in `net.ts`, and the `kGetNativeReadableProto` symbol in
`internal/shared.ts` (its last reader went in 032713c).

### Bake (`src/runtime/bake`, -830)

* `incremental_visualizer.html` and `memory_visualizer.html`: #36184
removed the `runtime_embed_file!` sites and the
`/_bun/incremental_visualizer` route that served them, so the two pages
are orphaned assets. The no-op `emit_*_visualizer_*` methods that #36184
left behind are still reachable and are not touched here.
* `client/websocket.ts` `getMainWebSocket()` (exported, never imported)
and a commented-out Zig `writeJsValue` in `serialized_failure.rs`.

### Shell (`src/runtime/shell`, `src/shell_parser`, -51)

* `Base::end_scope()`, an empty function, and its 12 call sites in
`states/*.rs`. `Script::deinit_from_interpreter` only called it and goes
too.
* `ShellErr::InvalidArguments`: never constructed (only matched), with
its four match arms.
* `bun_shell_parser` dependencies `scopeguard`, `const_format`,
`enum-map`, `enumset`, `typed-arena`, `bun_collections`: no `use` or
path reference in the crate.

### SQL (`src/sql`, `src/sql_jsc`, -22) and `hawk.toml` (-104)

* `bun_sql` dependencies `const_format`, `enum-map`, `enumset`, `libc`;
`bun_sql_jsc` dependencies `const_format`, `enum-map`, `enumset`,
`libc`, `bun_uws_sys`, and a commented-out `bun_runtime` edge that would
be a dependency cycle (`bun_runtime` depends on `bun_sql_jsc`).
* `pub use` re-exports nothing imports: `ArrayBuffer`, `JSCell`,
`host_fn` in `sql_jsc/jsc.rs` (the proc-macro is used by its absolute
path), `mysql::{MySQLConnection, MySQLQuery, MySQLStatement}` (every
consumer imports the leaf module), `postgres::SASL`.
* `hawk.toml`: 13 overrides for
`bun_sql::mysql::status_flags::StatusFlag::*` variants that #37229
removed; `StatusFlag` has one variant left.

### Rust (-18)

`zig__ModuleInfo__destroy` and `zig_log` in
`src/bundler/analyze_transpiled_module.rs`: `extern "C"` exports that no
C++ declares or calls (`BunAnalyzeTranspiledModule.cpp` frees through
`zig__ModuleInfoDeserialized__deinit`), with no hit in `vendor/WebKit`
either.

### Verification

* `rg` for every symbol across `src/`, `packages/`, `scripts/`, `test/`,
`docs/` and `build/debug/codegen/`; `.classes.ts` files, `src/codegen/`,
string-named lookups (`$getByIdDirectPrivate(this, "…")`, `$zig` /
`$cpp`) and token-paste macros (`name##PublicName`) were checked by
hand. Two candidates failed those checks during the run and stay:
`BunBuiltinNames` `mockedFunction` (reached through
`BUN_COMMON_STRINGS_EACH_NAME`) and `writer` (reached through
`$getByIdDirectPrivate(this, "writer")` in `ConsoleObject.ts`).
* `bun bd` builds. `cargo check --workspace` passes on all 12 target
triples. `cargo fmt --check`, prettier, oxlint and clang-format are
clean. `tsc -p src/js` reports the same errors before and after (only
line numbers move).
* `test/js/bun/websocket/websocket-server.test.ts` gains a pin for the
pub/sub paths this PR edits: with two sockets on shared topics,
`unsubscribe()` of a topic the socket never joined (or that does not
exist) reports false and changes nothing, leaving the last topic frees
the subscriber and a later `subscribe()` counts again, and `close()`
drops the socket from every topic without touching the other socket.
Because this PR only deletes unreachable code, the pin passes before and
after by design.
* `bun bd test`: `bun/websocket/websocket-server.test.ts`,
`web/websocket/websocket-permessage-deflate.test.ts`,
`bun/http/bun-serve-routes.test.ts`, `bun/http/serve-http3.test.ts`,
`node/http/node-http.test.ts`, `node/quic/quic-stream.test.ts`,
`node/net/node-net-server.test.ts`, `bun/util/inspect.test.js`, six
`bun/shell/*.test.ts` files, `sql/sql-mysql*.test.ts`,
`sql/sql-mariadb-json.test.ts`, `sql/postgres-binary-numeric.test.ts`,
`sql/sql-connect-error-reporting.test.ts`,
`node/stream/node-stream.test.js`,
`node/console/console-table-iterators.test.ts`,
`web/console/console-log.test.ts`, `internal/fifo.test.ts`,
`bake/dev/esm.test.ts`, `bake/dev/hot.test.ts`, `bun/repl/repl.test.ts`
(mode tests), `node/test/parallel/test-repl-definecommand.js`. Details
on the local failures that also fail on the released bun are under
Notes.

### Looked at and left alone

* `src/runtime/api/bun/h2/`: the outbound half
(`Connection::send_header_block` / `send_data` / `send_push_promise`,
`hpack::Coder::encode`, `SendWindow::available`) is only exercised by
its unit tests. The module doc describes it as the engine that will
replace `h2_frame_parser.rs`, so it is in progress, not dead.
* The bake visualizer protocol residue: `MessageId::Visualizer`, the
`IncrementalVisualizer` topic, and the empty `emit_*_visualizer_*`
methods and their timer. All reachable; removing them is a protocol
change that also touches the generated `generated.ts` ids.
* `ColumnDefinition41::{fixed_length_fields_length, decimals}` are
decoded and never read, but they mirror the MySQL wire struct; left in
place.
* `bun_sql::mysql::StatusFlags`'s `Display` impl prints nothing, but a
debug log in `MySQLConnection.rs` still formats it.

<details><summary>Notes</summary>

How the candidates were found: the dead-code PRs that were closed for
merge conflicts were re-applied against current `main` and every removal
re-verified (#37659 applied with no conflicts; its `assertion_error.ts`
and `util.ts` hunks had already landed via #39924 and #39628 and are not
in this PR). Read-only scans of `src/runtime/shell` +
`src/shell_parser`, `src/sql` + `src/sql_jsc` + `src/runtime/node`,
`src/runtime/bake` + `src/runtime/api` + `src/runtime/image`, and
`src/js` + `src/jsc/modules` produced the fresh items.
`src/runtime/node`, `src/runtime/api` (outside `h2/`),
`src/runtime/image` and `src/jsc/modules` came back clean.

Local test failures, all reproduced with `USE_SYSTEM_BUN=1` (the
released bun) in the same container, so none is caused by this diff:

* `websocket-server.test.ts`: eight `it.concurrent` cases that overlap
the 300,000-message `(benchmark)` test time out under the debug ASAN
build in this container (the whole file takes 64 s here against a 6.4 s
CI budget); each passes when run with `-t`.
* `node-http.test.ts` "request via http proxy": `ECONNREFUSED` from
`localhost` resolution in this container.
* `bunshell.test.ts` `-c`, `-f character device`, and the seven "stdin
redirect from a zero-length buffer" cases: this container's `/dev/null`
is a regular file, not a character device.
* `sql-mysql-binary-null-indexed.test.ts`: `Access denied for user
'root'@'localhost'` against the local MariaDB.
* `fifo.test.ts` "pushing and shifting a lot of items": a 10 s perf
budget exceeded under ASAN; the only `fifo.ts` change is the removal of
the unused `clear()` method.

Files shared with open PRs, different hunks:
`src/jsc/bindings/ZigGlobalObject.cpp` / `.h` (#39581 removes other
accessors; its `HTMLRewriterSinkPrototype()` hunk sits one blank line
above the `JSBufferList` accessors removed here), `Cargo.lock` (#39618
edits other package blocks).

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 4 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/bun/websocket/websocket-server.test.ts

<!-- robobun:evidence:end -->
@Jarred-Sumner
Jarred-Sumner merged commit fb4227f into main Aug 23, 2026
9 of 11 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/farm/99b45785/dead-code-cpp-scripts-ast branch August 23, 2026 06:42
robobun added a commit that referenced this pull request Aug 23, 2026
…) into farm/2df9101c/ci-slowest-docker-wait

main deleted scripts/buildkite-slow-tests.js (#39581); drop #35418's hunk for
it and the test that spawned it.
robobun added a commit that referenced this pull request Aug 23, 2026
binding.rs: #39581 removed Binding::init, which nothing called here either;
Binding::alloc keeps taking impl Into<Option<Loc>>.
robobun added a commit that referenced this pull request Aug 31, 2026
Both slow-test parsers (scripts/ci-slowest-tests.ts and
scripts/update-test-durations.mjs) measured a file as the gap between
its `[N/M] <path>` header and the next header they recognized. Every
other header runner.node.mjs emits (`--- napi prebuild:`, the
`--- [A-B/M] K files in parallel` bucket, `--- Running N parallel-safe`,
`--- End`, `--- Summary`, `--- Received <signal>, exiting`) was
invisible, so the serial test that happened to run before one of them
was charged the whole phase that followed.

- Those phase headers close the open span. The check is an allowlist of
  the titles the runner emits, not any `--- ` line, because streamed
  test output can still contain `--- a/<file>` or `--- ps ---`.
- `[N/M] <path>` matches with or without the `--- ` prefix. Bucket
  summary lines `[N/M] <path> (X.XXs)` use the inline timing.
  Concurrent dispatch spans are clamped to 500 ms.
- Retry/error labels (`<path> - code 1`) are a boundary that opens no
  span, so the retry backoff lands on neither attempt.
- A truncated log (no `--- End`) charges the open span to the last
  timestamp seen instead of dropping it.

Both scripts export parseLog behind an entrypoint guard, and
test/internal/ci-slowest-tests.test.ts exercises them against the same
header fixtures. scripts/buildkite-slow-tests.js from #35418 is gone
from main (#39581) and is not restored.

Regenerated test/expected-durations.json with the combined parser from
builds 108915, 108911, 108910, 108909, 108906.
Jarred-Sumner pushed a commit that referenced this pull request Sep 1, 2026
…sers, regenerate the table (#41070)

### Problem
- `windows-aarch64-11-test-bun` packs its 8 shards with the timings of
the windows x64 lane. Its serial files run about 2x slower and not
uniformly (`bundler/bundler_compile.test.ts` is 222s there, 48s in the
x64 column). One shard carries 922s of work against a 610s mean. This
lane finished last in 46 of the 47 completed builds since #40993 merged,
so it sets the build time: the median build went from 1342s to 1386s
while the linux lanes got faster (x64 glibc slowest shard 233s to 178s).
- The slow-test log parsers (`scripts/update-test-durations.mjs`,
`scripts/ci-slowest-tests.ts`) only recognize `[N/M] <path>` headers.
Every other header the runner emits (`--- napi prebuild:`, the `---
[A-B/M] K files in parallel` bucket, `--- Running N parallel-safe`) is
invisible, so the serial file that runs right before one is charged the
whole phase that follows. In source build 108642 this hit 19 of 20 linux
shards and 8 of 8 windows shards
(`js/valkey/unit/basic-operations.test.ts` recorded as 111.6s on
windows, real 0.1s). 11 of the phantom entries are 10s or more in the
default column, so `--skip-slower-than=10000` dropped those files from
the darwin PR lanes.

### Fix
- Add a `windows-aarch64` column, measured from
`windows-aarch64-11-test-bun`. `runner.node.mjs` picks it for the
windows-aarch64 step and falls back to `windows`, then the other
columns, for a file that lacks one.
- Fold in the parser fixes from #35418 (this PR supersedes it): the
runner's phase headers close the open span (an allowlist of the titles
the runner emits, since test output can contain `--- a/<file>`), bucket
summary lines use their inline `(X.XXs)` timing, retry labels open no
span, and a truncated log keeps its last open span. Both scripts export
`parseLog` behind an entrypoint guard, and
`test/internal/ci-slowest-tests.test.ts` runs them against the same
fixtures. `scripts/buildkite-slow-tests.js` from #35418 was deleted from
main in #39581 and is not restored.
- Regenerate `test/expected-durations.json` with the combined parser
from builds 108915, 108911, 108910, 108909, 108906 (`node
scripts/update-test-durations.mjs --builds 5`, 5906 entries).
- Verified: `bun bd test test/internal/ci-slowest-tests.test.ts` (18
pass). Replaying the LPT packing against per-file costs measured on
three other windows-aarch64 runs (builds 108785, 108786, 108801) gives a
slowest shard of 673s with the new column, against 922s with the table
on main (610s is the floor). Column selection checked for every step
key.

### Background
- `runner.node.mjs` bin-packs test files across `--max-shards` with LPT
(longest file first into the least loaded bin) using
`test/expected-durations.json`. Every shard computes the same assignment
from the same input, so the shards need no coordination.
- The table has one column per lane with distinct timing: `default`
(linux x64 debian), `asan`, `musl`, `windows`. Every other lane uses the
nearest column (darwin uses `default`).
- Per-file cost comes from the `_bk;t=<ms>` timestamps Buildkite
prefixes to each log line: the gap between one `[N/M] <path>` group
header and the next header. The parallel bucket runs one `bun test
--parallel` for many files and prints `[N/M] <path> (X.XXs)` per file
afterwards.
- The untiered darwin PR lanes pass `--skip-slower-than=10000`, which
drops a file when its column cost is 10s or more.
- `.buildkite/update-test-durations.yml` regenerates the file on a
schedule and uploads it as an artifact. A human commits it.

<details><summary>Notes</summary>

Measurement: Buildkite REST job timings for 95 builds created after the
merge of #40993 (09:17 UTC, 2026-08-31). 64 builds include the new table
(GitHub compare status `ahead`), 31 PR builds still ran on the old one.
Same fleet, same hours. Median of the slowest shard per lane, old table
to new table:

| lane | old | new |
|---|---|---|
| linux-x64-debian-13 | 233s | 178s |
| linux-x64-asan | 437s | 387s |
| linux-aarch64-debian-13 | 255s | 230s |
| linux-x64-ubuntu-2504 | 232s | 178s |
| musl (both arches) | 271-282s | 281-294s |
| windows-x64-2019 | 491s | 468s |
| windows-aarch64-11 | 829s | 887s |
| darwin-aarch64-any | 534s | 581s (2 shards, balanced per shard: 506s
and 518s) |
| build start to last test finished | 1342s | 1386s |

windows-aarch64 per-shard medians with the old table: `456 523 594 572
477 375 823 526` (shard 6 slowest in 177 of 188 builds). With the table
from #40993: `447 404 575 892 521 651 416 453` (shard 3 slowest in 49 of
49). The runner estimated that shard at 429s.

Largest windows-aarch64 costs in the new column (s): bundler_compile
219, serve-body-leak 154, expo 114, v8 97, test-fs-read-stream-pos 90,
napi 85.

Bucket header bug, source build 108642, default column (parsed vs real):
websocket-proxy-tunnel-client-leak 17.6 vs 0.0,
valkey/integration/complex-operations 32.8 vs 0.1,
regression/issue/24850 15.7 vs 0.1, sql-mysql-bigint-out-of-range 24.4
vs 0.3. Windows column: valkey/unit/basic-operations 122.5 vs 0.1,
regression/issue/28632 76.3 vs 0.1. In the logs checked, the `--- napi
prebuild:` header carries the same timestamp as the next file header, so
it misattributes nothing today; the allowlist still covers it.

From #35418, against build 86086 logs:
`test/js/bun/test/fake-timers/sinonjs/issue-2086.test.ts` parsed at
22554 ms before, 57 ms after (bun's own timer: 41 ms).
`test/regression/issue/24850.test.ts` 60575 ms before, 61 ms after.
Unique files found in one windows shard: 63 before, 829 after (the old
`ci-slowest-tests.ts` regex required a literal `[90m` SGR and the `--- `
prefix).

Side effect: `scripts/update-parallel-allowlist.mjs` takes the slowest
column per file. With the new column, 10 files that are under 15s on
every other lane are over 15s on windows-aarch64 and will leave the
parallel allowlist at its next regeneration (for example
`js/bun/spawn/spawn-noread-leak.test.ts` at 38.7s). That is the stated
intent of that check.

The regenerated default column has 44 entries at 10s or more against 55
on main. Files that the darwin PR lanes skipped because of phantom
entries and now run again: websocket-proxy.test.ts,
websocket-proxy-tunnel-client-leak, websocket-proxy-tunnel-upgrade-leak,
regression/issue/24850, regression/issue/patch-bounds-check,
cli/install/bun-install-proxy.

</details>

<!-- robobun:evidence:begin -->

---

**[stamp-90s]** gate passed · iteration 2 · 6 files touched

<details><summary>passes on PR (with fix)</summary>

```console
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/internal/ci-slowest-tests.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/ci-slowest-tests.test.ts
bun test v1.4.1 (a6c4cc2)

test/internal/ci-slowest-tests.test.ts:
(pass) scripts/ci-slowest-tests.ts parseLog > does not charge the parallel-safe phase to the last serial test [28.64ms]
(pass) scripts/ci-slowest-tests.ts parseLog > sums retry attempts and normalizes Windows path separators [6.28ms]
(pass) scripts/ci-slowest-tests.ts parseLog > closes the last serial test at the parallel-phase group header when no parallel headers follow [11.47ms]
(pass) scripts/ci-slowest-tests.ts parseLog > does not charge the parallel-bucket phase to the preceding serial test [12.70ms]
(pass) scripts/ci-slowest-tests.ts parseLog > ignores stray `--- ` lines that are test output, not group headers [7.17ms]
(pass) phase-header boundary > true --- napi prebuild: 3 addon(s), 23.9s [1.37ms]
(pass) phase-header boundary > true --- [52-257/829] 206 files in parallel (3×) [0.32ms]
(pass) phase-header boundary > true --- Running 444 parallel-safe tests (3-wide) [0.19ms]
(pass) phase-header boundary > true --- End [0.18ms]
(pass) phase-header boundary > true --- Summary [0.18ms]
(pass) phase-header boundary > true --- Received SIGTERM, exiting... [0.18ms]
(pass) phase-header boundary > false --- a/index.js [0.20ms]
(pass) phase-header boundary > false --- ps --- [0.17ms]
(pass) phase-header boundary > false --- logs --- [0.17ms]
(pass) phase-header boundary > false ------ [0.18ms]
(pass) phase-header boundary > false ---  [0.27ms]
(pass) phase-header boundary > false --- [52/829] test/a.test.ts [0.19ms]
(pass) scripts/update-test-durations.mjs parseLog > does not charge napi prebuild or the parallel-bucket phase to the preceding serial test [17.04ms]

 18 pass
 0 fail
 24 expect() calls
Ran 18 tests across 1 file. [2.22s]
Exit: 0
```

</details>

<details><summary>diff hotspot</summary>

```
scripts/ci-log-phase.mjs               |    32 +
 scripts/ci-slowest-tests.ts            |   282 +-
 scripts/runner.node.mjs                |    18 +-
 scripts/update-test-durations.mjs      |   116 +-
 test/expected-durations.json           | 49849 +++++++++++++++++--------------
 test/internal/ci-slowest-tests.test.ts |   196 +
 6 files changed, 28355 insertions(+), 22138 deletions(-)
```

</details>

**gate history** · 3 passed · 0 rejected · iteration 2

<details><summary>evidence per changed file</summary>

```
file                                    reads  edits  tests
scripts/ci-log-phase.mjs                    0      1      0
scripts/ci-slowest-tests.ts                 1      1      0
scripts/runner.node.mjs                     4      1      0
scripts/update-test-durations.mjs           4      6      0
test/expected-durations.json                0      0      0
test/internal/ci-slowest-tests.test.ts      3      4      0
```

</details>

<!-- robobun:evidence:end -->
robobun added a commit that referenced this pull request Oct 1, 2026
The watcher thread now frees the Watcher when a dev server stops.
MultiArrayList's drop is slab-only, so the owned file_path of every entry
that is still in the watchlist leaked. Since #39197, CI's ASAN lane applies
its exit-time leak check to test/bake/dev/css.test.ts, and that check
reports the leak from append_directory_assume_capacity::<true>.

impl Drop for Watcher drops the elements of the watchlist, as #30644 does.

KEventWatcher.rs: main deleted io_darwin_close_machport and its non-Darwin
stub in #39581. The rebase restores the Darwin definition only, so the
comment on the extern block no longer claims a stub for it.
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