Support vitest 5#86
Conversation
| } | ||
| } | ||
|
|
||
| function patchRunBenchmarks(): void { |
There was a problem hiding this comment.
By the way, you can do that just in a setup file without overriding the runner. This should also make it support the browser(?)
If there is any feedback, I would love to address them on vitest side; we would also like to use codspeed after Vitest 5 is out in our own repo
There was a problem hiding this comment.
@sheremet-va thanks for the heads up, currently trying out this approach.
The main pain point I am running into is that we are struggling to highjack the benchmark runner when we are in simulation/analysis mode.
When in this mode, what we used to do is highjack the whole run function, to have a very simple sequence of
- warmup
- manually call optimizer
- start valgrind instrumentation
- execute code once
- stop
But I'm struggling a bit to find a good way to replicate the behavior without overriding the whole runner.
I still do not have a proper api request, nor am I sure it's the way to go for the analysis mode, becuase maybe running browser mode under valgrind is not a good idea in the first place, and the main motivation for not highjacking the whole runner is actually to be as compatible as possible with the browser mode.
There was a problem hiding this comment.
Could it be possible to have a way for the plugin to hook/customize the createTinybench function? Maybe by defining some sort of abstract "benchmark backend" interface that we could fullfill, that would default to a tinybench implementation. It would also make it easier to switch away from tinybench on your end at some point.
The way tinybench is instanciated/ran does not allow for much flexibility on our end, and I cannot find a good implementation without overriding the whole runner as we did before for our analysis mode.
I'd love to exchange over this over discord/slack/a call if you want, or we can keep things here on github.
There was a problem hiding this comment.
Feel free to join the discord - https://chat.vitest.dev/
I will give you the ecosystem role, we can discuss it there. I just need your nickname
There was a problem hiding this comment.
I've joined and pinged on via MP, my nickname on the server is the same is github
25ef927 to
e04f0da
Compare
Merging this PR will regress 2 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | switch 1 |
144 ns | 192 ns | -25% |
| ❌ | WallTime | switch 2 |
180 ns | 228 ns | -21.05% |
| ⚡ | Simulation | switch 1 |
313.8 µs | 100.4 µs | ×3.1 |
| ⚡ | Memory | wait 500ms |
64.1 KB | 23 KB | ×2.8 |
| ⚡ | WallTime | short body |
1.8 µs | 1.6 µs | +11.21% |
| 🆕 | WallTime | iterative fibo 15 |
N/A | 900 ns | N/A |
| 🆕 | WallTime | iterative fibo 20 |
N/A | 900 ns | N/A |
| 🆕 | WallTime | recursive fibo 15 |
N/A | 22.4 µs | N/A |
| 🆕 | WallTime | recursive fibo 20 |
N/A | 239.9 µs | N/A |
| 🆕 | Memory | iterative fibo 15 |
N/A | 0 B | N/A |
| 🆕 | Memory | iterative fibo 20 |
N/A | 0 B | N/A |
| 🆕 | Memory | recursive fibo 15 |
N/A | 19 B | N/A |
| 🆕 | Memory | recursive fibo 20 |
N/A | 19 B | N/A |
| 🆕 | Simulation | iterative fibo 15 |
N/A | 17 µs | N/A |
| 🆕 | Simulation | iterative fibo 20 |
N/A | 17.5 µs | N/A |
| 🆕 | Simulation | recursive fibo 15 |
N/A | 153.7 µs | N/A |
| 🆕 | Simulation | recursive fibo 20 |
N/A | 1.4 ms | N/A |
| 👁 | Simulation | wait 500ms |
10.2 ms | 12 ms | -14.67% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing cod-2931-prepare-compatibility-with-vitest-5 (eec7ec7) with main (1999ec9)
Footnotes
-
53 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. ↩
Vitest 5 reworked the benchmark backend: the dedicated `NodeBenchmarkRunner` and the `vitest/runners` / `vitest/suite` entrypoints are gone, benchmarks now run inside `test()` through the unified `TestRunner`, and tinybench moved to v6 (stats moved from `result.benchmark` to `task.result.latency`, `includeSamples` became `retainSamples`). Detect the installed Vitest generation and select the integration seam behind a `VitestBackend` abstraction so the rest of the plugin never inspects the version: - v3/4 keep the custom benchmark runner per instrument mode (analysis/walltime). - v5 installs instrumentation from a setup file that patches the shared `TestRunner.runBenchmarks` static. A setup file (rather than a custom `test.runner`) leaves the runner untouched for non-benchmark tests and also applies to the browser pool.
Now that the plugin's own dev dependency tracks Vitest 5, add a dedicated Vitest 4 example so the legacy (v3/4) benchmark seam keeps explicit coverage alongside the existing with-vitest-v3 example. Mirrors that example, pinning vitest ^4.1.9. Refs COD-2931 Co-Authored-By: Claude <noreply@anthropic.com>
6791750 to
f511dee
Compare
f511dee to
c3a356b
Compare
Vitest 5 exposes a `benchmark.provider` option that hands a plugin the raw benchmark functions and their hooks directly. Wire the V5 backend to register a provider through `test.benchmark.provider` and add the provider itself, which runs analysis and walltime modes off the registrations it receives. Drop the previous V5 seam that patched the static `TestRunner.runBenchmarks` and `Bench.prototype.add` from a setup file, along with the `WeakMap` fn-capture it needed to work around tinybench v6's private `fn` field. The provider gets the fn and options as data, so none of that interception remains. The legacy (Vitest 3/4) runner path is untouched. Point the rollup build and the config-injection test at the new provider entry. Refs COD-2931
Greptile SummaryThis PR adds Vitest 5 support to the CodSpeed Vitest plugin. The main changes are:
Confidence Score: 4/5The new Vitest 5 path can record distinct benchmarks under one CodSpeed URI, and legacy fallback can skip instrumentation.
packages/vitest-plugin/src/v5/provider.ts and packages/vitest-plugin/src/vitestBackend.ts Important Files Changed
|
| let setupHappened = false; | ||
| let teardownHappened = false; | ||
|
|
||
| // TODO: Check if this can be avoided |
There was a problem hiding this comment.
This new TODO has no Linear issue reference, which violates the repository rule for TODO comments. Please change it to the TODO(COD-XXX): ... form or remove it.
| // TODO: Check if this can be avoided | |
| // TODO(COD-XXX): Check if this can be avoided |
Rule Used: Every TODO comment must reference a Linear issue: ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/vitest-plugin/src/globalSetup.ts
Line: 15
Comment:
**Untracked TODO Comment**
This new TODO has no Linear issue reference, which violates the repository rule for TODO comments. Please change it to the `TODO(COD-XXX): ...` form or remove it.
```suggestion
// TODO(COD-XXX): Check if this can be avoided
```
**Rule Used:** Every TODO comment must reference a Linear issue: ... ([source](https://app.greptile.com/codspeed/-/custom-context?memory=65193bc9-f65b-477d-9521-104b5aac5931))
How can I resolve this? If you propose a fix, please make it concise.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!
No description provided.