fix(runner): limit concurrency per task branch in addition to per leaf callbacks

[hi-ogawa]

Description

• Closes maxConcurrency no longer bounds in-flight tests in describe.concurrent, causing per-test resource leaks (regression from v3?) #10097
• Closes Reporting regression in V4: incorrect per-test duration with concurrent #10069
• Closes fix(runner): use per-group limiter for DFS-bounded concurrent scheduling #10108 (supersedes)

In #9653, we've changed concurrency limit from per-runTest level to per-indivisual user callbacks. While it sounds clean in principle, it lost runTest level serialization between siblings.

For example, with the following tests and maxConcurrent = 1:

beforeEach(async ({ task }) => {
  console.log("!> beforeEach", task.name)
  await sleep(10)
  console.log("!> beforeEach", task.name)
})

afterEach(async ({ task }) => {
  console.log("!> afterEach", task.name)
})

test.concurrent.for(["a", "b"])("%s", (_, { task }) => {
  console.log("!> test", task.name)
  await sleep(10)
  console.log("!< test", task.name)
})

before #9653:

!> beforeEach a
!< beforeEach a
!> test a
!< test a
!> afterEach a
!< afterEach a
!> beforeEach b
!< beforeEach b
!> test b
!< test b
!> afterEach b
!< afterEach b

and current main after #9653:

(Notably this still guarantees each callback level in-out concurrency limit but not beforeEach -> test -> afterEach lifecycle level in-out)

!> beforeEach a
!< beforeEach a
!> beforeEach b
!< beforeEach b
!> test a
!< test a
!> test b
!< test b
!> afterEach a
!< afterEach a
!> afterEach b
!< afterEach b

This PR brings back siblings serialization guarantee by introducing independent concurrency limit at each task branch. This now also guarantees sibling suite hooks concurrency guarantee which has never existed even before #9653.

TODO

• summary
• test

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]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	9d9e78d
🔍 Latest deploy log	https://app.netlify.com/projects/vitest-dev/deploys/69eff679f81d8e0008ae3bd8
😎 Deploy Preview	https://deploy-preview-10179--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[dbousamra]

@hi-ogawa Awesome! How can i test this pre release?

[pkg-pr-new]

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@10179

@vitest/browser-playwright

npm i https://pkg.pr.new/@vitest/browser-playwright@10179

@vitest/browser-preview

npm i https://pkg.pr.new/@vitest/browser-preview@10179

@vitest/browser-webdriverio

npm i https://pkg.pr.new/@vitest/browser-webdriverio@10179

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@10179

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@10179

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@10179

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@10179

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@10179

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@10179

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@10179

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@10179

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@10179

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@10179

vitest

npm i https://pkg.pr.new/vitest@10179

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@10179

commit: 9d9e78d

[hi-ogawa]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[hi-ogawa]

@dbousamra Please test it out the package in #10179 (comment).

[dbousamra]

Hi all.
When will this be released approx?

[hi-ogawa]

@dbousamra The fix is included in v5 beta for now https://github.com/vitest-dev/vitest/releases/tag/v5.0.0-beta.2 We can backport to v4 depending on user request (mainly you 😃)

[dbousamra]

@dbousamra The fix is included in v5 beta for now https://github.com/vitest-dev/vitest/releases/tag/v5.0.0-beta.2 We can backport to v4 depending on user request (mainly you 😃)

All depends on when we can expect a 5.0.0 proper release. Is it day, weeks, months away? Is 5.0.0 beta suitable for use now?

[hi-ogawa]

Stable 5 would be a month away. It's not necessary "unstable" but it's just the phase where we pile up breaking changes as beta progress. I'll consider backporting this one liner patch to v4 soon.

[dbousamra]

Hi @hi-ogawa

I've tried upgrading our project from v3 to v4.1.7. Definitely better, however still a huge performance difference between v3 and v4. Here's an AI summary of my attempts at different configs:

  ┌──────────────────────────────────┬───────────┬─────────────┬───────────────────────────────┐
  │              Config              │   Wall    │     CPU     │             Notes             │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v3 baseline                      │ 33.94s    │ 468%        │ (target)                      │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 starting (maxConcurrency: 4)  │ 72.51s    │ 227%        │ the regression                │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 no maxConcurrency (default 5) │ 85.80s    │ 191%        │ worst                         │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 pool: 'threads'               │ 75s       │ 221%        │ no help                       │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 isolate: false                │ 75s       │ 212%        │ + flaky failures (state leak) │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 experimental.viteModuleRunner │ 75s       │ 220%        │ + failures                    │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 experimental.fsModuleCache    │ 53s / 62s │ 311% / 268% │ marginal                      │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 maxConcurrency: 20            │ 38.40s    │ 414%        │ 47 race failures              │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 maxConcurrency: 15            │ 50s       │ 332%        │ 11 race failures              │
  ├──────────────────────────────────┼───────────┼─────────────┼───────────────────────────────┤
  │ v4 mc:10 + seq.concurrent        │ 45–63s    │ 270–370%    │ passes, best safe             │
  └──────────────────────────────────┴───────────┴─────────────┴───────────────────────────────┘

How do I go about investigating this further? Is this just an expected performance decrease, or should I chase this further?

[hi-ogawa]

@dbousamra I cannot tell anything actionable from those numbers. Probably worth experiments would be to try patching this part of concurrency (like hard-code huge number here), then keep limiter at suite branch only (the one introduced in this PR). If that improves both stability and perf, then the bottleneck could be the leaf level acquire logic. That might narrow down minimally reproducing something actionable.

vitest/packages/runner/src/run.ts

Lines 1009 to 1010 in 4fee2e3

	export async function runFiles(files: File[], runner: VitestRunner): Promise<void> {
	limitMaxConcurrency ??= limitConcurrency(runner.config.maxConcurrency)

[dbousamra]

@hi-ogawa thanks. Will try.

Are you able to tell me what the default concurrency is on v4? And that would mean the hooks are running with the tests yeah? I.e. concurrency of 4, would be 4 tests AND hook running at same time, never more?