fix(benchmark): clear BenchmarkResult.samples array to reduce memory usage#6541
Merged
sheremet-va merged 8 commits intovitest-dev:mainfrom Sep 25, 2024
Merged
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
hi-ogawa
changed the title
BenchmarkResult.samples array BenchmarkResult.samples array to reduce memory usage
AriPerkkio
reviewed
Sep 21, 2024
Member
AriPerkkio
left a comment
AriPerkkio
left a comment
There was a problem hiding this comment.
Would be great to add test case for this in https://github.com/vitest-tests/ and include it in Ecosystem CI. We already have some similar cases there. This one could be benchmark-large.
- https://github.com/vitest-tests/reporters-large
- https://github.com/vitest-tests/coverage-large
For example the coverage-large is there to verify that we don't run into similar OOM crash when coverage results of large project are collected. We used to store them in memory - #4603.
The reporters-large is there to verify that Vitest doesn't get completely stuck when there's a lot of reporting to do: #4710.
AriPerkkio
reviewed
Sep 21, 2024
| exclude: defaultExclude, | ||
| includeSource: [], | ||
| reporters: ['default'], | ||
| includeSamples: false, |
Member
There was a problem hiding this comment.
Is this not breaking change? Though benchmarking is experimental feature 🤔
Contributor
Author
There was a problem hiding this comment.
Right, this is a breaking change. We could start from default true, but I'm hoping that people haven't really needed to have raw samples. For example, since my last PR #5398. benchmark outputJson already omits raw samples.
Member
There was a problem hiding this comment.
@sheremet-va have we previously introduced breaking changes for experimental features in minor releases, or also on patch releases?
Co-authored-by: Ari Perkkiö <[email protected]>
hi-ogawa
commented
Sep 21, 2024
Contributor
Author
hi-ogawa
left a comment
hi-ogawa
left a comment
There was a problem hiding this comment.
Would be great to add test case for this in https://github.com/vitest-tests/ and include it in Ecosystem CI. We already have some similar cases there. This one could be
benchmark-large.
Sounds good! I'll prepare one.
| exclude: defaultExclude, | ||
| includeSource: [], | ||
| reporters: ['default'], | ||
| includeSamples: false, |
Contributor
Author
There was a problem hiding this comment.
Right, this is a breaking change. We could start from default true, but I'm hoping that people haven't really needed to have raw samples. For example, since my last PR #5398. benchmark outputJson already omits raw samples.
…o fix-benchmark-clear-samples
|
📝 Ran ecosystem CI: Open
|
AriPerkkio
approved these changes
Sep 23, 2024
Member
AriPerkkio
left a comment
AriPerkkio
left a comment
There was a problem hiding this comment.
Looks good to me! I've verified that the vitest-tests/benchmark-large crashes without this change.
sheremet-va
approved these changes
Sep 25, 2024
3 tasks
Contributor
|
When can we expect a new release? It'd be good to clarify the release cycle here in the docs. |
Contributor
Author
|
@aleclarson I'm not sure if we'll have a stable release cycle (we'll discuss about it), but we just integrated pkg pr new, so you can grab one from main if you want to test it early https://github.com/vitest-dev/vitest/runs/30935774010 |
renovate bot
added a commit
to mmkal/eslint-plugin-mmkal
that referenced
this pull request
Oct 2, 2024
##### [v2.1.2](https://github.com/vitest-dev/vitest/releases/tag/v2.1.2) ##### 🐞 Bug Fixes - Move `Vitest.setServer` to post `configureServer` hook to enable import analysis for workspace config loading - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6584 [<samp>(e7f35)</samp>](vitest-dev/vitest@e7f35214) - **benchmark**: - Clear `BenchmarkResult.samples` array to reduce memory usage - by [@hi-ogawa](https://github.com/hi-ogawa) and [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6541 [<samp>(a6407)</samp>](vitest-dev/vitest@a6407afc) - **browser**: - Fix dynamic import inside worker - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6569 [<samp>(ea2d4)</samp>](vitest-dev/vitest@ea2d429b) - Fix browser mock factory event race condition - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6530 [<samp>(f131f)</samp>](vitest-dev/vitest@f131f93b) - Serve ui assets as static - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6564 [<samp>(adcda)</samp>](vitest-dev/vitest@adcdaee8) - Update solidjs testing library lib - by [@CamilleTeruel](https://github.com/CamilleTeruel) in vitest-dev/vitest#6548 [<samp>(91442)</samp>](vitest-dev/vitest@91442dfc) - Use `data:` protocol on preview provider file upload - by [@userquin](https://github.com/userquin) in vitest-dev/vitest#6501 [<samp>(e9821)</samp>](vitest-dev/vitest@e9821f70) - Fix base for client script - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6510 [<samp>(f9528)</samp>](vitest-dev/vitest@f952874e) - Throw an error if "@vitest/browser/context" is imported outside of the browser mode - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6570 [<samp>(383f1)</samp>](vitest-dev/vitest@383f1791) - **coverage**: - Remove empty coverage folder on test failure too - by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6547 [<samp>(1371c)</samp>](vitest-dev/vitest@1371ca6a) - Include `*.astro` by default - by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6565 [<samp>(f8ff7)</samp>](vitest-dev/vitest@f8ff76a9) - `cleanOnRerun: false` to invalidate previous results - by [@AriPerkkio](https://github.com/AriPerkkio) in vitest-dev/vitest#6592 [<samp>(88bde)</samp>](vitest-dev/vitest@88bde99c) - **expect**: - Fix `toBeDefined` with `expect.poll` - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6562 [<samp>(f7da6)</samp>](vitest-dev/vitest@f7da6199) - **runner**: - Mark tests as skipped when `beforeAll` failed - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6524 [<samp>(fb797)</samp>](vitest-dev/vitest@fb79792d) - Support fixture parsing of lowered async syntax - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6531 [<samp>(b553c)</samp>](vitest-dev/vitest@b553c7d6) - Fix fixture parsing of lowered async syntax for non arrow functions - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6575 [<samp>(3de00)</samp>](vitest-dev/vitest@3de00ab6) - Guard test hook callback - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6604 [<samp>(14971)</samp>](vitest-dev/vitest@1497134e) - Run `onTestFinished` and `onTestFailed` during `retry` and `repeats` - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6609 [<samp>(c5e29)</samp>](vitest-dev/vitest@c5e29098) - **ui**: - List tests on ui when `--standalone` - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6577 [<samp>(d0bf8)</samp>](vitest-dev/vitest@d0bf89d3) - **vite-node**: - Fix esm false-detection inside comment - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6506 [<samp>(91f85)</samp>](vitest-dev/vitest@91f85997) - **vitest**: - Install dependencies with the same version when prompted - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6611 [<samp>(ed8b7)</samp>](vitest-dev/vitest@ed8b7c08) - Make env.SSR consistent between different pools - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6616 [<samp>(8a8d3)</samp>](vitest-dev/vitest@8a8d3f03) - Don't start a websocket server if api is disabled - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6617 [<samp>(82140)</samp>](vitest-dev/vitest@821400b8) - **workspace**: - Fix glob pattern detection - by [@hi-ogawa](https://github.com/hi-ogawa) in vitest-dev/vitest#6502 [<samp>(7727c)</samp>](vitest-dev/vitest@7727ca87) - Ignore DS_Store by default - by [@sheremet-va](https://github.com/sheremet-va) in vitest-dev/vitest#6571 [<samp>(d2a86)</samp>](vitest-dev/vitest@d2a86ff5) ##### [View changes on GitHub](vitest-dev/vitest@v2.1.1...v2.1.2)
1 task
This was referenced Oct 24, 2024
This was referenced Oct 25, 2024
This was referenced Oct 26, 2024
1 task
This was referenced Oct 28, 2024
This was referenced Oct 30, 2024
This was referenced Nov 5, 2024
This was referenced Nov 6, 2024
4 tasks
1 task
1 task
This was referenced Jan 19, 2025
1 task
spamshaker
pushed a commit
to spamshaker/vitest
that referenced
this pull request
Jun 30, 2025
…y usage (vitest-dev#6541) Co-authored-by: Ari Perkkiö <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I think we should clear
BenchmarkResult.samplesby default on test runner side. I addedbenchmark.includeSamplesoption which allows returning samples, but it's disabled by default.The fix is verified with this reproduction https://github.com/hi-ogawa/reproductions/tree/main/vitest-6539-bench-heap-oom
Plot of "used_heap_size"
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.