Skip to content

Comments

perf: optimize --merge-reports#8710

Closed
7rulnik wants to merge 1 commit intovitest-dev:mainfrom
7rulnik:optimize-flatted
Closed

perf: optimize --merge-reports#8710
7rulnik wants to merge 1 commit intovitest-dev:mainfrom
7rulnik:optimize-flatted

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

@7rulnik 7rulnik Oct 13, 2025

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

@sheremet-va sheremet-va Oct 21, 2025

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

@7rulnik 7rulnik Oct 13, 2025

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

@7rulnik 7rulnik Oct 13, 2025

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

sheremet-va commented Oct 14, 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.

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.

@hi-ogawa hi-ogawa self-assigned this Dec 15, 2025
@hi-ogawa hi-ogawa mentioned this pull request Jan 29, 2026
Draft
7 tasks
@hi-ogawa hi-ogawa removed their assignment Feb 2, 2026
@7rulnik
Copy link
Contributor Author

7rulnik commented Feb 23, 2026

closed in favor of #9549

@7rulnik 7rulnik closed this Feb 23, 2026
@7rulnik 7rulnik deleted the optimize-flatted branch February 23, 2026 13:59
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@7rulnik @sheremet-va @hi-ogawa