Repository navigation
ci: split the simulation benchmarks into shards by measured time - #22876
alexander-akait wants to merge 1 commit into
Conversation
|
coderabbitai
Bot
added
the
area: types
|
This PR is packaged and the instant preview of its latest commit is available. Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@22876
yarn add -D webpack@https://pkg.pr.new/webpack@22876
pnpm add -D webpack@https://pkg.pr.new/webpack@22876 |
Types CoverageCoverage after merging ci/balance-benchmark-shards into main will be
Coverage Report |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Change: Other Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
The title uses valid Conventional Commit syntax with the allowed type ci and accurately describes the benchmark sharding change. The branch prefix is not provided, so the required type-to-branch-pre… |
Provide the branch name or confirm that its prefix is ci. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Title check
-
Explanation
The title uses valid Conventional Commit syntax with the allowed type
ciand accurately describes the benchmark sharding change. The branch prefix is not provided, so the required type-to-branch-prefix match cannot be verified.Resolution
Provide the branch name or confirm that its prefix is
ci.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
Merging this PR will improve performance by 40.48%2 warnings, 1 note
Performance Changes
Tip Curious why performance improved? Comment Comparing 
|
Summary
On PRs the run isn't finished until the slowest benchmark shard is: it ends ~66 min after the run starts, while every other job is done by ~38 min. The 6 simulation shards are dealt round-robin, so they range from 25 to 59 min.
The shards are now split by measured time:
test/benchmarkTimings.jsonholds each task's seconds, taken from the logs of the last two green runs. Tasks go longest first, each to the shard with the least time so far. Every shard computes the same split independently. A task missing from the file counts as the median, so a new benchmark still runs. Memory mode keeps round-robin, since its two shards are already even (12–13 min).Measured work per shard (minutes):
Real runs vary per task by up to ±25%, so I expect the slowest shard at ~45–48 min instead of 52–59.
It also carries the one-line
ParserOptionstypedef from #22875, without whichlintfails onmain. It no-ops once #22875 lands.What kind of change does this PR introduce?
ci
Did you add tests for your changes?
No. I checked the split locally against the timings: each of the 226 tasks lands on exactly one shard, and an unknown task gets a shard. The benchmark jobs on this PR are the real check.
Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
Claude Code measured the shard times from CI logs and wrote the change on my request.
Generated by Claude Code
Summary by CodeRabbit