Skip to content

[CI] Fork PR benchmark baselines can run 8x against stale main #856

Description

@jhinpan

Summary

The Fly DSL test workflow can spend multiple hours running benchmark baselines that are both stale and unusable. This is most visible on pull requests from forks:

  1. the workflow checks out the PR head repository, so origin points at the contributor fork;
  2. the baseline step fetches origin/main, not the PR base repository and base SHA;
  3. a failed baseline is retried against up to eight older commits, with a full editable install and full benchmark suite for every attempt;
  4. unavailable baselines are silently skipped, so the workflow can remain green without producing either main or tag comparisons.

This is a workflow issue, not a regression introduced by the affected kernel PRs.

Evidence

In run 29366839827 for #801:

Runner Active job time Tests Current benchmark Baseline step
MI355 134m35s 30m40s 10m49s 76m06s
MI325 105m33s 14m41s 6m25s 57m23s
Navi 12m28s 1m10s 20s 1m09s

The workflow wall time was 157m50s. The MI355 and MI325 baseline steps each tried the same eight commits (main through main~7) from jhinpan/FlyDSL-lab, not ROCm/FlyDSL.

Every attempt failed for the same AITER API drift:

ImportError: cannot import name 'hk_mla_decode_fwd'
Did you mean: 'hk_mla_v32_decode_fwd'?

The failure happened in one MLA benchmark, but the harness continued through the rest of the suite before returning non-zero:

  • MI355 completed 64/65 benchmark entries per attempt;
  • MI325 completed 45/46 benchmark entries per attempt;
  • all successful rows were then discarded and the next historical commit was rebuilt and rerun.

The final result was:

No usable main benchmark baseline found.
Failed to fetch tags; skipping latest-tag comparison.

Both performance comparison steps exited successfully without comparing anything. A separate fork run, 29364875585, reproduced the same eight failures and spent 75m55s/58m07s in the MI355/MI325 baseline steps.

This is not primarily normal runner queue time. In a sample of the latest 20 complete GPU-matrix runs (13 success, 7 failure, excluding cancelled/docs-only/early checkout failures), median wall time was 154.1 minutes while median runner queue time was about 1-2 minutes. Median active time was 107.3 minutes on MI355 and 89.2 minutes on MI325.

Root cause

1. The baseline remote is the fork

The workflow sets the checkout repository from the PR head:

GITHUB_REPO_NAME: ${{ github.event.pull_request.head.repo.full_name || github.repository }}

It then checks out that repository and later runs:

git fetch origin main --depth=8

For #801, origin/main was jhinpan/FlyDSL-lab@c1b52184, 11 commits behind the PR base ROCm/FlyDSL@fa4f8ae2. The fork also had no tags, so the tag fetch could not produce a release baseline.

The same fork-remote bug exists in the label-triggered multi-GPU main baseline.

2. A single environmental failure is amplified eight times

The workflow loops over up to eight commits. Each iteration creates a worktree, performs an editable install, and runs the complete benchmark suite. Falling back to older source commits is especially ineffective for dependency/API failures: all eight candidates used the same unpinned current AITER checkout and failed identically.

The stale candidates predated #840 and #842, which made this AITER drift a clean skip for current source but cannot repair historical worktrees running their own older harness.

3. Missing performance data still looks successful

The baseline step is continue-on-error, and the comparison steps return zero when their CSV is missing. compare_benchmark.py is currently report-only as well; it does not enforce a regression threshold. The result is high GPU cost without a reliable performance signal.

4. Cancellation is defeated by job-level always()

The workflow has cancel-in-progress: true, but the real GPU test and multi-gpu job conditions use job-level always(). Superseded runs can therefore continue to occupy or wait for runners. The #801 run waited about 21 minutes for its previous run; another observed run had a 277.8-minute concurrency gate even though its MI355 job itself took about 80 minutes.

Cleanup steps should continue to use always(); the problem is the top-level condition on long-running GPU jobs.

Historical regression

  • #526, merged May 15, introduced the baseline workflow, the eight-commit fallback, and the 180-minute baseline timeout.
  • #597, merged June 2, changed the second main baseline to a latest-tag baseline while retaining the fallback loop.
  • Before baseline execution, representative runs completed in about 27-39 minutes.
  • Healthy runs then grew to roughly 50-80 minutes as tests expanded and PRs began running current + main + tag benchmark suites.
  • The stale-fork/AITER failure mode amplified the main suite to eight executions and pushed jobs beyond two hours.

Immediate fix / acceptance criteria

The first fix should remain surgical and limited to the workflow:

  • Resolve the canonical base repository from pull_request.base.repo.full_name.
  • Fetch the exact pull_request.base.sha; for a push to main, compare with github.event.before; retain main for manual dispatch.
  • Apply the same exact-base behavior to the multi-GPU baseline.
  • Attempt the main baseline once; do not hide environmental failures by walking eight older commits.
  • Fetch release tags from the base repository, not the fork.
  • Bound the combined baseline step well below the current 180-minute timeout.
  • Emit visible warnings when a baseline/comparison is unavailable.
  • Replace job-level always() on real GPU jobs with !cancelled() while retaining cleanup always().
  • Preserve the comparison label main for compatibility with the existing CI dashboard parser, while logging the exact repository and SHA.

Based on the observed run, removing the seven redundant retries should save roughly 65 minutes on MI355 and 50 minutes on MI325 in this failure mode.

Follow-up optimization opportunities

These should be reviewed independently rather than expanding the first bug-fix PR:

  1. Pin AITER to a tracked Git SHA and define a stable Triton compatibility policy. [CI] Unpin exact triton wheel hash to fix install failure #600 documents why volatile wheel build-suffix pins are unsuitable, but that does not prevent pinning AITER source revisions.
  2. Preserve partial baseline CSVs and compare the valid row intersection instead of discarding 64 good rows because one optional benchmark failed.
  3. Generate per-architecture baseline artifacts on main/release and let PRs download them, rather than rerunning main and the latest tag on every PR.
  4. Split required correctness CI from full performance CI; use changed paths/run_benchmark.sh --only for focused PR benchmarks and retain full suites for labels/nightly.
  5. Rework the approximately 2.1 GB MLIR cache so PRs restore but only main saves, and remove the entire workflow file from the cache key.
  6. Prebuild/prefetch the CI image and its AITER/Triton/MLIR dependencies.

Related work

  • #840 and #842 address symptoms of AITER API drift, not baseline selection or repeated execution.
  • #852 is an orthogonal open PR for docs-only GPU skipping. Its lightweight required-status shim intentionally uses always() and should not be changed by this fix.
  • #547, #600, and issue [Issue]: CI tests are failing on MI325 and MI355 on all recent PRs #545 show prior repository-wide CI failures caused by AITER/Triton dependency drift.
  • #675 and #737 add performance dashboard consumption but do not provide reusable baseline artifacts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingperformancePerformance related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions