Skip to content

ci(gc): give gc-native-roots a concurrency group - #7393

Merged
proggeramlug merged 2 commits into
mainfrom
ci/gc-native-roots-concurrency
Aug 4, 2026
Merged

ci(gc): give gc-native-roots a concurrency group#7393
proggeramlug merged 2 commits into
mainfrom
ci/gc-native-roots-concurrency

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Three of this workflow's four platform arms have never executed.

Measured, not inferred

I went looking because #7392 asks whether an RS4GC segfault is ELF-specific — macOS uses the same lowering, so if it were lowering-generic that arm should fail too. Checking ten consecutive gc-native-roots runs:

2026-08-04T18:55  fix/rs4gc-arm-clang-pair     macos=queued
2026-08-04T18:51  main                         macos=queued
2026-08-04T18:51  fix/rs4gc-arm-clang-pair     macos=queued
2026-08-04T18:50  main                         macos=queued
… 10/10 queued, zero executions

ubuntu-latest (x86-64 ELF) and windows-latest (PE) the same. Only the aarch64 arm has ever reached a runner — which is precisely why it is the only arm anyone has ever seen go red or green.

Cause

The workflow has no concurrency block at all, so nothing supersedes a stale run and the four-arm matrix multiplies on every push. Today's merge volume saturated the queue outright.

cancel-in-progress: false alone would not fix it: GitHub allows at most one PENDING run per group and cancels the previously pending one when a new run enters, regardless of that setting (#7205). Keying push runs on the SHA gives every merged commit its own group while PR runs supersede freely — the shape llvm-inprocess.yml already uses from #7357.

Why it matters beyond tidiness

This is CLAUDE.md's fourth hazard in a different guise: three quarters of this matrix has been reporting nothing while presenting as four-platform coverage. Every "the ELF arm is the only one red" conclusion drawn today — including mine — rested on arms that had simply never run.

It also makes #7392 unanswerable as filed. "Is the segfault ELF-specific?" cannot be distinguished from "the macOS arm has never run the probe" until this lands.

Summary by CodeRabbit

  • Chores

    • Improved automated validation run management by preventing outdated pull-request runs from continuing unnecessarily.
    • Ensured push validations remain separated by commit, improving coverage across supported environments and reducing stale results.
  • Documentation

    • Added release documentation describing the updated validation behavior and improved platform coverage.

Ralph Küpper added 2 commits August 4, 2026 21:00
The workflow had none, so nothing ever superseded a stale run and its
four-arm matrix multiplied across every push. Ten consecutive runs were
checked: the macos-14 arm was `queued` in EVERY one of them -- never
executed, not once. ubuntu-latest (x86-64 ELF) and windows-latest (PE)
likewise. Only the aarch64 arm ever reached a runner, which is why it was
the only arm ever observed red or green.

Three quarters of this matrix has been reporting nothing while presenting
as four-platform coverage. It also made #7392 unanswerable: whether that
segfault is ELF-specific cannot be told apart from "the macOS arm has
never run the probe".

cancel-in-progress: false alone would not fix it -- GitHub allows at most
one PENDING run per group and cancels the previously pending one when a
new run enters, regardless of that setting (#7205). Keying push runs on
the SHA gives every merged commit its own group while PR runs supersede
freely, the same shape llvm-inprocess.yml already uses (#7357).
@proggeramlug
proggeramlug merged commit c49c92d into main Aug 4, 2026
0 of 12 checks passed
@proggeramlug
proggeramlug deleted the ci/gc-native-roots-concurrency branch August 4, 2026 19:01
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f93b205-2c38-49e7-91cb-284a5ffc2c47

📥 Commits

Reviewing files that changed from the base of the PR and between 7797198 and c8bc26f.

📒 Files selected for processing (2)
  • .github/workflows/gc-native-roots.yml
  • changelog.d/7393-gc-native-roots-concurrency.md

📝 Walkthrough

Walkthrough

The GC native roots workflow now groups runs by event and ref or commit SHA. Pull-request runs cancel older in-progress runs, while push runs remain separate. A changelog entry documents the change.

Changes

GC native roots concurrency

Layer / File(s) Summary
Concurrency wiring and documentation
.github/workflows/gc-native-roots.yml, changelog.d/7393-gc-native-roots-concurrency.md
The workflow adds event-specific concurrency groups. Pull-request runs cancel predecessors, and push runs use commit SHA groups without cancellation. The changelog documents the behavior and matrix-run effects.

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

Possibly related PRs

  • PerryTS/perry#7228: Both changes use SHA-keyed push groups and superseding pull-request runs in GC workflows.
  • PerryTS/perry#7253: Both changes modify moving-GC workflow concurrency and cancellation behavior.
  • PerryTS/perry#7357: Both changes use event and ref-or-SHA-based GitHub Actions concurrency controls.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/gc-native-roots-concurrency

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

proggeramlug added a commit that referenced this pull request Aug 4, 2026
* docs(plan): fold in the 2026-08-04 findings

Two things this plan treated as measured were not.

Statepoints could not compile on aarch64-ELF at all -- a hard failure on
a default-on path, from two stacked bugs (#7390: the compact stack-map
parser did not model GNU-as `sym = expr`, emitted only at -O3 and only on
ELF; and the assembler was not told the -mcpu the code generator was
told, so Graviton-emitted SVE was rejected) behind two toolchain ones
(#7384, #7388).

And three of the four RS4GC matrix arms had NEVER executed, in any run,
for want of a concurrency group (#7393). Every "the ELF arm is the only
one red" conclusion rested on arms that never reached a runner. That is a
fifth way a gate cannot fail, and it is now written down.

Also folded in: nine Layer 3 rooting fixes and the rule they share
(ordering, not missing roots; a fault that MOVES is a real fix, one that
does not move by a byte was already dead before you rooted it); #7380's
type confusion and the `gc_type == GC_TYPE_OBJECT` generalisation; RSS
-69% (#7377); and the first honest performance measurement -- two
benchmarks that measure nothing (#7395) and the array-store guard's
siting cost (#7396).

The Layer 1 framing is corrected: lower_exprs_rooted already implements
the RFC's proposal for codegen operands, gated on
any_later_ref_may_trigger_gc, and all four arms of func_ref.rs use it. So
the gap is Layer 3, where #7389 supplies the first structural answer.

* docs: name the fragment for its real PR (#7397)

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.

1 participant