Skip to content

perf(ci): refresh server test shard weights from CI timings (slowest shard 146.5 s → 125.1 s median) - #30866

Open
Pyseph wants to merge 1 commit into
pingdotgg:mainfrom
Pyseph:perf/ci-server-shard-weights
Open

Pyseph wants to merge 1 commit into
pingdotgg:mainfrom
Pyseph:perf/ci-server-shard-weights

Conversation

@Pyseph

@Pyseph Pyseph commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Test Server splits the 463 server test files over 8 runners with WeightedShardSequencer, balanced on apps/server/src/testUtils/shardWeights.json. #20044 rebuilt that table from CI wall times on Oct 15; since then #22557 scaled the weights, #22717 and #22753 edited entries by hand, and new test files were added, so it no longer describes the CI runners:

  • 253 of 463 files have no entry (the table also keeps 4 for deleted files) and are assumed to take 0.6 s. In CI 114 of them take 0.5 s or more (the biggest: scripts/dev-replay-agent.test.ts 9.1 s, userInputAttachments.test.ts 9.5 s, KiroReplayFixtures 8.3 s, ClaudeAdapterV2 6.2 s, RemoteDelegation 5.8 s).
  • Git-heavy files run several times slower than recorded: GitVcsDriverCore.test.ts 7.2 s recorded, 32.6 s median in CI; GitManager.test.ts 13.7 s vs 33.6 s; storageCleanup.test.ts 1.4 s vs 12 s.

So the weight sums per shard are even (about 68 s each) but the real shards are not. In the 20 runs below the slowest shard's tests took a median 147 s, the fastest 91 s (a 55 s median spread). The slowest Test Server job is the longest job of a PR in 8 of those 20 runs (every run where Test Web did not run), and in 10 of 20 once the 22 s step that #30752 moves out of Test Web is gone.

Change

Data only: replace shardWeights.json with the per-file median of the vitest Duration line from 20 recent successful ci.yml runs (160 shard logs), written in the same format as update-test-shard-weights.ts writes (0.1 s rounding, files of at least 0.5 s, keys sorted, two-space indent, trailing newline; the old file round-trips through that formatting byte for byte). The measure differs from the generator's: vitest's Duration includes import time, as the sequencer's comment asks, where the generator records local test time. It also differs from #20044, which used the gaps between shard log lines; weights built from those gaps scored worse here (slowest shard 128.8 s against 125.1 s). 299 files are recorded instead of 214: 114 newly, and 29 old entries dropped (25 files whose CI median is below 0.5 s, plus the 4 deleted files).

Nothing else changes: the sequencer, test:cached, the matrix and the tests are untouched. Only which files share a runner changes; each file still runs alone in its own vitest process, so test isolation (fileParallelism: false) and per-file cache keys are the same.

Why not the generator: update-test-shard-weights.ts runs the whole suite once on the machine it is started on and records that machine's timings. I did not change it; like #20044, this is a CI-derived data refresh. Teaching the generator to read the test-cache-N JSON that shards already upload would keep the table fresh, but that is a larger change I would only attempt after a maintainer asks for it.

Relation to #28746 ("drop shard weights for deleted test files"): this file contains none of the four deleted entries that PR removes, so it is a superset of it. Whichever lands first, the other conflicts textually in this one file; if #28746 lands first, rebase this on it and keep this version of the table (the same four keys are absent). If this lands first, #28746 becomes a no-op.

Scope and approval

No issue or discussion: behaviour-preserving CI maintenance of a data file that the repo already regenerates by hand (#14025, #20044, #22557, #22717, #22753 all updated it). No test, product or workflow code changes. If you would rather discuss it first, I am happy to move it to an Ideas discussion.

Verification

Method and raw rows: Duration of every vp test run '<file>' in the 8 shard logs of 20 successful ci.yml PR runs (41800013420 ... 41810223239, 2026-11-04 08:30-09:50 UTC, 160 jobs, no task-cache hits), via gh api repos/pingdotgg/t3code/actions/jobs/<id>/logs.

  1. The sequencer port reproduces CI. I re-implemented WeightedShardSequencer.shard() (sort by weight descending then path, greedy to the first least-loaded shard) and applied it, with the current table, to each run's file list: all 9,357 file placements in the 20 runs match the shard each file actually ran on. I also ran the repository's real weightedShardSequencer.ts (only the vitest base class stubbed) over the 463 files with the old and the new table: identical split to the port in both cases.
  2. Leave-one-run-out: for each run, weights = median of the other 19 runs; sequencer split of that run's files; shard time = sum of that run's real per-file wall times (start of one vp test run to the start of the next). The slowest shard's tests, current table -> refreshed table:
median mean range runs improved
slowest shard, tests (s) 146.5 -> 125.1 145.1 -> 124.5 112-178 -> 102-167 20 of 20 (smallest gain 2.2 s; median gain per run 17.5 s)
slowest minus fastest shard (s) 55 -> 18

A perfectly even split would be 112 s (mean shard), so 125 s leaves 13 s of noise and the 10-17 s per-shard task-runner overhead. Run-to-run noise on the git-heavy files is large (25-48 s for GitVcsDriverCore), which is why weights from a single run would not do.
3. Different time of day: the table from the 20 runs, scored on 9 older runs from 05:51-06:19 UTC (vitest Duration + 0.25 s per file): slowest shard 141.8 -> 123.4 s median, 8 of 9 improved, one 1 s worse.
4. Job level (real job durations, shard job time adjusted by the simulated change of its own shard sum): the slowest Test Server job over the 20 runs 173 -> 148 s median. Where a server shard was the longest job of the run (8 of 20), the longest job gets 20.4 s shorter at the median (7.4-35.1 s, 12.0 % of the job). Where Test Web is longer the PR does nothing for that run (it is not slower anywhere).

Not checked: the real effect in CI (this PR's own run shows one more sample); the weights will go stale as tests are added, like the current ones did.

Claude Sonnet 5.5 and Claude Opus 5.5 in T3 Code (Claude Code harness).

pingdotgg#20044 rebuilt shardWeights.json from CI wall times; since then weights were scaled and edited by hand and new test files were added, so the table names 214 of 463 server test files and rates git-heavy files at a fraction of their CI time (GitVcsDriverCore 7.2 s recorded, about 32 s on the shard runners). The 8 shards ended up 90-180 s apart.

Replace the table with the per-file median of the vitest Duration lines from 20 recent successful ci.yml runs, written in the format update-test-shard-weights.ts uses (0.1 s rounding, files of 0.5 s or more, sorted keys). Only which files share a runner changes; each file still runs alone through test:cached. The four entries for deleted test files are gone as a side effect.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Nov 4, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b42fca3

Macroscope's review found this PR approvable — This refresh changes server-test shard timing weights, affecting runner balance rather than product behavior or test coverage. The change is confined to an ignored data path, with no material operational exposure identified.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The test-file shard-weight map is updated. Existing weights change, and entries are added or removed to match the listed test files.

Changes: Test shard weights

Layer / File(s) Summary
Update test shard weights
apps/server/src/testUtils/shardWeights.json
The map has updated weights for existing tests, entries for newly listed tests, and no entries for removed tests.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Suggested reviewers: t3dotgg

Merge Risk: Minimal · up to b42fc

This data-only update is mergeable with normal CI validation; no actionable runtime or test-selection failure is established.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the performance-focused CI change: refreshing server test shard weights from CI timings. It uses a conventional commit format and includes the measured improvement without…
Description check Passed The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the motivation, implementation, scope, measured results, limitations, and agent detai…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly identifies the performance-focused CI change: refreshing server test shard weights from CI timings. It uses a conventional commit format and includes the measured improvement without unrelated detail.

Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the motivation, implementation, scope, measured results, limitations, and agent details. The verification is specific and reports what was not checked.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9e85538 and b42fca3.

Files selected for processing: 1
  • apps/server/src/testUtils/shardWeights.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@Pyseph

Pyseph commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Note

Written by Claude Opus 5.5 on behalf of Pyseph

This PR's own CI run, 41842144065, is one real sample of the refreshed table. Its 8 Test Server jobs took 129-166 s, a 37 s spread.

For comparison, here are the 32 other successful ci.yml runs from the same window, 13:36-14:20 UTC, all on the current table. I excluded 7 more whose shards finished in 12-24 s, since those were task-cache hits.

This PR, refreshed table Same-window runs on the current table (n=32)
Slowest Test Server job 166 s median 207.5 s (155-308); this run was faster than 28 of 32
Slowest minus fastest shard job 37 s median 76.5 s (16-165); this run was narrower than 31 of 32

The runners were slower this afternoon than in the 08:30-09:50 UTC window the table was built from, so these times don't compare directly with the 173 -> 148 s in the description. It's still a single run.

Run IDs (job durations from gh run view --json jobs)

41846542366 41846071671 41845668698 41845446469 41844377892 41844365840 41844349458 41844199707 41844020617 41843959497 41843650911 41843285900 41843265627 41843195573 41843037751 41842994156 41842566700 41842461330 41842068955 41841763173 41841644879 41841423676 41841383920 41841007240 41840586090 41840401266 41840373857 41840165501 41839748160 41839662101 41839615522 41839577747

This branch has not been deployed

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

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant