Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,10 @@ Based on the codebase analysis, to add stats access features:
## Repository Management Memories

- Use pnpm instead of npm
- To run tests in a package use moon <package-name>:test
- The monorepo uses [Turborepo](https://turborepo.com). Run a task for a single
package with a filter, using the package's full name (e.g. `@codspeed/tinybench-plugin`):
- Test: `pnpm turbo run test --filter=<package-name>`
- Build: `pnpm turbo run build --filter=<package-name>`
- Typecheck: `pnpm turbo run typecheck --filter=<package-name>`
- Lint: `pnpm turbo run lint --filter=<package-name>`
- Run a task across all packages by omitting `--filter` (e.g. `pnpm turbo run build`).
9 changes: 7 additions & 2 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ export const setupCore = () => {
}

native_core.InstrumentHooks.setIntegration("codspeed-node", __VERSION__);
linuxPerf.start();
// In walltime, we use node's native profiling options rather than our own, cf `getV8Flags`
if (getCodspeedRunnerMode() !== "walltime") {
linuxPerf.start();
}
checkV8Flags();

// Collect Node.js runtime environment to detect changes that could
Expand All @@ -68,7 +71,9 @@ export const setupCore = () => {
};

export const teardownCore = () => {
linuxPerf.stop();
if (getCodspeedRunnerMode() !== "walltime") {
linuxPerf.stop();
}
};

export type {
Expand Down
58 changes: 41 additions & 17 deletions packages/core/src/introspection.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,56 @@
import { writeFileSync } from "fs";
import path from "path";

import { getInstrumentMode } from ".";

const CUSTOM_INTROSPECTION_EXIT_CODE = 0;

const V8_LOG_FILENAME_PATTERN = "codspeed-v8-%p.log";

export const getV8Flags = () => {
const nodeVersionMajor = parseInt(process.version.slice(1).split(".")[0]);
const instrumentMode = getInstrumentMode();

const flags = ["--interpreted-frames-native-stack", "--allow-natives-syntax"];

if (instrumentMode === "analysis") {
flags.push(
...[
"--hash-seed=1",
"--random-seed=1",
"--no-opt",
"--predictable",
"--predictable-gc-schedule",
"--expose-gc",
"--no-concurrent-sweeping",
"--max-old-space-size=4096",
],
);
if (nodeVersionMajor < 18) {
flags.push("--no-randomize-hashes");
switch (instrumentMode) {
case "analysis": {
flags.push(
...[
"--hash-seed=1",
"--random-seed=1",
"--no-opt",
"--predictable",
"--predictable-gc-schedule",
"--expose-gc",
"--no-concurrent-sweeping",
"--max-old-space-size=4096",
],
);
if (nodeVersionMajor < 18) {
flags.push("--no-randomize-hashes");
}
if (nodeVersionMajor < 20) {
flags.push("--no-scavenge-task");
}

break;
}
if (nodeVersionMajor < 20) {
flags.push("--no-scavenge-task");

case "walltime": {
// Emit the V8 jitdump
flags.push("--perf-prof");
const v8LogDir = process.env.CODSPEED_V8_LOG;
if (v8LogDir) {
flags.push(
...[
"--log-code",
"--no-log-source-code",
"--no-logfile-per-isolate",
`--logfile=${path.join(v8LogDir, V8_LOG_FILENAME_PATTERN)}`,
],
);
}
}
}
Comment on lines +44 to 55

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The walltime case is missing a break statement, unlike the analysis case above it. While harmless here since there is no subsequent case, the inconsistency can confuse future readers and will trip no-fallthrough linters.

Suggested change
if (v8LogDir) {
flags.push(
...[
"--log-code",
"--no-log-source-code",
"--no-logfile-per-isolate",
`--logfile=${path.join(v8LogDir, V8_LOG_FILENAME_PATTERN)}`,
],
);
}
}
}
if (v8LogDir) {
flags.push(
...[
"--log-code",
"--no-log-source-code",
"--no-logfile-per-isolate",
`--logfile=${path.join(v8LogDir, V8_LOG_FILENAME_PATTERN)}`,
],
);
}
break;
}
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


Expand Down
28 changes: 16 additions & 12 deletions packages/tinybench-plugin/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,38 @@ export abstract class BaseBenchRunner {
}

protected wrapWithInstrumentHooks<T>(fn: () => T, uri: string): T {
InstrumentHooks.startBenchmark();
const runStart = InstrumentHooks.currentTimestamp();
const runStart = this.openInstrumentWindow();
try {
return fn();
} finally {
const runEnd = InstrumentHooks.currentTimestamp();
InstrumentHooks.stopBenchmark();
InstrumentHooks.setExecutedBenchmark(process.pid, uri);
this.sendBenchmarkMarkers(runStart, runEnd);
this.closeInstrumentWindow(uri, runStart);
}
}

protected async wrapWithInstrumentHooksAsync(
fn: Fn,
uri: string,
): Promise<unknown> {
InstrumentHooks.startBenchmark();
const runStart = InstrumentHooks.currentTimestamp();
const runStart = this.openInstrumentWindow();
try {
return await fn();
} finally {
const runEnd = InstrumentHooks.currentTimestamp();
InstrumentHooks.stopBenchmark();
InstrumentHooks.setExecutedBenchmark(process.pid, uri);
this.sendBenchmarkMarkers(runStart, runEnd);
this.closeInstrumentWindow(uri, runStart);
}
}

protected openInstrumentWindow(): bigint {
InstrumentHooks.startBenchmark();
return InstrumentHooks.currentTimestamp();
}

protected closeInstrumentWindow(uri: string, runStart: bigint): void {
const runEnd = InstrumentHooks.currentTimestamp();
InstrumentHooks.stopBenchmark();
InstrumentHooks.setExecutedBenchmark(process.pid, uri);
this.sendBenchmarkMarkers(runStart, runEnd);
}

protected abstract getModeName(): string;
protected abstract runTaskAsync(task: Task, uri: string): Promise<void>;
protected abstract runTaskSync(task: Task, uri: string): void;
Expand Down
45 changes: 41 additions & 4 deletions packages/tinybench-plugin/src/walltime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,60 @@ import {
type BenchmarkStats,
type Benchmark as CodspeedBenchmark,
} from "@codspeed/core";
import { Bench, Fn, Task, TaskResult } from "tinybench";
import { Bench, Fn, Hook, Task, TaskResult } from "tinybench";
import { BaseBenchRunner } from "./shared";

export function setupCodspeedWalltimeBench(
bench: Bench,
rootCallingFile: string,
): void {
const runner = new WalltimeBenchRunner(bench, rootCallingFile);
runner.installInstrumentHooks();
runner.setupBenchMethods();
}

class WalltimeBenchRunner extends BaseBenchRunner {
private codspeedBenchmarks: CodspeedBenchmark[] = [];

// Carries the window start timestamp from the setup hook to the teardown
// hook. Tasks run strictly sequentially, so a single field is enough.
private runStart: bigint | null = null;

protected getModeName(): string {
return "walltime mode";
}

/**
* Drive the instrumentation window from the task's setup/teardown hooks so it
* brackets only tinybench's measured loop, excluding warmup and the
* statistics computation (`processRunResult`) that surround it. The user's
* own hooks are preserved and still run in their original order relative to
* the work under test.
*/
public installInstrumentHooks(): void {
// `bench.opts` is typed `Readonly`, but tinybench mutates it at runtime and
// always resolves `setup`/`teardown` to (at least) a no-op default.
const opts = this.bench.opts as { setup: Hook; teardown: Hook };
const userSetup = opts.setup;
const userTeardown = opts.teardown;
Comment on lines +45 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 If a user doesn't pass setup/teardown to their Bench constructor, bench.opts.setup and bench.opts.teardown are typed as optional (setup?: Hook in BenchOptions). The comment asserts tinybench always resolves them to no-ops, but the explicit type cast as { setup: Hook; teardown: Hook } is a sign the compiler can't confirm this. If either value is undefined at the time installInstrumentHooks runs, calling userSetup(task, mode) or userTeardown(task, mode) will throw TypeError: X is not a function for every walltime benchmark run. Since the peerDependency spans all tinybench >=4.0.1 versions, this behaviour should not rely on an undocumented implementation detail. Nullish fallbacks cost nothing and eliminate the risk entirely.

Suggested change
const opts = this.bench.opts as { setup: Hook; teardown: Hook };
const userSetup = opts.setup;
const userTeardown = opts.teardown;
const opts = this.bench.opts as { setup?: Hook; teardown?: Hook };
const noop: Hook = () => {};
const userSetup = opts.setup ?? noop;
const userTeardown = opts.teardown ?? noop;


opts.setup = (task, mode) => {
const setupResult = userSetup(task, mode);
if (mode === "run") {
this.runStart = this.openInstrumentWindow();
}
return setupResult;
};
Comment on lines +49 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 If the user supplied an async setup hook, userSetup(task, mode) returns a Promise<void> that is captured in setupResult but not awaited before openInstrumentWindow() is called. The instrument window therefore opens while the async setup is still in flight, potentially attributing asynchronous prep time (e.g. data seeding, connection reset) to the measurement. Awaiting the user hook first keeps the marker placement accurate.

Suggested change
opts.setup = (task, mode) => {
const setupResult = userSetup(task, mode);
if (mode === "run") {
this.runStart = this.openInstrumentWindow();
}
return setupResult;
};
opts.setup = async (task, mode) => {
await userSetup(task, mode);
if (mode === "run") {
this.runStart = this.openInstrumentWindow();
}
};


opts.teardown = (task, mode) => {
if (mode === "run" && task) {
this.closeInstrumentWindow(this.getTaskUri(task), this.runStart!);
this.runStart = null;
}
return userTeardown(task, mode);
};
}

protected async runTaskAsync(task: Task, uri: string): Promise<void> {
// Override the function under test to add a static frame
this.wrapTaskFunction(task, true);
Expand All @@ -37,21 +73,22 @@ class WalltimeBenchRunner extends BaseBenchRunner {
}

await mongoMeasurement.start(uri);
await this.wrapWithInstrumentHooksAsync(() => task.run(), uri);
await task.run();
await mongoMeasurement.stop(uri);

this.registerCodspeedBenchmarkFromTask(task);
}

protected runTaskSync(task: Task, uri: string): void {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
protected runTaskSync(task: Task, _uri: string): void {
// Override the function under test to add a static frame
this.wrapTaskFunction(task, false);

if (this.bench.opts.warmup) {
task.warmup();
}

this.wrapWithInstrumentHooks(() => task.runSync(), uri);
task.runSync();

this.registerCodspeedBenchmarkFromTask(task);
}
Expand Down
Loading