Skip to content

Commit cc1f7a2

Browse files
not-matthiasclaude
andcommitted
fix(tinybench): emit a single walltime marker pair per task
Per-iteration markers made marker volume proportional to the iteration count (millions for fast benches): every addMarker call is a synchronous FIFO round-trip to the runner, which overwhelmed it and timed out the walltime CI job. Buffer round timestamps in JS instead and flush one BENCHMARK_START/BENCHMARK_END pair bracketing the measured rounds after the run. Also exclude warmup from markers (the wrapper previously emitted markers during warmup rounds) and fix the sync path to use warmupSync(): the un-awaited async warmup() never actually ran before runSync(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9925d1a commit cc1f7a2

1 file changed

Lines changed: 65 additions & 17 deletions

File tree

packages/tinybench-plugin/src/walltime.ts

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,39 @@ class WalltimeBenchRunner extends BaseBenchRunner {
3030

3131
protected async runTaskAsync(task: Task, uri: string): Promise<void> {
3232
// Override the function under test to add a static frame
33-
this.wrapTaskFunction(task, true);
33+
const markers = this.wrapTaskFunction(task, true);
3434

3535
// run the warmup of the task right before its actual run
3636
if (this.bench.opts.warmup) {
3737
await task.warmup();
38+
// warmup rounds must not be covered by benchmark markers
39+
markers.reset();
3840
}
3941

4042
await mongoMeasurement.start(uri);
41-
await this.wrapWithInstrumentHooksAsync(() => task.run(), uri);
43+
await this.wrapWithInstrumentHooksAsync(async () => {
44+
await task.run();
45+
markers.flush();
46+
}, uri);
4247
await mongoMeasurement.stop(uri);
4348

4449
this.registerCodspeedBenchmarkFromTask(task);
4550
}
4651

4752
protected runTaskSync(task: Task, uri: string): void {
4853
// Override the function under test to add a static frame
49-
this.wrapTaskFunction(task, false);
54+
const markers = this.wrapTaskFunction(task, false);
5055

5156
if (this.bench.opts.warmup) {
52-
task.warmup();
57+
task.warmupSync();
58+
// warmup rounds must not be covered by benchmark markers
59+
markers.reset();
5360
}
5461

55-
this.wrapWithInstrumentHooks(() => task.runSync(), uri);
62+
this.wrapWithInstrumentHooks(() => {
63+
task.runSync();
64+
markers.flush();
65+
}, uri);
5666

5767
this.registerCodspeedBenchmarkFromTask(task);
5868
}
@@ -65,24 +75,16 @@ class WalltimeBenchRunner extends BaseBenchRunner {
6575
return this.finalizeWalltimeRun(false);
6676
}
6777

68-
private wrapTaskFunction(task: Task, isAsync: boolean): void {
78+
private wrapTaskFunction(task: Task, isAsync: boolean): TaskMarkerTracker {
6979
const { fn } = task as unknown as { fn: Fn };
70-
const pid = process.pid;
71-
72-
// Emit per-round markers so the walltime instrument can bracket the actual
73-
// benchmark body (excluding the harness) when attributing perf samples.
74-
const finishRound = (roundStart: bigint): void => {
75-
const roundEnd = InstrumentHooks.currentTimestamp();
76-
InstrumentHooks.addMarker(pid, MARKER_TYPE_BENCHMARK_START, roundStart);
77-
InstrumentHooks.addMarker(pid, MARKER_TYPE_BENCHMARK_END, roundEnd);
78-
};
80+
const markers = new TaskMarkerTracker();
7981

8082
if (isAsync) {
8183
// eslint-disable-next-line no-inner-declarations
8284
async function __codspeed_root_frame__() {
8385
const roundStart = InstrumentHooks.currentTimestamp();
8486
await fn();
85-
finishRound(roundStart);
87+
markers.recordRound(roundStart);
8688
}
8789
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8890
(task as any).fn = __codspeed_root_frame__;
@@ -91,11 +93,13 @@ class WalltimeBenchRunner extends BaseBenchRunner {
9193
function __codspeed_root_frame__() {
9294
const roundStart = InstrumentHooks.currentTimestamp();
9395
fn();
94-
finishRound(roundStart);
96+
markers.recordRound(roundStart);
9597
}
9698
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9799
(task as any).fn = __codspeed_root_frame__;
98100
}
101+
102+
return markers;
99103
}
100104

101105
private registerCodspeedBenchmarkFromTask(task: Task): void {
@@ -145,6 +149,50 @@ class WalltimeBenchRunner extends BaseBenchRunner {
145149
}
146150
}
147151

152+
/**
153+
* Buffers round timestamps in JS so a single BENCHMARK_START/BENCHMARK_END
154+
* marker pair brackets all measured rounds of a task, emitted once after the
155+
* run. Every `addMarker` call is a synchronous FIFO round-trip to the runner,
156+
* so emitting markers per round made marker volume proportional to the
157+
* iteration count and overwhelmed the runner (CI timeouts) while inflating
158+
* the measured samples.
159+
*/
160+
class TaskMarkerTracker {
161+
private firstRoundStart: bigint | null = null;
162+
private lastRoundEnd: bigint | null = null;
163+
164+
recordRound(roundStart: bigint): void {
165+
if (this.firstRoundStart === null) {
166+
this.firstRoundStart = roundStart;
167+
}
168+
this.lastRoundEnd = InstrumentHooks.currentTimestamp();
169+
}
170+
171+
/** Discard the rounds recorded so far (e.g. warmup rounds). */
172+
reset(): void {
173+
this.firstRoundStart = null;
174+
this.lastRoundEnd = null;
175+
}
176+
177+
/** Emit a single marker pair bracketing the recorded rounds. */
178+
flush(): void {
179+
if (this.firstRoundStart === null || this.lastRoundEnd === null) {
180+
return;
181+
}
182+
InstrumentHooks.addMarker(
183+
process.pid,
184+
MARKER_TYPE_BENCHMARK_START,
185+
this.firstRoundStart,
186+
);
187+
InstrumentHooks.addMarker(
188+
process.pid,
189+
MARKER_TYPE_BENCHMARK_END,
190+
this.lastRoundEnd,
191+
);
192+
this.reset();
193+
}
194+
}
195+
148196
const TINYBENCH_WARMUP_DEFAULT = 16;
149197

150198
function convertTinybenchResultToBenchmarkStats(

0 commit comments

Comments
 (0)