Benchmark shards run ~2x slower when PRs overlap, hitting the 120-min timeout on doc-only changes

[marius-bughiu]

Observation

Benchmark shards run roughly 2x slower when more than one PR's Benchmarks workflow is in flight, which pushes the heaviest shards into the timeout-minutes: 120 cap and makes a fully-green PR unreachable.

Baseline — the last green run on main (30429534576, 2026-07-29):

shard	0	1	2	3	4	5	6	7
min	53	53	44	43	40	38	45	64

Same suite on #330 (30502468719), a documentation-only diff with zero IL change:

shard	0	1	2	3	4	5	6	7
min	103	106	85	87	78	71	83	>120 (still running)

Every shard is 1.75–2.0x its baseline. Shard 1 landed at 106 min against a 120 min cap; shard 7 has been running over two hours and will be killed.

On #331 (30502250767) — also documentation-only — benchmark (shard 1) ran from 00:18:08Z to 02:18:09Z, exactly 120 minutes, and was cancelled by the timeout. That single shard is the only non-success among that PR's 18 checks.

What was in flight during the window: three branches' Benchmarks runs overlapping — chore/code-review/lru-mru-hit-enumerator-docs, chore/code-review/intern-table-never-allocates-doc, and fix/issue-333-deque-clear-noop-version (which pushed five times in six minutes). At 8 shards each, that is up to 24 concurrent runner jobs.

Why this matters

1. It is not the PR's fault and cannot be fixed by the PR author. Both affected PRs change only XML doc comments and Markdown. A contributor sees a red or perpetually-pending check caused entirely by what else happened to be running.
2. It defeats the sharding design. benchmarks.yml explains that the matrix was widened 6 → 8 because "the heaviest shard measured ~65 min per slice … and the heaviest shard overran this timeout", and the comment ends: "Re-measure and widen the matrix again if shard durations creep back toward it." That advice assumes shard duration is a property of the suite. It is not — it is a property of the suite times how many other runs are competing, so widening the matrix chases a moving target.
3. It may quietly corrupt the measurements, which is the worse failure. The same-runner A/B invariant is designed so hardware cancels out of the head-vs-base delta. That holds only if head and base see the same contention. With runs starting and finishing at staggered times, a shard's head slice and base slice can experience very different neighbour load, so the delta no longer cancels cleanly. A shard that times out is loud; one that completes under uneven contention silently publishes a skewed number to the dashboard.

Approaches to consider

1. Serialize the workflow. A concurrency: { group: benchmarks, cancel-in-progress: false } at workflow level would queue runs instead of overlapping them. Slower to feed back, but it restores the one-run-at-a-time assumption the timeout and the A/B invariant were both calibrated under, and it makes durations predictable again.
2. Don't run Benchmarks at all on diffs that cannot move a number. A paths-ignore for **/*.md, docs/**, and doc-comment-only changes would have skipped it entirely for both PRs here. This is the cheapest win and independently worthwhile — a 45-minute-per-shard suite on a Markdown edit is pure waste.
3. Raise timeout-minutes — treats the symptom, and if contention is the cause the ceiling just moves.
4. Worth confirming the diagnosis first: re-run the docs(StringInternTable): qualify the "never allocates" claim on GetOrAdd(string) #331 suite when nothing else is queued and check whether shard 1 returns to ~53 min. If it does, contention is confirmed and option 1 or 2 is the fix rather than a wider matrix.

I would do 2 first (cheap, obviously correct) and then 1.

[marius-bughiu]

Root-cause evidence from #349, so this does not have to be re-derived.

The failing shard is always shard 0, and shard 0 is StringHasherBenchmark.

Simulating SelectShard's greedy LPT packing over the real CaseCount values at total = 8 gives:

shard loads: [89, 92, 90, 96, 96, 92, 92, 92]
shard 0:     StringHasherBenchmark (81 cases) + EnumSetBenchmark (8) = 89

StringHasherBenchmark is 81 cases; the next heaviest class in the whole suite is PartialSortBenchmark at 32. The bin packer is working correctly — it balances case counts well (89-96 across eight shards) — but one indivisible class is 81 of them, so shard 0's load is dominated by a single class the packer cannot split. Grepping a cancelled shard 0 log confirms it: StringHasherBenchmark appears 810 times, and nothing else does.

Why it passes on main and fails on PRs. On main a shard measures its slice once. On a PR each shard measures head and then base on the same runner — deliberately, to keep the A/B comparison on one machine — so the same slice costs roughly double and shard 0 lands just past timeout-minutes: 120. Recent main runs (1c7edff, 6b8d50e) are green; the PR path is where it breaks.

It is not load-dependent and not contention-only. Four current PRs fail the same check at the same wall clock:

PR	Adds a benchmark class?	benchmark (shard 0)
#344	no (doc-only)	fail, 2h0m15s
#345	no (doc-only)	fail, 2h0m15s
#346	no (doc-only)	fail, 2h0m15s
#349	yes, 8 cases -> shard 4	fail, 2h0m

Three of those are doc-only, so #300's head/base repacking is not the mechanism here — with no class-list change the head and base packings are identical. I also re-ran shard 0 on a quiet pool and it reproduced, so overlapping-PR contention makes it worse but is not the cause; the baseline cost is already over the line.

Two fixes, and the second looks better.

1. Widen SHARD_TOTAL past 8. The workflow comment already anticipates this ("Re-measure and widen the matrix again if shard durations creep back toward it"). But it does not help much: StringHasherBenchmark is indivisible, so more shards cannot lighten shard 0 below the 81 cases that dominate it. This buys almost nothing.
2. Split StringHasherBenchmark. It is 3x the next heaviest class and is the only reason any shard is near the limit. Splitting it (by hasher family, or by the [Params] axis) lets the packer do its job, and it is the only change that actually reduces the critical path.

CaseCount's heuristic is also worth a look — it is a proxy for cases, not for time, so a class with few cases but slow iterations is invisible to it. Not the problem here, but the same shape of bug.