Skip to content

test(builtins-table): run the same-ids test's generators without simulated-throw traces and tighten its assertions - #47467

Closed
robobun wants to merge 3 commits into
mainfrom
robobun/49891bf2/same-ids-test-no-simulated-throw-traces
Closed

robobun wants to merge 3 commits into
mainfrom
robobun/49891bf2/same-ids-test-no-simulated-throw-traces

Conversation

@robobun

@robobun robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts takes 48 s on debian 13 x64-asan (build 135385). Two overlapping generator runs are the whole cost.
  • BUN_JSC_dumpSimulatedThrows=1, which the lane sets (scripts/runner.node.ts:2618), is 20 s of one run's 45 s (local release-asan). On Linux its trace has no frames.
  • The assertions are exit codes and one boolean: a failure shows nothing, and a generator that ignores --types-only passes.

Fix

  • The two runs get bunEnv without that variable. BUN_JSC_validateExceptionChecks stays, so an unchecked exception still aborts.
  • One toEqual covers both runs: stderr has only builtins table: summaries, exit code 0, no signal. toBe prints the id lines that differ. A new check requires two different native-call lists.
  • A debug build gets 15 minutes: its two runs take 7 to 8 minutes together.
  • Verified in CI: 29.0 s there in build 135774, 1 pass. bun bd test: 0 pass (139.6 s) before, 1 pass (415 s) after.

Background

  • src/codegen/builtins/index.ts numbers the built-ins. A prebuilt WebKit has those ids compiled in, so every build configuration must produce them.
  • validateExceptionChecks aborts when native code leaves a possible exception unchecked. dumpSimulatedThrows adds a backtrace() there. Bun's C++ has no unwind tables, so that returns one frame.
  • Considered: remove the variable lane-wide (outside this file), or generate() in-process (one numbering per process).

Downsides

  • Where backtrace() works, a failed check in these runs prints no simulated-throw trace.
  • On a debug build, a hung generator fails the test after 15 minutes.
  • If no src/js file calls a *_test.bind.ts binding, the new check fails.
Notes

CI. On debian 13 x64-asan main's file took 48.2 s in build 135385. This PR took 29.3 s in build 135753 (its first commit) and 29.0 s in build 135774 (its last commit), each with 1 pass and 3 expect() calls. The later commits change only the limit of a debug build and the comparison of the list names. The builtins table: lines of the two runs are no longer in the lane's log.

Where the 48 s go. One run of main's file with a release-asan build of f7607a3 and the variables that scripts/runner.node.ts sets on the ASAN lane, times since the start of bun test:

Event Time
imports done 0.48 s
globAllSources() done 1.84 s
both generators started 1.88 s
first builtins table: operands without uses line 27.30 s
last summary line 50.86 s
both generators ended 51.37 s
file done 51.46 s

The log of build 135385 has the same shape on debian 13 x64-asan: the first operands without uses line 27.0 s after the file's header, the summaries at 47.7 and 47.9 s, the file done at 48.2 s. So the test's own work is 2 s. The two runs are two processes already.

What one run costs. One run of the generator alone, release-asan build, --types-only as for a build without assertions:

Variables Time
all that the lane sets 44.9 s
the lane's, without the validation variables of the rows below 24.3 s
BUN_JSC_validateExceptionChecks=1 23.9 s
BUN_JSC_validateExceptionChecks=1, BUN_JSC_dumpSimulatedThrows=1 43.5 s
BUN_DESTRUCT_VM_ON_EXIT=1, LeakSanitizer on 24.7 s

BUN_JSC_randomIntegrityAuditRate and BUN_GARBAGE_COLLECTOR_LEVEL change the time by less than 1 s.

Why the trace has no frames on Linux.

  • With the option, ThrowScope::simulateThrow() calls StackTrace::captureStackTrace(100) (WebKit 8c6c963e3c, Source/JavaScriptCore/runtime/ThrowScope.cpp:96). That calls backtrace(), and drops the first two frames.
  • scripts/build/flags.ts:527 compiles C and C++ with -fno-unwind-tables -fno-asynchronous-unwind-tables on every unix target. backtrace() of glibc walks the stack with unwind tables.
  • Under gdb, the first three calls of WTFGetBacktrace in the release-asan build each return 1 frame of 102.
  • A known unchecked exception shows the result. test/no-validate-exceptions.txt lists test/js/node/test/parallel/test-repl-inspect-defaults.js. Its three REPL lines, piped to bun-asan --interactive with BUN_JSC_validateExceptionChecks=1, abort the process (exit 134) with and without BUN_JSC_dumpSimulatedThrows=1. Both reports name the two scopes (getNonIndexPropertySlot @ JSObjectInlines.h:292, requireId @ InternalModuleRegistry.cpp:371). With the variable the report has one more line, The simulated exception was thrown at:, and no frame under it. The debug build also prints no frame under that line.
  • In CI only the ASAN build gets the variable, and the only ASAN lane is Linux x64 (.buildkite/ci.ts).

The file, before and after. Time that bun test reports for the file. Each row is one pair, main's file first and then this PR's, one after the other. The container has CPU steal that changes from minute to minute, so compare within a pair.

Build and variables Before After
release-asan, the lane's variables 61.4 s 39.1 s
64.1 s 35.8 s
59.0 s 31.6 s
64.6 s 40.8 s
56.4 s 36.1 s
52.1 s 29.7 s
55.9 s 35.5 s
53.0 s 33.4 s
54.0 s 33.1 s
release-asan, none of the lane's validation variables 27.1 s 30.1 s
31.0 s 27.9 s
release (the 1.4.3-canary build of the container) 7.5 s 7.8 s
8.3 s 8.1 s
debug (bun bd test) 139.6 s, 0 pass 415 s, 1 pass

Each pair with the lane's variables is 19.6 to 28.3 s shorter. An earlier set of three pairs, with the same code and other comments, was 45.8, 44.9 and 47.7 s before and 24.9, 26.2 and 27.5 s after. The new assertions alone change nothing: 44.3, 45.2 and 46.8 s in that set.

One more release pair is left out: main's run spent 9 s before the test began, for a reason that was not looked into (19.6 s, and 8.0 s for this PR).

A debug build cannot pass main's file. One run of the generator alone takes 408 s with it, and the limit of the test is 120 s. This PR gives a debug build 15 minutes. Three runs of the file with a long enough limit took 510, 438 and 415 s, and the test in them 491, 403 and 397 s. Each has 1 pass and 3 expect() calls. The last one is of the file as it is now. With the first commit of this PR, which kept the 120 s, it ended at the limit after 138.6 s. To run the file with a local build, export what bun scripts/build/aot-paths.ts --profile=<profile> prints. CI's test machines have the engine's folder beside the binary, as aot-engine.

Assertions.

Before After
How a run ends expect(await generator.exited).toBe(0). stderr goes to the terminal. stderr is read. Its lines without the builtins table: summaries, the exit code and the signal of both runs are one toEqual.
The ids expect(withAssertions === withoutAssertions).toBe(true) expect(..).toBe(..) on the two texts of aot_builtin_ids.rs
Two builds not checked BunAOTHostBindings.cpp names the list of native calls of its build (Bun__js2nativeList_<hash>, the stamp of src/codegen/generate-js2native.ts). The names in the two files, as sets, must differ.

stdout stays ignored: the generator writes its files into --codegen-path and prints on stderr only.

Checks of the assertions, each with a changed generator:

  • nativeCalls() ignores --types-only (isTypesOnly = false in src/codegen/builtins-manifest-model.ts). Then both runs are one build. Main's file passes. This file fails: Expected: not Set { "Bun__js2nativeList_5b66e219", }.
  • A binding of a types-only schema gets no entry (if (isTypesOnly) continue;). Main's file fails with Expected: true, Received: false. This file fails with two hunks: NAME_COUNT is 5592 and 5706, and BuiltinName::prototype is Self(4339) and Self(4420).
  • generate() prints one more line on stderr, and throws for the build with assertions. Main's file fails after 0.4 s with Expected: 0, Received: 1, and the other generator runs on after bun test has ended. This file waits for both and shows the line under both builds, and the error and exit code 1 under withAssertions. So a failed run is reported only when the other run has ended.

What this PR leaves alone.

  • The other files of test/internal/builtins-table that run the generator hand it bunEnv as it is. every-build-configuration.test.ts runs these two command lines with BUN_JSC_dumpSimulatedThrows.
  • scripts/runner.node.ts sets the variable for every file on the lane. What it costs there for other files was not measured here.
  • generate() in-process: nativeCalls() keeps one numbering per process (src/codegen/builtins-manifest-model.ts:93), so a second call is not a second build. Two calls on one thread would also take the time of two runs.

[auto-merge] gate passed · iteration 2 · 1 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts
bun test v1.4.4 (367d939d9)

test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts:
(pass) one prebuilt engine serves a build with and a build without assertions [380761.31ms]

 1 pass
 0 fail
 3 expect() calls
Ran 1 test across 1 file. [398.08s]
Exit: 0
diff hotspot
.../same-ids-in-every-build-configuration.test.ts  | 40 ++++++++++++++++------
 1 file changed, 30 insertions(+), 10 deletions(-)

gate history · 1 passed · 1 rejected · iteration 2

evidence per changed file
file                                                      reads  edits  tests
…ins-table/same-ids-in-every-build-configuration.test.ts      3      2     33

…lated-throw traces and tighten its assertions

The two generator runs get bunEnv without BUN_JSC_dumpSimulatedThrows.
The ASAN lane sets that variable, and it takes 20 s of the 45 s of one
run there. BUN_JSC_validateExceptionChecks stays.

The test asserts the stderr lines, the exit code and the signal of both
runs in one toEqual. It compares the two id files with toBe, which
prints the lines that differ. It also requires that the two builds
number different lists of native calls, so a generator that ignores
--types-only fails the test.
@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:08 AM PT - Oct 30th, 2026

❌ @robobun, your commit 6a91d04 has 2 failures in Build #135774 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 47467

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

bun-47467 --bun

@robobun

robobun commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. The diff is green. CI is red for files that this PR does not touch.

  • The changed file has 1 pass on debian 13 x64-asan in builds 135753 and 135774. It is not among the failures of either build.
  • Build 135774 is red for test/aot/engine/ir-6.test.ts on five lanes: alpine 3.23 x64 and aarch64, debian 13 x64-asan, windows 2019 x64 and windows 11 aarch64. The same jobs failed in build 135385, the final run of Remove 146 unsafe blocks from the dev server and the Bun.* APIs #47393, which is the last commit on main. On debian 13 x64-asan that run has the same message for pipeline/gc/custom-accessor-reached-computed-key.ir.
  • Build 135774 is also red for test/js/web/request/request-clone-leak.test.ts (debian 13 x64-asan): an RSS delta of 64 MB against a limit of < 64.
  • Every other failure of the build passed on a retry.

How this was measured:

  • CI, debian 13 x64-asan: 48.2 s for main's file in build 135385. 29.3 s and 29.0 s for this PR in builds 135753 and 135774.
  • A release-asan build of f7607a3 on Linux x64, with the variables that scripts/runner.node.ts sets on the ASAN lane: 52 to 65 s before and 30 to 41 s after, in nine interleaved pairs.
  • bun bd test: 0 pass in 139.6 s before, because the test ends at its 120 s limit. 1 pass in 415 s after, because a debug build now has 15 minutes. With a debug build the two runs of the generator take 7 to 8 minutes together.

To run the file with a local build, export what bun scripts/build/aot-paths.ts --profile=<profile> prints.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The test now generates assertion-enabled and assertion-disabled outputs in separate directories. It checks that both runs succeed without unexpected stderr, that their native-call list identifiers differ, and that their generated aot_builtin_ids.rs files match.

Changes

Builtins table test

Layer / File(s) Summary
Generate and compare build outputs
test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts
The test generates output for both build configurations, checks process results and stderr, verifies that native-call list identifiers differ, and checks that the generated aot_builtin_ids.rs files match.

Suggested reviewers: jarred-sumner

Priority: Low

Merge Risk: Minimal · up to 67f14

This test-only change strengthens generator comparisons and suppresses simulated-throw traces. No concrete merge-blocking regression is established, although the two output-format concerns remain unverified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the test, the removal of simulated-throw traces, and the stronger assertions. It is specific and directly related to the main changes.
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.
Description check Passed The description provides a clear change summary and extensive verification evidence. It uses different headings from the template, but it covers both required topics: what the PR changes and how the c…
Full details: Description check

Explanation

The description provides a clear change summary and extensive verification evidence. It uses different headings from the template, but it covers both required topics: what the PR changes and how the changes were tested.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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

Additional comments: 2
test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts-40-40: 1

40-40: Minor · Potential issue · Low value

Unverified finding
We couldn't verify this from the code. It is shown for review, not as a verified issue.

nativeCallListOf takes the first matching identifier anywhere in BunAOTHostBindings.cpp, rather than identifying the list the generated bindings actually use. Multiple identifiers in that file could make the assertion compare the wrong lists.

The test could pass when the two builds use the same active native-call list, or fail when their active lists differ.

This can occur when: Each generated file contains an earlier identifier, such as one in a comment or secondary declaration, before its active native-call-list identifier.

test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts-35-35: 1

35-35: Minor · Potential issue · Low value

Unverified finding
We couldn't verify this from the code. It is shown for review, not as a verified issue.

The new filter treats every stderr line beginning with builtins table: as a summary. A generator diagnostic using that prefix could therefore be excluded from errors.

The test could accept a run that reported a generator error.

This can occur when: The generator reports a failed or incomplete operation on a builtins table: line but exits zero.


Review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between f7607a3 and 67f14ff.

Files selected for processing: 1
  • test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

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

…ild and compare every native-call list name

One run of the generator takes about 7 minutes with a debug build, so
the test could not pass there within 120 s. A debug build has 30 minutes
now. Every other build keeps 120 s.

The check that the two builds number different lists of native calls
compares every Bun__js2nativeList_<hash> name of each
BunAOTHostBindings.cpp as a Set, not only the first one.
@robobun

robobun commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

Both findings are checked.

  1. nativeCallListOf and several identifiers. The emitter takes one stamp and writes it twice: the extern "C" declaration and the nativeCallListStamp reference (aot/builtins/generator/emit-cpp-bindings.ts:404 and :409 of WebKit 8c6c963e3c). So each BunAOTHostBindings.cpp names one list, and the first match was that list. d1c78e1 makes the check independent of that: it compares every identifier that each file names, as a Set.

  2. The builtins table: filter. No change. The generator prints two lines with that prefix in every run, and both are reports: operands without uses (aot/builtins/generator/build-model.ts:33) and the count of results that are not described (src/codegen/builtins/index.ts:385). An error does not use the prefix: exitWithError prints error: ... and exits with code 1, and any other error is thrown. every-build-configuration.test.ts filters the same prefix.

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

…sured time

With a debug build the two generator runs took 397, 403 and 491 s
together. The limit for a debug build is 15 minutes, not 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.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: the test file this PR changes, test/internal/builtins-table/same-ids-in-every-build-configuration.test.ts, was removed on main by #47316 (a47a315), so this change no longer applies.

  • That commit moved the test into every-build-configuration.test.ts, where it shares the two generator runs with the other tests of the file.
  • Those runs do not have the cost that this PR removed. generatorEnv in test/internal/builtins-table/engine-files.ts sets BUN_JSC_dumpSimulatedThrows to 0 for every generator run of the folder (AOT: front end, test runs and tests for the rewritten compiler #47942).
  • The assertions of this PR are not on main. The test still compares the two id files with ===, so a failure shows no difference.

@robobun robobun closed this Nov 8, 2026
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.

1 participant