Repository navigation
fix: clear dead stack before asynchronous GC requests #209
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
e0e9d4b
fix: clear dead stack before asynchronous GC requests
steipete d84380b
bench: measure paired asynchronous GC cost
steipete 053c23e
chore: merge current main into async GC fix
steipete 6ee63f0
bench: request GC before each worker park
steipete 6b7c19f
perf: bulk-clear dead stack before garbage collection
steipete 760c745
fix: keep asynchronous GC stack clearing observable
steipete f08cee2
fix: use JSC pointer barrier for stack clearing
steipete 3f65cff
perf: use platform zeroing for asynchronous GC
steipete File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| // node bench/gc-async-stack.mjs <baseline-bun> <candidate-bun> <output-directory> | ||
| import assert from "node:assert/strict"; | ||
| import { spawnSync } from "node:child_process"; | ||
| import { createHash } from "node:crypto"; | ||
| import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; | ||
| import { arch, cpus, platform } from "node:os"; | ||
| import { join, resolve } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { Worker } from "node:worker_threads"; | ||
|
|
||
| const script = fileURLToPath(import.meta.url); | ||
| const cpuMs = usage => (usage.user + usage.system) / 1000; | ||
|
|
||
| async function measure(name) { | ||
| assert(process.versions.bun, "Run benchmark cases with the selected Bun executable"); | ||
| if (name === "gc-loop") { | ||
| for (let i = 0; i < 2000; i++) Bun.gc(false); | ||
| Bun.gc(true); | ||
| const cpu = process.cpuUsage(); | ||
| const start = performance.now(); | ||
| for (let i = 0; i < 200000; i++) Bun.gc(false); | ||
| return { iterations: 200000, elapsedMs: performance.now() - start, cpuMs: cpuMs(process.cpuUsage(cpu)) }; | ||
| } | ||
|
|
||
| assert.equal(name, "idle-worker"); | ||
| const worker = new Worker( | ||
| `const { parentPort } = require("node:worker_threads"); | ||
| async function turn(i) { | ||
| globalThis.payload = new Array(524288).fill(i); | ||
| // The controller can throttle after quiet ticks; request collection before every park. | ||
| Bun.gc(false); | ||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
| } | ||
| (async () => { | ||
| for (let i = 0; i < 25; i++) await turn(i); | ||
| Bun.gc(true); | ||
| parentPort.postMessage("ready"); | ||
| parentPort.once("message", async () => { | ||
| const start = performance.now(); | ||
| for (let i = 0; i < 500; i++) await turn(i); | ||
| parentPort.postMessage({ iterations: 500, elapsedMs: performance.now() - start, | ||
| checksum: payload[0] + payload.length }); | ||
| parentPort.close(); | ||
| }); | ||
| })().catch(error => { throw error; });`, | ||
| { eval: true }, | ||
| ); | ||
| let measured; | ||
| await new Promise((resolve, reject) => { | ||
| let cpu; | ||
| worker.on("error", reject); | ||
| worker.on("message", message => { | ||
| if (message === "ready") { | ||
| cpu = process.cpuUsage(); | ||
| worker.postMessage("go"); | ||
| } else { | ||
| measured = { ...message, cpuMs: cpuMs(process.cpuUsage(cpu)) }; | ||
| } | ||
| }); | ||
| worker.on("exit", code => { | ||
| if (code !== 0 || !measured) reject(new Error(`Worker exited with ${code} before completing the workload`)); | ||
| else resolve(); | ||
| }); | ||
| }); | ||
| assert.equal(measured.checksum, 524288 + 499); | ||
| return measured; | ||
| } | ||
|
|
||
| if (process.argv[2] === "--case") { | ||
| console.log(JSON.stringify(await measure(process.argv[3]))); | ||
| } else { | ||
| const [baselineArg, candidateArg, outputArg] = process.argv.slice(2); | ||
| assert(baselineArg && candidateArg && outputArg, "Expected baseline, candidate, and output directory"); | ||
| const output = resolve(outputArg); | ||
| assert(!existsSync(output), "Use a fresh output directory to preserve every comparison"); | ||
| mkdirSync(output, { recursive: true }); | ||
| const baseEnv = { ...process.env, BUN_RUNTIME_TRANSPILER_CACHE_PATH: "0", NO_COLOR: "1" }; | ||
| for (const key of ["NODE_OPTIONS", "BUN_OPTIONS", "BUN_DIAG_ASYNC_GC_CLEAR", "BUN_GC_TIMER_DISABLE", | ||
| "BUN_GC_TIMER_INTERVAL", "BUN_IDLE_GC_SECONDS", "BUN_GARBAGE_COLLECTOR_LEVEL", "BUN_TEST_DRAIN_EVENT_LOOP"]) | ||
| delete baseEnv[key]; | ||
| const binaries = { baseline: resolve(baselineArg), candidate: resolve(candidateArg) }; | ||
| const identities = Object.fromEntries(Object.entries(binaries).map(([label, exe]) => { | ||
| const child = spawnSync(exe, ["-p", "JSON.stringify({revision:Bun.revision,webkit:process.versions.webkit,arch:process.arch})"], | ||
| { env: baseEnv, encoding: "utf8" }); | ||
| assert.equal(child.status, 0, child.stderr); | ||
| return [label, { ...JSON.parse(child.stdout), sha256: createHash("sha256").update(readFileSync(exe)).digest("hex") }]; | ||
| })); | ||
| assert.notEqual(identities.baseline.sha256, identities.candidate.sha256, "Select two distinct executables"); | ||
| assert.equal(identities.baseline.arch, arch()); | ||
| assert.equal(identities.candidate.arch, arch()); | ||
| const result = { platform: platform(), arch: arch(), cpu: cpus()[0].model, identities, trials: [], summaries: {} }; | ||
| const save = () => writeFileSync(join(output, "results.json"), JSON.stringify(result, null, 2) + "\n"); | ||
| save(); | ||
| const median = values => { | ||
| const sorted = [...values].sort((a, b) => a - b); | ||
| return (sorted[4] + sorted[5]) / 2; | ||
| }; | ||
| for (const name of ["gc-loop", "idle-worker"]) { | ||
| for (let round = 1; round <= 10; round++) { | ||
| for (const label of round % 2 ? ["baseline", "candidate"] : ["candidate", "baseline"]) { | ||
| const dir = join(output, `${name}-${round}-${label}`); | ||
| mkdirSync(join(dir, "home"), { recursive: true, mode: 0o700 }); | ||
| mkdirSync(join(dir, "tmp"), { mode: 0o700 }); | ||
| const env = { ...baseEnv, HOME: join(dir, "home"), TMPDIR: join(dir, "tmp"), | ||
| BUN_GC_TIMER_DISABLE: name === "gc-loop" ? "1" : "0", BUN_GC_TIMER_INTERVAL: "1", | ||
| BUN_IDLE_GC_SECONDS: "1,2", BUN_GC_RUNS_UNTIL_SKIP_RELEASE_ACCESS: "0" }; | ||
| if (name === "idle-worker") env.BUN_JSC_numberOfGCMarkers = "2"; | ||
| const child = spawnSync(binaries[label], [script, "--case", name], | ||
| { env, encoding: "utf8", timeout: 120000, maxBuffer: 1024 * 1024 }); | ||
| writeFileSync(join(dir, "stdout.log"), child.stdout ?? ""); | ||
| writeFileSync(join(dir, "stderr.log"), child.stderr ?? ""); | ||
| assert.equal(child.status, 0, child.error?.message ?? child.stderr); | ||
| const measurement = JSON.parse(child.stdout); | ||
| assert(Number.isFinite(measurement.elapsedMs) && measurement.elapsedMs > 0); | ||
| assert(Number.isFinite(measurement.cpuMs) && measurement.cpuMs > 0); | ||
| result.trials.push({ name, round, label, ...measurement }); | ||
| save(); | ||
| console.log(JSON.stringify(result.trials.at(-1))); | ||
| } | ||
| } | ||
| const rows = label => result.trials.filter(t => t.name === name && t.label === label); | ||
| const summary = {}; | ||
| for (const metric of ["elapsedMs", "cpuMs"]) { | ||
| const baseline = rows("baseline"), candidate = rows("candidate"); | ||
| summary[metric] = { baselineMedian: median(baseline.map(t => t[metric])), | ||
| candidateMedian: median(candidate.map(t => t[metric])), | ||
| pairedMedianRatio: median(candidate.map((t, i) => t[metric] / baseline[i][metric])) }; | ||
| } | ||
| // Idle wall time includes intentional parking; CPU time exposes the collection cost. | ||
| summary.primaryMetric = name === "idle-worker" ? "cpuMs" : "elapsedMs"; | ||
| summary.passed = summary[summary.primaryMetric].pairedMedianRatio <= 1.02; | ||
| result.summaries[name] = summary; | ||
| save(); | ||
| } | ||
| console.log(JSON.stringify(result.summaries, null, 2)); | ||
| assert(Object.values(result.summaries).every(summary => summary.passed), "Async GC cost exceeded 1.02"); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For the Windows ARM64 worker-finalization failure, this adds unconditional stack clearing without identifying the retaining stack word or ruling out real references. The commit explicitly says the historical retaining slot remains unknown and collection-time marking provenance is still follow-up work. Root
AGENTS.mdrequires both a retained-object heap snapshot excluding other retainers and a debugger session identifying the stack word and owning frame before accepting a conservative-stack explanation. The passing stack-clear toggle trials establish that this changes the outcome, not why the object survives. Capture those artifacts and fix the identified retention mechanism before extending this layout-dependent mitigation to both asynchronous entry points.AGENTS.md reference: AGENTS.md:L209-L209
Useful? React with 👍 / 👎.