FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: add MacOS support by not-matthias · Pull Request #81 · CodSpeedHQ/codspeed-node · GitHub

/ codspeed-node Public

feat: add MacOS support - #81

Merged
not-matthias merged 3 commits into
mainfrom
cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos
Jun 12, 2026
ConversationCommits (3)ChecksFiles changed
Merged

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

Conversation

not-matthias commented Jun 2, 2026

Copy link
Copy Markdown
Member

No description provided.

codspeed Bot commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will degrade performance by 53.38%

⚠️ 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

⚡ 3 improved benchmarks
❌ 16 regressed benchmarks
✅ 206 untouched benchmarks
🆕 1 new benchmark

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory fibo 15 5 B 2,960 B -99.83%
❌ Memory long body 960 B 67,448 B -98.58%
❌ WallTime iterative fibo 15 312 ns 840 ns -62.86%
❌ WallTime iterative fibo 20 360 ns 876 ns -58.9%
❌ Simulation test_recursive_fibo_10 139 µs 282.1 µs -50.71%
❌ WallTime short body 1.7 µs 2.6 µs -32.71%
❌ WallTime short body 1.8 µs 2.5 µs -30%
❌ WallTime short body 1.7 µs 2.4 µs -29.21%
❌ WallTime short body 1.7 µs 2.4 µs -28.28%
❌ WallTime short body 1.8 µs 2.4 µs -28.08%
❌ WallTime short body 1.8 µs 2.4 µs -25.49%
❌ Simulation switch 2 8.3 µs 11.1 µs -25.08%
❌ Memory short body 408 B 536 B -23.88%
❌ Memory short body 408 B 536 B -23.88%
❌ WallTime fibo 10 2.1 µs 2.7 µs -20.63%
❌ Simulation wait 1ms 20 µs 22.8 µs -11.97%
⚡ Memory wait 500ms 64.1 KB 23 KB ×2.8
⚡ Memory wait 1ms 16 B 7 B ×2.3
⚡ WallTime switch 1 324 ns 288 ns +12.5%
🆕 WallTime fibo darwin N/A 8.7 ms N/A

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 (3411926) with main (4dae798)

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
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 marked this pull request as ready for review June 3, 2026 15:22

greptile-apps Bot commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Greptile Summary

This PR adds macOS support to the CodSpeed Node.js SDK by extracting the repeated inline __codspeed_root_frame__ function pattern into shared wrapWithRootFrame/wrapWithRootFrameSync utilities in @codspeed/core, adding walltime benchmark marker support to both the tinybench and vitest plugins, and wiring up a dedicated codspeed-walltime-macos CI job with a macOS-only benchmark guard.

  • packages/core/src/rootFrame.ts introduces wrapWithRootFrame and wrapWithRootFrameSync, which are consumed by benchmark.js-plugin, tinybench-plugin, and vitest-plugin to replace four previously duplicated inline function declarations.
  • packages/tinybench-plugin/src/shared.ts and packages/vitest-plugin/src/walltime/index.ts add InstrumentHooks.addMarker calls for BENCHMARK_START/BENCHMARK_END so the runner can locate benchmark boundaries in walltime profiles; note the two plugins use opposite marker-vs-stopBenchmark ordering (vitest emits markers before stop; tinybench emits them after).
  • .github/workflows/codspeed.yml adds codspeed-walltime-macos (runner: macos-latest, runner-version: latest, no staging URL) and the old walltime-macos-test job is removed from ci.yml.

Confidence Score: 5/5

Safe to merge — the refactoring is mechanical and correct, the macOS job configuration is clean, and all previously flagged workflow concerns have been resolved.

The changes are a clean extraction of a repeated inline-function pattern into shared utilities, with correct invocation at every call site. The new macOS CI job uses runner-version: latest and routes to the production upload endpoint. The walltime marker ordering difference between the two plugins was already discussed at length in prior review threads.

The sendBenchmarkMarkers path in packages/tinybench-plugin/src/shared.ts is not exercised by the integration tests (all tests run under instrumentation mode, causing early return). No file requires blocking attention.

Important Files Changed

Filename Overview
packages/core/src/rootFrame.ts New utility — correctly wraps any sync or async function inside a named __codspeed_root_frame__ frame and exports both variants from the package index.
packages/tinybench-plugin/src/shared.ts Adds walltime addMarker calls inside wrapWithInstrumentHooks(Async); markers are emitted after stopBenchmark(), which is the inverse of the vitest-plugin ordering.
packages/vitest-plugin/src/walltime/index.ts Correctly emits both markers before stopBenchmark(), consistent with the inline comment about FIFO ordering; also restores task.fn on every exit path.
packages/tinybench-plugin/tests/index.integ.test.ts Adds currentTimestamp, addMarker, and writeWalltimeResults mocks; however all tests run with CODSPEED_RUNNER_MODE=instrumentation, so sendBenchmarkMarkers always returns early and the new marker code path is not exercised.
.github/workflows/codspeed.yml Adds codspeed-walltime-macos job with runner-version: latest, no staging URL override, and no diagnostic steps — previously flagged issues have all been resolved.

Reviews (22): Last reviewed commit: "feat: add macOS-only vitest benchmark an..." | Re-trigger Greptile

greptile-apps Bot reviewed Jun 3, 2026
View reviewed changes
Comment thread packages/vitest-plugin/src/walltime/index.ts Outdated
Comment thread packages/vitest-plugin/benches/macos.bench.ts Outdated
Comment thread .github/workflows/codspeed.yml Outdated
Comment thread .github/workflows/codspeed.yml Outdated
not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch 5 times, most recently from 762c0d8 to 93cf800 Compare June 4, 2026 16:41
GuillaumeLagrange requested changes Jun 9, 2026
View reviewed changes

GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

olgtm

Comment thread packages/vitest-plugin/src/walltime/index.ts Outdated
Comment thread .github/workflows/codspeed.yml Outdated
not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch from 93cf800 to e08c11f Compare June 10, 2026 10:19
greptile-apps Bot reviewed Jun 10, 2026
View reviewed changes
Comment thread .github/workflows/codspeed.yml Outdated

avalanche-staging Bot commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Congrats! CodSpeed is installed 🎉

🆕 1 new benchmark was detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

⚠️ 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.

Detected benchmark

not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch from 792ee5f to fea06fe Compare June 10, 2026 16:36
not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch from fea06fe to 48a2b46 Compare June 10, 2026 16:42
GuillaumeLagrange approved these changes Jun 11, 2026
View reviewed changes

GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

GuillaumeLagrange commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Although CI is failing.
Some of it is due to github instabilities of yesterday, but one of the job failed typechecking, I think because of core library. I'll elt you check

not-matthias force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch 3 times, most recently from 7a06ec3 to c184ad6 Compare June 11, 2026 17:50
feat(vitest,tinybench): emit benchmark markers inside the sample window
b7f1be7 
Emit benchmark start/end markers for the tinybench plugin and the vitest
walltime runner, wrapping the measured function in a root frame.

The runner consumes the instrument-hooks FIFO stream in order and expects
SampleStart > BenchmarkStart > BenchmarkEnd > SampleEnd nesting per
benchmark, so the markers must land inside the sample window:

- vitest: emit the marker pair before stopBenchmark(), and move
  stopBenchmark() plus the markers into a finally block so a throwing
  benchmark cannot leave the profiler started-but-never-stopped
- tinybench: emit markers per task between start/stop instead of a single
  run-level pair; wrap the body in try/finally to keep start/stop balanced
  when a benchmark throws
- benchmark.js: bind wrapWithRootFrame/wrapWithRootFrameSync to the real
  implementations in the integ test's core mock

Add a regression test asserting both markers land between startBenchmark
and stopBenchmark in walltime mode.
feat: add macOS-only vitest benchmark and CI job
3411926 
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 force-pushed the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch from c184ad6 to 3411926 Compare June 12, 2026 07:13
not-matthias merged commit 3411926 into main Jun 12, 2026
28 of 29 checks passed
not-matthias deleted the cod-2720-bump-instrument-hooks-in-codspeed-node-to-support-macos branch June 12, 2026 07:42
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

greptile-apps[bot] greptile-apps[bot] left review comments
GuillaumeLagrange GuillaumeLagrange approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL