Skip to content

Commit bd4cee5

Browse files
not-matthiasclaude
andcommitted
fix(tinybench): emit a single walltime marker pair per bench run
Move marker emission from per-iteration (inside the task fn) to a single BENCHMARK_START/BENCHMARK_END pair bracketing the whole bench run in setupBenchMethods. Each addMarker call is a synchronous FIFO round-trip to the runner, so per-iteration markers scaled with iteration count (millions for fast benches), overwhelmed the runner, and timed out the walltime CI job — while also inflating the measured samples. Emission is gated to walltime mode so the analysis runner sharing setupBenchMethods is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cc1f7a2 commit bd4cee5

3 files changed

Lines changed: 37 additions & 70 deletions

File tree

packages/tinybench-plugin/src/shared.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import { InstrumentHooks, setupCore, teardownCore } from "@codspeed/core";
1+
import {
2+
getInstrumentMode,
3+
InstrumentHooks,
4+
MARKER_TYPE_BENCHMARK_END,
5+
MARKER_TYPE_BENCHMARK_START,
6+
setupCore,
7+
teardownCore,
8+
} from "@codspeed/core";
29
import { Bench, Fn, Task } from "tinybench";
310
import { getTaskUri } from "./uri";
411

@@ -63,25 +70,47 @@ export abstract class BaseBenchRunner {
6370
protected abstract finalizeAsyncRun(): Task[];
6471
protected abstract finalizeSyncRun(): Task[];
6572

73+
/**
74+
* Emit a single BENCHMARK_START/BENCHMARK_END marker pair bracketing the
75+
* whole bench run. Each `addMarker` call is a synchronous FIFO round-trip
76+
* to the runner, so marker volume must stay bounded: emitting markers per
77+
* iteration previously overwhelmed the runner and timed out CI.
78+
*/
79+
private sendRunMarkers(runStart: bigint, runEnd: bigint): void {
80+
if (getInstrumentMode() !== "walltime") {
81+
return;
82+
}
83+
InstrumentHooks.addMarker(
84+
process.pid,
85+
MARKER_TYPE_BENCHMARK_START,
86+
runStart,
87+
);
88+
InstrumentHooks.addMarker(process.pid, MARKER_TYPE_BENCHMARK_END, runEnd);
89+
}
90+
6691
public setupBenchMethods(): void {
6792
this.bench.run = async () => {
6893
this.setupBenchRun();
6994

95+
const runStart = InstrumentHooks.currentTimestamp();
7096
for (const task of this.bench.tasks) {
7197
const uri = this.getTaskUri(task);
7298
await this.runTaskAsync(task, uri);
7399
}
100+
this.sendRunMarkers(runStart, InstrumentHooks.currentTimestamp());
74101

75102
return this.finalizeAsyncRun();
76103
};
77104

78105
this.bench.runSync = () => {
79106
this.setupBenchRun();
80107

108+
const runStart = InstrumentHooks.currentTimestamp();
81109
for (const task of this.bench.tasks) {
82110
const uri = this.getTaskUri(task);
83111
this.runTaskSync(task, uri);
84112
}
113+
this.sendRunMarkers(runStart, InstrumentHooks.currentTimestamp());
85114

86115
return this.finalizeSyncRun();
87116
};

packages/tinybench-plugin/src/walltime.ts

Lines changed: 5 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
import {
22
calculateQuantiles,
3-
InstrumentHooks,
4-
MARKER_TYPE_BENCHMARK_END,
5-
MARKER_TYPE_BENCHMARK_START,
63
mongoMeasurement,
74
msToNs,
85
msToS,
@@ -30,39 +27,29 @@ class WalltimeBenchRunner extends BaseBenchRunner {
3027

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

3532
// run the warmup of the task right before its actual run
3633
if (this.bench.opts.warmup) {
3734
await task.warmup();
38-
// warmup rounds must not be covered by benchmark markers
39-
markers.reset();
4035
}
4136

4237
await mongoMeasurement.start(uri);
43-
await this.wrapWithInstrumentHooksAsync(async () => {
44-
await task.run();
45-
markers.flush();
46-
}, uri);
38+
await this.wrapWithInstrumentHooksAsync(() => task.run(), uri);
4739
await mongoMeasurement.stop(uri);
4840

4941
this.registerCodspeedBenchmarkFromTask(task);
5042
}
5143

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

5648
if (this.bench.opts.warmup) {
5749
task.warmupSync();
58-
// warmup rounds must not be covered by benchmark markers
59-
markers.reset();
6050
}
6151

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

6754
this.registerCodspeedBenchmarkFromTask(task);
6855
}
@@ -75,31 +62,24 @@ class WalltimeBenchRunner extends BaseBenchRunner {
7562
return this.finalizeWalltimeRun(false);
7663
}
7764

78-
private wrapTaskFunction(task: Task, isAsync: boolean): TaskMarkerTracker {
65+
private wrapTaskFunction(task: Task, isAsync: boolean): void {
7966
const { fn } = task as unknown as { fn: Fn };
80-
const markers = new TaskMarkerTracker();
8167

8268
if (isAsync) {
8369
// eslint-disable-next-line no-inner-declarations
8470
async function __codspeed_root_frame__() {
85-
const roundStart = InstrumentHooks.currentTimestamp();
8671
await fn();
87-
markers.recordRound(roundStart);
8872
}
8973
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9074
(task as any).fn = __codspeed_root_frame__;
9175
} else {
9276
// eslint-disable-next-line no-inner-declarations
9377
function __codspeed_root_frame__() {
94-
const roundStart = InstrumentHooks.currentTimestamp();
9578
fn();
96-
markers.recordRound(roundStart);
9779
}
9880
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9981
(task as any).fn = __codspeed_root_frame__;
10082
}
101-
102-
return markers;
10383
}
10484

10585
private registerCodspeedBenchmarkFromTask(task: Task): void {
@@ -149,50 +129,6 @@ class WalltimeBenchRunner extends BaseBenchRunner {
149129
}
150130
}
151131

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-
196132
const TINYBENCH_WARMUP_DEFAULT = 16;
197133

198134
function convertTinybenchResultToBenchmarkStats(

packages/tinybench-plugin/tests/index.integ.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const mockCore = vi.hoisted(() => {
1616
startBenchmark: vi.fn(),
1717
stopBenchmark: vi.fn(),
1818
setExecutedBenchmark: vi.fn(),
19+
currentTimestamp: vi.fn().mockReturnValue(0n),
20+
addMarker: vi.fn(),
1921
},
2022
optimizeFunction: vi
2123
.fn()

0 commit comments

Comments
 (0)