Repository navigation
perf(coverage): speed up v8 report generation (bounded-memory merge + precompiled globs) - #10506
Conversation
github-actions
Bot
added
the
maybe automated
|
Hello @toxik. Your PR has been labeled To keep your PR open, please follow these steps:
Please, do not generate or format the response with AI. If you do not speak English, reply in your native language or use translation software like Google Translate or Deepl. If the response is generated, the PR will be closed automatically. These measures help us reduce maintenance burden and keep the team's work efficient. See our AI contributions policy for more context. |
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Hello, I am indeed a human and there was indeed coding agent help with this PR from Claude Code. |
sheremet-va
removed
the
maybe automated
|
📝 Ran ecosystem CI: Open
|
AriPerkkio
left a comment
AriPerkkio
left a comment
There was a problem hiding this comment.
The early coverage map merging is there intentionally. We cannot hold all coverage results in memory at once, as it would lead to out-of-memory crashes. That's the reason why we do file-system round-trip in the first place, instead of just holding everything in memory.
Related issue:
generateCoverage() folded mergeProcessCovs([merged, coverage]) once per coverage file and re-scanned the growing merged.result on every file to repair startOffset/isExtendedContext, making the read phase O(n^2) in the number of coverage files. Collect the raw coverages and merge them in a single pass, restoring the dropped fields via lookup maps built while reading. Profiled on a real 1124-test-file app: the read+merge phase dropped from ~24.5s to ~0.3s (single merge ~95ms). Together with the isIncluded fix, total coverage generation went from 52.8s to 7.1s. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
isIncluded() called picomatch.isMatch(file, patterns, options) per file, which recompiles every glob into a regex on each call. The globCache does not help during coverage filtering because each result entry has a unique filename, so every call paid the full compile cost. Compile the include/exclude matchers once and reuse them. Profiled on a real 1124-test-file app: the include/exclude filtering step dropped from ~23.3s to ~0.38s (~61x), with identical results. Together with the single-pass merge, total coverage generation went from 52.8s to 7.1s. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
14a9977 to
65dd2ad
Compare
Co-authored-by: Ari Perkkiö <[email protected]>
65dd2ad to
43c3903
Compare
toxik
changed the title
|
📝 Ran ecosystem CI: Open
|
|
📝 Ran ecosystem CI: Open
|
AriPerkkio
changed the title
There was a problem hiding this comment.
Looks good now, thanks for addressing all the findings @toxik! 💯
Sorry for long review time.
On my machine (M4 Pro chip) I saw following improvements using artificial benchmark:
┌────────────────┬───────────────────────┬───────────────────┬─────────┐
│ coverage files │ old incremental merge │ new per-URL merge │ speedup │
├────────────────┼───────────────────────┼───────────────────┼─────────┤
│ 200 │ 77 ms │ 38 ms │ 2.0x │
├────────────────┼───────────────────────┼───────────────────┼─────────┤
│ 800 │ 795 ms │ 125 ms │ 6.4x │
├────────────────┼───────────────────────┼───────────────────┼─────────┤
│ 2000 │ 5368 ms │ 271 ms │ 19.8x │
└────────────────┴───────────────────────┴───────────────────┴─────────┘
Memory usage is a bit higher than in main, but it's not crashing:
- This PR: https://github.com/vitest-dev/vitest-ecosystem-ci/actions/runs/29315467267/job/87028381562
- Main: https://github.com/vitest-dev/vitest-ecosystem-ci/actions/runs/29314233868/job/87024505831
Description
Two independent, self-contained performance fixes to coverage report generation (
generateCoverage/BaseCoverageProvider), measured on a real 1124-test-file app (~39 MB of raw V8 coverage across the run). Numbers below are best-of-3 runs, baseline vs this PR on the same machine:isIncludedfiltering)generateCoveragePeak heap during coverage generation drops from ~2.5 GB → ~1.15 GB, and resident raw coverage is now bounded to ≤16 MB regardless of suite size.
Tests
No behavior change. Both fixes are verified against the existing
test/coverage-testsuites (v8 + istanbul), which assert coverage output and the set of included files.pnpm typecheckandpnpm lintpass. A backport to thev4release line will follow.🤖 Generated with Claude Code