Skip to content

feat: add MacOS support#81

Open
not-matthias wants to merge 9 commits into
mainfrom
cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos
Open

feat: add MacOS support#81
not-matthias wants to merge 9 commits into
mainfrom
cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos

Conversation

@not-matthias
Copy link
Copy Markdown
Member

No description provided.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 2, 2026

Merging this PR will degrade performance by 91.73%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 5 improved benchmarks
❌ 15 regressed benchmarks
✅ 169 untouched benchmarks
🆕 1 new benchmark
⏩ 36 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory wait 1ms 16 B 14,792 B -99.89%
WallTime short body 1.7 µs 346.8 µs -99.51%
WallTime short body 1.8 µs 351.6 µs -99.5%
WallTime short body 1.7 µs 339.4 µs -99.5%
WallTime short body 1.7 µs 342.4 µs -99.5%
WallTime short body 1.8 µs 341.2 µs -99.48%
WallTime short body 1.8 µs 334.7 µs -99.45%
WallTime fibo 10 2.1 µs 369.4 µs -99.43%
WallTime fibo 15 21.5 µs 351.6 µs -93.87%
WallTime end 35.5 µs 390.5 µs -90.92%
WallTime long body 214.3 µs 500.1 µs -57.14%
WallTime long body 211.7 µs 492.5 µs -57.01%
WallTime long body 217.2 µs 493.2 µs -55.96%
WallTime long body 217.3 µs 492 µs -55.84%
WallTime wait 1ms 1 ms 1.4 ms -30.02%
Memory wait 500ms 64.1 KB 15 KB ×4.3
Simulation short body 121.2 µs 73.3 µs +65.44%
Simulation one 589.9 µs 401.9 µs +46.77%
Simulation wait 500ms 12.2 ms 10 ms +22.65%
Simulation long body 382.1 µs 333.4 µs +14.61%
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos (d7cdf16) with main (4dae798)

Open in CodSpeed

Footnotes

  1. 36 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@not-matthias not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch 2 times, most recently from f0b1349 to 3982c34 Compare June 2, 2026 10:36
Add benches/macos.bench.ts guarded with describe.skipIf(!darwin) so it
only runs on macOS, and wire the walltime-macos-test CI job to execute it
via a direct `node vitest.mjs bench --run macos` invocation.
@not-matthias not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch from 32c3b31 to 1d75a61 Compare June 3, 2026 13:23
@not-matthias not-matthias marked this pull request as ready for review June 3, 2026 15:22
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 3, 2026

Greptile Summary

This PR adds macOS walltime support by emitting paired BENCHMARK_START/BENCHMARK_END markers from inside __codspeed_root_frame__ so the profiler can bracket the actual benchmark body excluding harness overhead. It also introduces a dedicated codspeed-walltime-macos CI job on macos-26-intel and bumps the instrument-hooks submodule to expose the new native addMarker/currentTimestamp API.

  • Marker emission: Both the tinybench and vitest plugins now call finishRound at the end of each iteration to emit timestamped start/end markers; the vitest plugin adds promise-aware detection to handle async benchmarks via .then.
  • CI restructuring: The old walltime-macos-test job in ci.yml is removed and replaced by codspeed-walltime-macos in codspeed.yml, targeting a named macos-26-intel runner.
  • New benchmark: macos.bench.ts adds a describe.skipIf(!isMacOS) guarded Fibonacci benchmark exercised exclusively by the new macOS job.

Confidence Score: 5/5

Safe to merge — the functional changes are correct and the new marker-emission logic handles both sync and async benchmarks without breaking existing behavior.

The core logic change (emitting paired BENCHMARK_START/END markers inside codspeed_root_frame) is mechanically sound: timestamps are captured at the right points, the promise-aware path in the vitest plugin correctly passes the fulfillment value through, and the native API surface is properly wired from the submodule through @codspeed/core to both plugins. All previously identified concerns were already flagged in prior review threads and do not introduce runtime regressions.

.github/workflows/codspeed.yml warrants a follow-up pass once the sip-resign-exec-redirect runner branch is stabilised, to remove the diagnostic step and pin runner-version to a fixed ref.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Removes the walltime-macos-test CI job; macOS walltime coverage is now owned by codspeed-walltime-macos in codspeed.yml.
.github/workflows/codspeed.yml Adds codspeed-walltime-macos job on macos-26-intel; contains a diagnostic resign+exec step and a mutable runner-version branch reference (both flagged in prior threads).
packages/core/src/native_core/instruments/hooks Bumps the instrument-hooks submodule to add addMarker/currentTimestamp native API and MARKER_TYPE_BENCHMARK_* constants.
packages/tinybench-plugin/src/walltime.ts Adds per-round finishRound helper emitting BENCHMARK_START/BENCHMARK_END markers in both async and sync paths.
packages/vitest-plugin/benches/macos.bench.ts New macOS-only vitest benchmark guarded by describe.skipIf(!isMacOS); stale CI job reference in the comment was flagged in a prior thread.
packages/vitest-plugin/src/walltime/index.ts Replaces the simple return fn() wrapper with a promise-aware version that emits per-round start/end markers for both sync and async benchmarks.

Sequence Diagram

sequenceDiagram
    participant TB as Tinybench (originalRun)
    participant RF as __codspeed_root_frame__
    participant FN as User benchmark fn
    participant IH as InstrumentHooks (native)

    TB->>IH: startBenchmark()
    loop Per iteration
        TB->>RF: call fn()
        RF->>IH: currentTimestamp() - roundStart
        RF->>FN: fn() / await fn()
        FN-->>RF: result (sync or Promise)
        alt Promise result
            RF->>RF: "result.then(value => finishRound + return value)"
        else Sync result
            RF->>RF: finishRound(roundStart)
        end
        RF->>IH: addMarker(pid, BENCHMARK_START, roundStart)
        RF->>IH: addMarker(pid, BENCHMARK_END, roundEnd)
        RF-->>TB: return result
    end
    TB->>IH: stopBenchmark()
    TB->>IH: setExecutedBenchmark(pid, uri)
Loading

Reviews (3): Last reviewed commit: "chore: try with intel mac" | Re-trigger Greptile

Comment on lines +93 to +97
if (
result !== null &&
typeof result === "object" &&
typeof (result as PromiseLike<unknown>).then === "function"
) {
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 multi-clause thenable check should be extracted into a named const per the project's rule that non-obvious or multi-clause boolean conditions must be named with an is*/has* prefix.

Suggested change
if (
result !== null &&
typeof result === "object" &&
typeof (result as PromiseLike<unknown>).then === "function"
) {
const isThenable =
result !== null &&
typeof result === "object" &&
typeof (result as PromiseLike<unknown>).then === "function";
if (isThenable) {

Rule Used: Extract any non-obvious or multi-clause boolean co... (source)

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!

Comment on lines +10 to +11
// macOS-only benchmark: skipped on every other platform, so it only runs on
// the `walltime-macos-test` CI job (see .github/workflows/ci.yml).
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 comment references the removed walltime-macos-test job in ci.yml. That job was deleted by this PR; the macOS benchmark is now run by codspeed-walltime-macos in .github/workflows/codspeed.yml.

Suggested change
// macOS-only benchmark: skipped on every other platform, so it only runs on
// the `walltime-macos-test` CI job (see .github/workflows/ci.yml).
// macOS-only benchmark: skipped on every other platform, so it only runs on
// the `codspeed-walltime-macos` CI job (see .github/workflows/codspeed.yml).

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!

Comment on lines +111 to +118
- name: Diagnose resign+exec on this runner
run: |
sw_vers; uname -m; sysctl -n hw.memsize hw.ncpu
cp /bin/sh /tmp/sh_copy
/usr/bin/codesign -s - -f /tmp/sh_copy
lipo -info /tmp/sh_copy
/tmp/sh_copy -c 'echo RESIGNED_ARM64E_OK; uname -m'
echo "exit=$?"
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 Diagnostic step left in production workflow

The "Diagnose resign+exec on this runner" step copies /bin/sh, codesigns it, and re-executes it — this is clearly debugging scaffolding for the SIP/resign investigation. Leaving it in the permanent workflow adds noise to every future CI run and exposes internal implementation details. It should be removed once the resign-exec behaviour is confirmed working.

working-directory: packages/vitest-plugin
run: pnpm turbo run bench --env-mode=loose --filter=@codspeed/vitest-plugin
mode: walltime
runner-version: branch:sip-resign-exec-redirect
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 Mutable branch reference for runner-version

runner-version: branch:sip-resign-exec-redirect points to a live branch that can be force-pushed or rebased at any time. If the branch tip changes between two benchmark runs, the profiling behaviour changes silently, making measurement results non-comparable across commits. Consider pinning to a specific commit SHA or tag once the feature branch is stable, e.g. runner-version: sha:<commit>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant