Repository navigation
Conversation
…he-build-knows.test.ts and state what each command prints The programs of this file hand console.log one text. A value of unknown type made six builds compile the code that formats any value: 436 functions of Bun's library, not 66. The three ways to start a Worker from what the program makes at run time are in one program. The first argument of a run selects the way. That program is built once. A fourth way starts a Worker from the text that the build knows, to show that the executable holds that text. Each run of plain Bun and of an executable states its stdout, an empty stderr, the exit code and the signal. A build states its warnings and errors by code and place. A run that must stop at the interpreter names the Worker that it stops at.
|
Updated 5:37 AM PT - Oct 31st, 2026
❌ @robobun, your commit 7115b59 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 47536That installs a local version of the PR into your bun-47536 --bun |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Build helper and executable checks test/aot/workers-from-texts-that-the-build-knows.test.ts |
The shared build helper accepts entry-point arrays and executable names. Tests check build diagnostics, executable output, and compiled program counts. |
Runtime-created Worker texts test/aot/workers-from-texts-that-the-build-knows.test.ts |
Tests check runtime-created Worker messages and interpreter-forbidden failures. A Worker text known to the build remains executable when the interpreter is forbidden. |
Worker path resolution test/aot/workers-from-texts-that-the-build-knows.test.ts |
Tests compare exact output for package, workspace, relative, normalized, scoped-package, and missing Worker paths. |
Worker diagnostics and build failures test/aot/workers-from-texts-that-the-build-knows.test.ts |
Tests assert diagnostic codes and locations for failed imports and invalid data URLs. Type errors must fail the build without writing an executable. |
Possibly related issues
oven-sh/bun#47237: The tests cover compiled Worker path resolution, but the summary does not establish that they address the issue’s subfolder case.oven-sh/bun#40903: The tests exercise embedded Worker loading in AOT builds, but the issue concerns--bytecode, and coverage of its failing path is not established.
Suggested reviewers: jarred-sumner
Merge Risk: 
Minimal · up to 7115b
-
The changes consolidate AOT Worker test builds and strengthen output and diagnostic assertions. No merge-blocking issue is identified; merge after normal checks pass.
Pre-merge checks: 
4
-
Full details: Description check
-
Explanation
The description follows the repository template, which has two sections: "What does this PR do?" and "How did you verify your code works?". The Problem, Fix, and Background subsections explain the purpose of the change, and the Fix section and the Notes block include measured build counts, CPU times, and test run results, so the verification section is filled in. The description is mostly complete and states the intended behavior change (none) and the test results. Gaps: the description is very long and dense, and it does not state the macOS timing (the author says it was not measured locally, and the CI table is the only macOS evidence). The description also refers to unrelated PR numbers (
#47129,#47316,#47463,#47468) without a clear statement of how they relate to this change, and it lacks a short plain summary at the top. These gaps are non-critical, so the check passes.
-
- 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.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Since the exact stdout listings and warning ordering were only verified on Linux per the description, and dropping the value formatter from six fixtures is a deliberate coverage trade-off, a human look from an aot test maintainer would still be worthwhile.
What was reviewed:
- The shared
builtWithEveryWay()memo: itsslot()is released before each of the three tests takes its own, so no slot deadlock; a failed build rejects all three tests (as the description states) andafterAlldisposes the folder. said()/placeOf(): thewarn:/error: TSnnnn ... \n at <place>shape matches what sibling aot tests already parse;.does not match\rso CRLF lines still match; data: URLs are kept whole, file paths reduced withbasename.- Every replaced assertion is strictly tighter than base (exact
{stdout, stderr, exitCode, signal}, exact warning lists,.exe-aware size comparison that was vacuous on Windows before,existsSync(file)false on a failed build). - The crash line
crashIfInterpreterIsUsed: Worker <specifier>is what src/jsc/bindings/BunAOTRuntime.cpp prints, so the per-way regexes can match.
Extended reasoning...
The change rewrites one test file, test/aot/workers-from-texts-that-the-build-knows.test.ts (222 insertions, 117 deletions), with no production code touched and no security-sensitive surface. It merges three per-text builds into one memoized shared executable selected by argv, changes fixture console.log calls to single-string form so the AOT build compiles fewer library functions, and replaces partial matches with exact object comparisons via new helpers (ended, said, placeOf, builtSaying). The bug hunt ran dry with no findings and five candidates ruled out, and no CODEOWNER covers test/aot. Defer rather than approve because the restructuring is not mechanical, the exact-output and warning-order assertions were verified only on Linux by the author, and the reduced fixture coverage of the value formatter is a judgment call for a maintainer.
|
On the two points of the review: Platforms other than Linux. Build 136054 runs this file on every lane. It passes on the seven lanes that have finished it: debian 13 x64 and aarch64, ubuntu 25.04 x64 and aarch64, alpine 3.23 x64 and aarch64, and debian 13 x64-asan. The Windows and macOS shards have not reached the file yet. Their results go into the description when they finish. The value formatter. Two programs of this file still make the build compile it. The program that imports |
|
Closing: #47942 rewrites the front end of the ahead-of-time compiler and its tests for the engine's new compiler (oven-sh/WebKit#997, merged), so this no longer applies and would conflict with it. If the problem still shows once that is in, please open a new pull request against |
|
Understood. This pull request stays closed. Two findings here do not depend on the diff, for whoever looks at this file after #47942. I measured both on main at 34cbc70, so the numbers can differ with the new compiler.
The description above has the measurements. |
Behaviour change: none
Problem
test/aot/workers-from-texts-that-the-build-knows.test.tstakes 64 s on darwin x64 (build 135961). Builds take 98% of its CPU time.console.log("message", event.data). For a value of unknown type the build compiles Bun's value formatter: 436 functions, not 66.Fix
console.logone text and print the same lines.bun bd test test/aot/workers-from-texts-that-the-build-knows.test.tstakes 117 to 164 s, was 142 to 271 s. CI, beside the same files: 12% to 39% less time on 10 lanes. darwin x64 ran the file alone (14.5 s). macOS is not measured locally.Background
bun build --compile --aotand starts it. The build compiles the library code that the program reaches.BUN_JSC_crashIfInterpreterIsUsed=1ends a process where it interprets code.--aot-separate-programs: canary builds only.Downsides
test/aot/parallel/workersdo.node:worker_threadsbuild is unchanged: a third of the CPU time now. 12 local vCPUs gain nothing (7.6 s, 7.4 s).Notes
Where the time goes. Release build (canary
34cbc70fa), Linux x64. Each build alone, one at a time, best of 3. CPU is user plus system time. The functions and the engine runs are from--aot-report.node:worker_threadsWhat
console.logcosts a build. A program of two lines that starts one Worker and prints in itsonmessage:console.log("message " + event.data)console.log(`message ${event.data}`)console.log("message", "text")console.log(1 + 1)console.log(event.data)console.log("message", event.data)132 of the 371 functions more are in
internal:util/inspect. The rest is what it reaches: iterator helpers,Array.prototype, the sort,node:url.Buffer.from(text).toString()brings the same code, so the shared program still compiles 446 functions. #47129 tooknode:worker_threadsout of the programs of this file that are not about it, for the same reason.All local times. Linux x64, 12 vCPUs. The host takes a changing part of the CPU time:
/proc/statcounts 19 s of steal while 12 builds run at once for 2.6 s. So the rows withtasksetare the stable ones. Main's file and this PR's ran in turns.bun bd test <file>)taskset -c 0-3)taskset -c 0-1)bun-profileofbun run build:releasewith the variables thatscripts/runner.node.tssets:BUN_GARBAGE_COLLECTOR_LEVEL=1,BUN_JSC_randomIntegrityAuditRate=1.0,BUN_FEATURE_FLAG_NO_ORPHANS=1.node:worker_threadsbuild ends. That build takes 3.1 to 4.4 s alone and longer beside the other builds.expect()calls and this PR's has 59.macOS. This machine is Linux x64, so no macOS time is measured locally. The table of CI below has both macOS lanes. In build 135961 the file ran on a darwin x64 agent beside three other files that build executables. #47463 and #47468 measured that the first start of a new executable waits 1 to 3 s on such agents. This file starts 11 new executables now, was 13.
CI, like for like. The runner puts a test file that a PR changes in front and runs it alone (
modifiedSerialTestsinscripts/runner.node.ts). In build 136054 it could not do that on 10 of the 11 lanes: there the GitHub API answered 403 (rate limit) to the request for the files that this PR changes. So on those lanes this file ran in its usual group, beside other files that build executables. Build 135961 is a build of another PR, with main's file. In both builds the file is in the same shard, in the same group of files. Seconds for this file, one run each:expect()calls.The shared program. Its second line starts a Worker from a text that the build knows, as before. Then
ways[process.argv[2]]()starts one of four Workers:Buffer.from(text).toString()makes at run time: adata:URL, the text of aBlob, the type of aBlob. Each run printsmessage 1ormessage 2. With the interpreter forbidden each stops withcrashIfInterpreterIsUsed: Worker <URL of that Worker>and prints nothing.message 1. So the executable holds that text, and a run that stops has stopped at the Worker of its own way.Why the three texts that are left out keep a build each. A build with one such text runs the engine 4 times for a syntax error and 3 times for an import that is not found. A build with all three texts in one program runs it 3 times. So one text changes what the build does for another, and a shared build would not test what each build alone does.
Assertions, before and after.
: error. After: every line, an empty stderr, exit code 0, no signal.message 1ormessage 2for any of the three. After: stdout, an empty stderr, exit code 0 and no signal, and the one message of each way.crashIfInterpreterIsUsed. After: the line names the Worker (data:text/javascript,postMessage(1)or ablob:URL), stdout is empty, and the exit code is not 0.warn: <code>andwarn: TS100072. After: the first warning has the code and the line and column in the file or text. TheTS100072warning names the line and column of thenew Workerthat starts from it. Every other warning is about that text.error. After: also no warning with a code.error: TS2322and noTS100072, exit code 1. After: the one message isTS2322at line 1, column 5, inworker.tsfor the file. Exit code 1, no signal, and no executable is written. For the text only:1:5is compared: the build names the text/$bun/plugin/entrytext/data%3atext/javascript%2c...there.Bun.file(path).sizeof both executables is equal. That is 0 for a path that does not exist, and on Windows the files areprogram.exeandlisted.exe. After:statSyncof the file with.exeon Windows, and both builds report the same number of engine runs.toEqual, and most of these are rows oftest.each.The assertions fail when they must. Each line is one change to this PR's file, run with the release build:
inner.js:1:2forinner.js:1:1in the expected warning: the test of the file that is left out fails.1:13for1:14as the place of the syntax error: its test fails.new Blob(["postMessage(2)"])withoutmade(): the build knows the text, the run does not stop, and the test fails at the missingcrashIfInterpreterIsUsedline.worker.tswithout its type error: the test fails on exit code 0, an empty list of messages and an executable that was written.Not done.
node:worker_threadsprogram. A program of one line,import { isMainThread } from "node:worker_threads", compiles 2338 functions and takes 8 s of CPU time to build. The test asserts that the engine runs once for it, so the build cannot be smaller.made()withTextEncoderandTextDecoder: a program of one way then compiles 66 functions, not 442. It was not taken:Buffer.from(text).toString()is how the file makes a text that the build cannot know.Seen in bun, not changed here.
EngineRunsin--aot-report). The same program without the bad Worker takes 2. The three builds of that kind are 3.8 s of the 24.9 s./$bun/plugin/entrytext/data%3atext/javascript%2c...:1:5). A warning about a text names it by its URL (data:text/javascript,postMessage((:1:14).Also run.
BUN_JSC_validateExceptionChecks=1,BUN_DESTRUCT_VM_ON_EXIT=1,ASAN_OPTIONS,LSAN_OPTIONS): 16 pass in 128.8 s.bun test --parallel=3 --max-concurrency=2witha-worker-in-a-loop-is-terminated-under-any-inherited-signal-mask.test.tsanda-program-that-starts-itself-as-a-worker-ends-every-time.test.tspasses.AOT_PROGRAMS_JOBS=1) and with-tfor one of the three tests of the shared program: pass, and the folder of the shared program is removed.test/aot/test-files-self-skip.test.ts,test/aot/every-test-runs-in-ci.test.tsandtest/internal/source-lintspass.bun fmt --check,oxlintandtscreport nothing for the file.tscreports the same four errors intest/aot/supported.tsas on main.