Repository navigation
Raise perf timeout 300->600 (post-#2 shard rotation tipped knife-edge case) - #4
Merged
Merged
Conversation
Post-#2 shard rotation tipped the ~45B-instruction edit_distance case from usually-pass to usually-timeout at 300s wall clock. 2x headroom; genuine hangs still fail. AI-Tool: Muse Code (Meta Muse Spark)
michael-h-hu
approved these changes
Nov 3, 2026
michael-h-hu
left a comment
There was a problem hiding this comment.
Review: PR #4 — Raise perf timeout 300->600 (round 1)
Verdict: Approve — no findings
What was inspected
- Full diff (
gh pr diff 4): single file.github/workflows/test.yml,
measure.py --timeout 300->--timeout 600plus a 4-line comment
explaining why (edit_distance/test_case_5 ~45B Ir on the 300s
wall-clock knife-edge under callgrind with --jobs 2; post-#2 shard
rotation tipped it to usually-timeout). - Comment accuracy: matches the known master failure (run 41719149101,
edit_distance/test_case_5 timeout) and the mechanism (wall-clock
timeout under callgrind, not an instruction-count regression). - Scope: CI-only change, no production code, no test logic touched.
The per-case timeout only bounds wall-clock per measurement; the
instruction counts being measured are unaffected. - YAML validity: workflow parses (validated).
- CI:
gh pr checks 4— all green on HEAD 982e7d7, including all four
perf-callgrind shards and the report job (run 41726600850). The
previously-failing shard shape now passes with the new timeout.
Why this is the right fix
The failure is environmental (callgrind wall-clock variance + shard
packing), not a code regression, so raising the wall-clock bound with
documented rationale is the minimal correct fix — not a test weakening.
Genuine hangs still fail at 600s, as the comment notes.
Residual risks / notes
- A 2x timeout doubles worst-case CI time for a genuinely-hung case;
acceptable for a perf-measurement job. - If the case keeps growing toward 600s, the next step would be
re-sharding or splitting the case, not another bump. No action now.
AI-Tool: Muse Code (Meta Muse Spark)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Un-reds master after #2: raises the perf-callgrind case timeout 300→600s.
Root cause
No defect in #2. It added 1 snippet (11 cases), and
measure.pyshardsby snippet — shard 3's membership rotated, and the new co-scheduled
neighbors under
--jobs 2tippeddp/edit_distance/solution/test_case_5(~45B instructions, sits exactly on the 300s wall-clock knife-edge under
callgrind) from usually-pass to usually-timeout (4 failures in 5 attempts
post-land; it passes at 44–45B Ir when the runner is fast).
Fix
2x timeout headroom with an explanatory comment. Genuine hangs still
fail; nothing measured changes.
Validation
YAML parsed. CI on this PR runs the affected shard — green here plus a
master re-run proves the fix.
AI-Tool: Muse Code (Meta Muse Spark)