Repository navigation
test(aot): share one build and run the tests of entry-point-names.test.ts concurrently #47463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,15 +2,18 @@ | |
| // it under any extension of JavaScript or TypeScript. Programs start workers and import modules by those names, and tools read | ||
| // them from the executable. With --aot the same names find the same modules. What a module shows as its own path is another | ||
| // matter: that is the path of its source below the virtual root. | ||
| import { expect, setDefaultTimeout } from "bun:test"; | ||
| import { afterAll, expect, setDefaultTimeout } from "bun:test"; | ||
| import { bunEnv, bunExe, isWindows, tempDir } from "harness"; | ||
| import { readFileSync } from "node:fs"; | ||
| import { readdirSync, readFileSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { built, spawned } from "./harness/commands"; | ||
| import { test } from "./supported"; | ||
| import { test as oneAtATime } from "./supported"; | ||
|
|
||
| setDefaultTimeout(240_000); | ||
|
|
||
| // Each test has folders of its own. The two that ask one executable only read it. | ||
| const test = oneAtATime.concurrent; | ||
|
|
||
| const virtualRoot = isWindows ? "B:/~BUN/root/" : "/$bunfs/root/"; | ||
| const stems = { typed: "ts", module: "mjs", plain: "js", common: "cjs" }; | ||
| const extensions = ["js", "ts", "mjs", "cjs"]; | ||
|
|
@@ -91,12 +94,12 @@ const additional = [ | |
| const expected = ["absolute", "relative"] | ||
| .flatMap(kind => [ | ||
| ...Object.entries(stems).flatMap(([stem, written]) => | ||
| extensions.map(extension => `${kind} ${stem}.${extension} ${stem}.${written}`), | ||
| extensions.map(extension => `${kind} ${stem}.${extension} ${stem}.${written}\n`), | ||
| ), | ||
| `${kind} worker.js worker.ts`, | ||
| `${kind} worker.ts worker.ts`, | ||
| `${kind} worker.js worker.ts\n`, | ||
| `${kind} worker.ts worker.ts\n`, | ||
| ]) | ||
| .join("\n"); | ||
| .join(""); | ||
|
|
||
| async function build(folder: string, flags: string[], entryPoints: string[], name: string): Promise<string> { | ||
| const executable = join(folder, name + (isWindows ? ".exe" : "")); | ||
|
|
@@ -106,22 +109,33 @@ async function build(folder: string, flags: string[], entryPoints: string[], nam | |
| return executable; | ||
| } | ||
|
|
||
| const shared = new DisposableStack(); | ||
| afterAll(() => shared.dispose()); | ||
|
|
||
| for (const [title, flags] of [ | ||
| ["bundled", []], | ||
| ["compiled", ["--aot"]], | ||
| ] as const) { | ||
| // One test asks what this executable prints, one what its file holds: one build, in a folder that outlives both. | ||
| let once: Promise<string> | undefined; | ||
| function withEveryEntryPoint(): Promise<string> { | ||
| if (!once) { | ||
| const dir = shared.use(tempDir("aot-entry-point-names", files)); | ||
| once = build(String(dir), [...flags], ["src/main.ts", ...additional], title); | ||
|
Comment on lines
+121
to
+124
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Developers running this file on a debug build with few cores get spurious failures: a --aot build killed at 90 s. The file now starts up to 12 builds at once (test/aot/entry-point-names.test.ts:127 and every build() call) with no slot() bound, unlike in-place-executables.test.ts:15-20 which gates each command on a processor. spawned() kills a child at limitOfARun = 90 s (test/aot/harness/commands.ts:58) when not ASAN. Fix: wrap each build() in slot() from ./harness/commands so concurrent builds never exceed availableParallelism, matching the sibling file. Why this was flaggedTrigger: Verification: Trigger: a direct run on a debug Bun without ASAN on a machine with few cores. test/aot/entry-point-names.test.ts:15 makes 11 tests start together. Each build calls
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 1c05c67. Each command now goes through I checked the case with a debug build on 2 cores and 20 concurrent tests. Without the slots, the tests with an
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correction to my reply above: I removed the slots again in 852c22d. This note is not addressed. The slots help in the case you describe (2 cores, debug build: longest command 49 s, against 112 to 122 s without them). In CI they cost time. On ubuntu x64, ubuntu aarch64 and debian aarch64 the file went from 2.5 to 3.2 s (build 135747) to 5.8 to 6.6 s (build 135759), while the eight files after it on the same shards stayed within about 5%. A machine with four processors has four slots and the file has five That is one run for each lane, and debian x64 went the other way (5.5 s to 4.0 s). The next CI run shows whether the three lanes return to about 3 s. So a debug build without a sanitizer on few processors can still reach the 90 s limit of
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The CI run I pointed to is in: 4.2 s, 3.1 s and 4.7 s on the three lanes without the slots (they were 6.3 s, 5.8 s and 6.6 s with them, and 3.2 s, 2.5 s and 3.0 s in the first build). The lanes came down, but two did not return to about 3 s. The details are in my reply on the newer thread at line 15. |
||
| } | ||
| return once; | ||
| } | ||
|
|
||
| test(`${title}: an entry point is found under the name of its output and under every extension`, async () => { | ||
| using dir = tempDir("aot-entry-point-names", files); | ||
| using elsewhere = tempDir("aot-entry-point-names-elsewhere", {}); | ||
| const executable = await build(String(dir), [...flags], ["src/main.ts", ...additional], title); | ||
| const run = await spawned([executable], String(elsewhere), bunEnv); | ||
| expect({ stdout: run.stdout.trim(), exitCode: run.exitCode }).toEqual({ stdout: expected, exitCode: 0 }); | ||
| const run = await spawned([await withEveryEntryPoint()], String(elsewhere), bunEnv); | ||
| expect(run).toEqual({ stdout: expected, stderr: "", exitCode: 0, signal: null }); | ||
| }); | ||
|
|
||
| test(`${title}: the executable lists its entry points under the names of their outputs`, async () => { | ||
| using dir = tempDir("aot-entry-point-names-list", files); | ||
| const bytes = readFileSync(await build(String(dir), [...flags], ["src/main.ts", ...additional], title)); | ||
| const isListed = (name: string) => bytes.includes(Buffer.from(virtualRoot + name + "\0")); | ||
| const bytes = readFileSync(await withEveryEntryPoint()); | ||
| // The file is the size of Bun and the names are near its end. | ||
| const isListed = (name: string) => bytes.lastIndexOf(Buffer.from(virtualRoot + name + "\0")) !== -1; | ||
| const names = [...Object.keys(stems), "worker"].map(stem => `sub/${stem}.js`); | ||
| expect(names.filter(name => !isListed(name))).toEqual([]); | ||
| }); | ||
|
|
@@ -131,9 +145,11 @@ for (const [title, flags] of [ | |
| using elsewhere = tempDir("aot-entry-point-names-same-stem-elsewhere", {}); | ||
| const executable = await build(String(dir), [...flags], ["./same.js", "./same.ts"], title); | ||
| const run = await spawned([executable], String(elsewhere), bunEnv); | ||
| expect({ stdout: run.stdout.trim(), exitCode: run.exitCode }).toEqual({ | ||
| stdout: "same.js\nimport same.ts\nworker same.ts", | ||
| expect(run).toEqual({ | ||
| stdout: "same.js\nimport same.ts\nworker same.ts\n", | ||
| stderr: "", | ||
| exitCode: 0, | ||
| signal: null, | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -147,12 +163,12 @@ for (const [title, flags] of [ | |
| const others = ["./page.css", "./page.html"]; | ||
| const executable = await build(String(dir), [...flags], [...others, "./first.js", "./second.js"], title); | ||
| const run = await spawned([executable], String(dir), bunEnv); | ||
| expect({ stdout: run.stdout.trim(), exitCode: run.exitCode }).toEqual({ stdout: "first.js true", exitCode: 0 }); | ||
| expect(run).toEqual({ stdout: "first.js true\n", stderr: "", exitCode: 0, signal: null }); | ||
|
|
||
| const command = [bunExe(), "build", "--compile", ...flags, ...others, "--outfile", join(String(dir), "none")]; | ||
| const refused = await spawned(command, String(dir), bunEnv); | ||
| expect(refused.exitCode).not.toBe(0); | ||
| expect(refused.stdout + refused.stderr).toContain("No output files to bundle"); | ||
| expect(refused).toMatchObject({ stderr: expect.stringContaining("No output files to bundle"), exitCode: 1 }); | ||
| expect(readdirSync(String(dir)).filter(name => name.startsWith("none"))).toEqual([]); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -161,17 +177,19 @@ test("compiled: a Worker starts from the source it names beside an entry point w | |
| using elsewhere = tempDir("aot-entry-point-names-worker-elsewhere", {}); | ||
| const executable = await build(String(dir), ["--aot"], ["./main.ts", "./task.ts"], "worker"); | ||
| const run = await spawned([executable], String(elsewhere), bunEnv); | ||
| expect({ stdout: run.stdout.trim(), exitCode: run.exitCode }).toEqual({ stdout: "task.js", exitCode: 0 }); | ||
| expect(run).toEqual({ stdout: "task.js\n", stderr: "", exitCode: 0, signal: null }); | ||
| }); | ||
|
|
||
| test("compiled: a module shows the path of its source", async () => { | ||
| using dir = tempDir("aot-entry-point-names-shown", files); | ||
| using elsewhere = tempDir("aot-entry-point-names-shown-elsewhere", {}); | ||
| const executable = await build(String(dir), ["--aot"], ["src/shown.ts", "src/sub/typed.ts"], "shown"); | ||
| const run = await spawned([executable], String(elsewhere), bunEnv); | ||
| expect({ stdout: run.stdout.trim(), exitCode: run.exitCode }).toEqual({ | ||
| stdout: `${virtualRoot}shown.ts\n${virtualRoot}sub/typed.ts`, | ||
| expect(run).toEqual({ | ||
| stdout: `${virtualRoot}shown.ts\n${virtualRoot}sub/typed.ts\n`, | ||
| stderr: "", | ||
| exitCode: 0, | ||
| signal: null, | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -196,7 +214,11 @@ test("compiled: two entry points with one output name are an error, as in a bund | |
| String(dir), | ||
| bunEnv, | ||
| ); | ||
| expect(result.exitCode).not.toBe(0); | ||
| expect(result.stderr).toContain("Multiple files share the same output path"); | ||
| // The error names the output that the two share. | ||
| expect(result).toMatchObject({ | ||
| stderr: expect.stringMatching(/Multiple files share the same output path[^]*\bsub[\\/]typed\.js\b/), | ||
| exitCode: 1, | ||
| }); | ||
| expect(readdirSync(String(dir)).filter(name => name.startsWith("equal"))).toEqual([]); | ||
| } | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 (optional) The issue filed at test/aot/entry-point-names.test.ts:124 is back: commit 852c22d removed the slot() wrapper, so developers on a debug Bun without ASAN and few cores again get --aot builds killed at 90 s. With test.concurrent at line 15 all 11 tests start together and build() at line 107 spawns 12 builds with no slot; spawned() kills a build at limitOfARun = 90_000 (test/aot/harness/commands.ts:58) when isASAN is false. Fix: keep every build in this file behind slot() (or another bound on concurrent builds) so no single --aot debug build waits past 90 s on 2-4 cores, while keeping the runs of executables unbounded if the CI timing matters.
Why this was flagged
Trigger:
bun bd --asan=off test test/aot/entry-point-names.test.tson a machine with 2 to 4 cores. test/aot/entry-point-names.test.ts:15 makes every test concurrent and bun test's default max-concurrency is 20, so all tests start at once and build() at line 107 calls spawned() for each of 12 builds, 5 of them with --aot, with no slot() gate. In test/aot/harness/commands.ts:73-79 the timeout is limitOfARun = 90_000 (line 58) because isASAN is false and moreBuildFlags is empty; Bun.spawn kills the child with SIGKILL at line 89 and built() at commands.ts:170-175 fails on the exit code. The PR's own notes measured that without slot() five --aot debug builds on 2 cores take 112 to 122 s each, over that limit. On the base branch the tests ran in series, one build at a time, so no build competed for cores. Commit 1c05c67 had applied the remedy the resolved entry at line 124 asked for; commit 852c22d removed it for CI speed on 4-core release-profile lanes, which does not cover the non-ASAN debug case.Verification: 852c22d deletes
const spawned = (...command) => slot(() => spawnedAtOnce(...command)); build() at test/aot/entry-point-names.test.ts:107 calls it with no gate. test/aot/harness/commands.ts:58limitOfARun = 90_000, chosen at lines 73-79 whenisASANis false; built() at 170-175 fails on the nonzero exit code. On the base the file used the serialtest, so this is new.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct, and it is open. I have not pushed a change for it yet, for two reasons.
The reason I gave for removing the slots is not confirmed. With the slots, three CI lanes with four processors took about twice as long (one run for each lane). My explanation is a local trace: four slots, five
--aotbuilds, so the fifth build starts when the first ends. Build 135810 runs the head without the slots. If those lanes do not return to about 3 s, the explanation is wrong and I put the slots back.If the explanation holds, slots for the builds alone keep the cost. The command that waits in the trace is the fifth
--aotbuild, not a run. A bound equal to the number of processors protects a machine with 2 processors and costs a build length on a machine with 4. A larger bound removes the cost and, on 2 processors, lets four--aotbuilds run at once, which I have not measured.One limit of my own numbers: the 112 to 122 s are from a debug build with ASAN and
--max-concurrency=20, used in place of a build without a sanitizer. I have not run a debug build without ASAN.The candidate I see is a gate that applies only to a debug build without a sanitizer, since that is the one configuration with the 90 s limit and slow builds. I will not push it before I can run such a build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build 135810 has the three lanes. Same shard in each build, one run each:
I wrote that I put the slots back if these lanes do not return to about 3 s. Two of them did not. As I worded it, the condition is not met.
I keep the slots out all the same, for this reason: the file got slower on all three lanes when the slots went in and faster on all three when they came out, by about 2 s or more each time. The two builds without slots differ by 0.6 to 1.7 s on the same lane, so the direction is clearer than the size. I cannot say how much the slots cost. "Twice as long" was too strong.
This note is still not addressed. A debug build without a sanitizer on few processors can reach the 90 s limit with this file.