Skip to content

perf: optimize --merge-reports #8710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: optimize --merge-reports #8710

wants to merge 1 commit into from

Conversation

@7rulnik
Copy link
Contributor

@7rulnik 7rulnik commented Oct 13, 2025

Description

I noticed that vitest run --merge-reports takes a lot of time in our CI. After short profiling I discovered that bottleneck is parsing via flatted.

Author of flatted suggests to move to https://github.com/ungap/structured-clone

Here are some benchmarks:

# before
hyperfine --warmup 3 --runs 50 "node node_modules/vitest/vitest.mjs  run --merge-reports --reporter=json --outputFile.json=./vitest-reports/report.json || true"
Benchmark 1: node node_modules/vitest/vitest.mjs  run --merge-reports --reporter=json --outputFile.json=./vitest-reports/report.json || true
  Time (mean ± σ):      4.024 s ±  0.151 s    [User: 4.413 s, System: 0.416 s]
  Range (min … max):    3.826 s …  4.390 s    50 runs

# after
hyperfine --warmup 3 --runs 50 "node node_modules/vitest/vitest.mjs  run --merge-reports --reporter=json --outputFile.json=./vitest-reports/report.json || true"
Benchmark 1: node node_modules/vitest/vitest.mjs  run --merge-reports --reporter=json --outputFile.json=./vitest-reports/report.json || true
  Time (mean ± σ):      1.926 s ±  0.059 s    [User: 2.632 s, System: 0.369 s]
  Range (min … max):    1.758 s …  2.039 s    50 runs

And some profiling via cpupro:

image image

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
  • Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

@netlify
Copy link

netlify bot commented Oct 13, 2025

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6521656
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/68ed580ad72853000807af10
😎 Deploy Preview https://deploy-preview-8710--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

7rulnik
7rulnik commented Oct 13, 2025
View reviewed changes
packages/ui/client/components/FileDetails.vue
Comment on lines -110 to -113
moduleGraph
= typeof window.structuredClone !== 'undefined'
? window.structuredClone(moduleGraph)
: toJSON(moduleGraph)
Copy link
Contributor Author

@7rulnik 7rulnik Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As I understood toJSON was used as polyfiil. Should we keep it?

structuredClone available in all major browsers for 3 years already https://caniuse.com/wf-structured-clone

7rulnik
7rulnik commented Oct 13, 2025
View reviewed changes
test/reporters/package.json
"test": "vitest"
},
"devDependencies": {
"@ungap/structured-clone": "^1.3.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be?

Suggested change
"@ungap/structured-clone": "^1.3.0",
"@ungap/structured-clone": "catalog:",

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's have it in the catalog file since it's used everywhere

7rulnik
7rulnik commented Oct 13, 2025
View reviewed changes
packages/browser/src/node/rpc.ts
Comment on lines 371 to +372
* Replacer function for serialization methods such as JS.stringify() or
* flatted.stringify().
* flatted.stringify(). // here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to write that comment correctly because the name of dep is more complicated now

7rulnik
7rulnik commented Oct 13, 2025
View reviewed changes
packages/vitest/src/utils/serialization.ts
Comment on lines 16 to +17
* Replacer function for serialization methods such as JS.stringify() or
* flatted.stringify().
* flatted.stringify(). //here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://github.com/vitest-dev/vitest/pull/8710/files#r2427220945

@7rulnik
Copy link
Contributor Author

7rulnik commented Oct 13, 2025

I am not so sure about tests… I tried to run pnpm run test:ci on main branch and got some of test failed. And looks it's not really stable tests. I mean set of tests that run is different every time.

@7rulnik 7rulnik changed the title perf: optimize reports merge perf: optimize --merge-reports Oct 13, 2025
@sheremet-va
Copy link
Member

I am not so sure about tests… I tried to run pnpm run test:ci on main branch and got some of test failed. And looks it's not really stable tests. I mean set of tests that run is different every time.

Some tests are indeed flaky, but the changes in your PR clearly broke other tests, like reporters: https://github.com/vitest-dev/vitest/actions/runs/18476660566/job/52642594279?pr=8710

The lint stage should also pass without errors

@7rulnik
Copy link
Contributor Author

7rulnik commented Oct 21, 2025

Just in case: I will continue work on it a bit later. I am short on time during few weeks probably.

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

Reviewers

@sheremet-va sheremet-va sheremet-va left review comments

At least 1 approving review is required to merge this pull request.

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

@7rulnik @sheremet-va