Skip to content

Raise perf timeout 300->600 (post-#2 shard rotation tipped knife-edge case) - #4

Merged
m-hu merged 1 commit into
masterfrom
fix/timeout-knife-edge
Nov 3, 2026
Merged

m-hu merged 1 commit into
masterfrom
fix/timeout-knife-edge

Conversation

@m-hu

@m-hu m-hu commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

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.py shards
by snippet — shard 3's membership rotated, and the new co-scheduled
neighbors under --jobs 2 tipped dp/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)

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 michael-h-hu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 600 plus 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)

@m-hu
m-hu merged commit 5885d8c into master Nov 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants