Skip to content

ci: save rust-cache on main only; finish LLBC artifact handoff#209

Open
youknowone wants to merge 2 commits into
mainfrom
ci-rust-cache-save-on-main
Open

ci: save rust-cache on main only; finish LLBC artifact handoff#209
youknowone wants to merge 2 commits into
mainfrom
ci-rust-cache-save-on-main

Conversation

@youknowone

Copy link
Copy Markdown
Owner

Problem

PR #198's macOS CI jobs failed fast at the Check prepared cache hits guard: the prepared LLBC entry that prepare-charon-llbc saved was LRU-evicted before the late-scheduled macOS consumers restored it. The repo's Actions cache had exceeded GitHub's 10 GB limit (~10–12 GB across main + open PR refs), and the dominant consumers are the per-ref Swatinem rust-cache target caches (~500–600 MB each × job × ref).

#198 already moved the main consumers' LLBC handoff from actions/cache to upload/download-artifact (run-scoped, not evictable). This PR cuts the root footprint and finishes that migration.

Changes

  1. rust-cache: save only on main. save-if: ${{ github.ref == 'refs/heads/main' }} on all five Swatinem steps (prepare, cargo-test, pyre-check, cpython-tests, wasm-build). PR jobs become restore-only and pull main's default-branch cache via restore-keys, so each PR no longer persists its own ~500–600 MB target cache.

    • Cost: a PR's own incremental build state is not carried across its pushes (it recompiles its diff from main's baseline each run); deps and unchanged crates still come warm from main.
    • Hits work because GitHub lets any PR restore the default-branch cache; this is the pattern Swatinem documents for save-if.
  2. wasm-build: LLBC via artifact. The one consumer majit: resized ListRepr port, BUILD_LIST residualization, optheap setfield flush, and truth-value lowering fix #198's migration missed — it still restored LLBC with actions/cache and hard-failed on a miss. Switched to download-artifact and dropped .pyre-build/charon-src from its Charon restore path, matching the other jobs.

Effect

Removes the per-ref cache multiplication that pushed the repo past the budget, and makes the LLBC handoff eviction-proof across all consumers.

🤖 Generated with Claude Code

Add save-if: github.ref == 'refs/heads/main' to every Swatinem
rust-cache step (prepare, cargo-test, pyre-check, cpython-tests,
wasm-build). PR jobs now restore the cargo target cache from main's
default-branch entry via restore-keys but no longer persist their own.

Each PR previously saved a ~500-600 MB target cache per OS/job; summed
across the open PR refs this pushed the repository past GitHub's 10 GB
Actions cache budget, and the resulting LRU eviction was thrashing the
prepared LLBC/cargo entries. Restoring from main keeps PR builds warm
without each ref multiplying the footprint.

Assisted-by: Claude
The wasm-build job still restored the LLBC set with actions/cache and
hard-failed on a cache miss — the one consumer the earlier artifact
migration skipped. Switch it to download-artifact (run-scoped, exempt
from the 10 GB cache budget / LRU eviction) and drop .pyre-build/
charon-src from its Charon restore path, matching the other consumers.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@youknowone, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 5 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23999d83-078a-4007-80ca-8a344815fb91

📥 Commits

Reviewing files that changed from the base of the PR and between ae6b152 and b1ba8a8.

📒 Files selected for processing (1)
  • .github/workflows/pyre-ci.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci-rust-cache-save-on-main

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 and usage tips.

youknowone added a commit that referenced this pull request Jul 3, 2026
Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five
Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check,
cpython-tests, wasm-build). PR jobs become restore-only and pull main's
default-branch cache via restore-keys, so each PR no longer persists its
own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB
cache budget.

This is the still-applicable part of #209; that PR's other commit
(LLBC handoff to wasm-build via artifact) is already superseded on main,
where wasm-build restores LLBC through download-artifact.

Assisted-by: Claude
youknowone added a commit that referenced this pull request Jul 3, 2026
Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five
Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check,
cpython-tests, wasm-build). PR jobs become restore-only and pull main's
default-branch cache via restore-keys, so each PR no longer persists its
own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB
cache budget.

This is the still-applicable part of #209; that PR's other commit
(LLBC handoff to wasm-build via artifact) is already superseded on main,
where wasm-build restores LLBC through download-artifact.

Assisted-by: Claude
youknowone added a commit that referenced this pull request Jul 3, 2026
Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five
Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check,
cpython-tests, wasm-build). PR jobs become restore-only and pull main's
default-branch cache via restore-keys, so each PR no longer persists its
own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB
cache budget.

This is the still-applicable part of #209; that PR's other commit
(LLBC handoff to wasm-build via artifact) is already superseded on main,
where wasm-build restores LLBC through download-artifact.

Assisted-by: Claude
youknowone added a commit that referenced this pull request Jul 3, 2026
… + CALL_ASSEMBLER default-on) (#347)

* jit-trace: exclude bridge walks from terminate_no_replay store-commit

`terminate_no_replay` keeps a Terminate walk's eager list stores
(journal-commit) so the loop-free function portal can return the walk's
result without replaying it. A bridge Terminate walk does not go through
that portal: its caller resumes the guarded region through the blackhole,
which re-applies the stores. Keeping them here as well double-applied
them. Gate the predicate on `!is_bridge_trace` so bridge walks roll their
stores back before the blackhole replay.

Assisted-by: Claude

* builtins: route hasattr/getattr attribute-name check through checkattrname

hasattr/getattr validated the name inline with their own
"hasattr()/getattr(): attribute name must be string" messages; route both
through checkattrname (operation.py:41-45), which raises the unified
"attribute name must be string, not '<type>'" message setattr/delattr
already use.

Assisted-by: Claude

* optimizeopt/heap: treat runtime-minted mutable descrs as written by every non-elidable call

A descr minted at runtime (ei_index unset, u32::MAX) is outside the
compute_bitstrings universe, so no call's write bitstring can ever name
it; a bitcheck miss proves nothing. Treating the miss as "not written"
let optheap keep a getfield cache across a store residual and re-read a
stale value (module-global cell fold: 'acc = acc + a; acc = acc + b'
dropped the first term). Field and array descr loops in
force_from_effectinfo now conservatively invalidate mutable
out-of-universe descrs on every non-elidable call.

Assisted-by: Claude

* jit-trace: record word-ABI void helpers through a sized descr with caller-supplied effect

jit_store_name_to_namespace / jit_list_append are extern "C" helpers
whose C signature returns a dummy machine word (-> i64, value ignored)
but were recorded as plain void residuals. Add
make_call_descr_void_word_abi (result_type=Void, result_size=8, distinct
interning key) and TraceCtx::call_void_typed_word_abi so a
signature-exact backend lowering (wasm call_indirect) can select the
(i64xn) -> i64 type and drop the result. The new entry point takes a
caller-supplied EffectInfo — these helpers write the heap (namespace
cells, list storage), and the opcode-default empty write set let optheap
CSE a getfield across the call; both call sites record MOST_GENERAL
(graphanalyze.py:60 analyze_external_call for an unanalyzed external
writer). The call_*_typed record tail is factored into
record_call_with_descr (invalidate-before-record, pyjitpl.py:2683-2684).

Assisted-by: Claude

* codewriter: widen BhSizeSpec/BhDescr vtable to u64

The vtable field is an ob_type pointer captured in the producing process
(the opcode_descrs.bin build script) and crosses the build->runtime
serialization boundary. Declared usize, a 64-bit host pointer failed
bincode's width check when deserialized on a wasm32 runtime. Store it as
u64 and cast at the consumers; the value is an opaque identity word
either way (real vtables are re-resolved via type_id -> gc_cache).

Assisted-by: Claude

* metainterp: extend MC_DIAG guard-to-bridge tallies; gated fbw bridge walk prints

MC_DIAG grows 8 -> 18 slots: compile_bridge entry/InvalidLoop/
retrace_requested/arity-giveup (8-11), start_bridge_tracing entry and its
early-return reasons (12-17), and stack_almost_full()==true (5, formerly
reserved). Under the existing fbw debug-abort flag, the full-body walker
also prints a bridge-walk epilogue line (commit state + outcome kind) and
a note when a bridge STORE_SUBSCR specialization declines to the generic
residual.

Assisted-by: Claude

* backend-wasm: bridge chaining and CALL_ASSEMBLER fast paths, on by default

Bridge chaining (PYRE_WASM_ENABLE_BRIDGES=0 disables): compiled bridges
resolve guard exits in-module through a global fail-index space with
nested chaining and a REF_HOME_FLOOR-sized ref-home area; declined
bridges fall back to host round-trips. WASM_BRIDGES_ENABLED flips to
default-true and the dormant-tracer comment block is dropped.

CALL_ASSEMBLER (PYRE_WASM_CA=0 disables, now default-on):
- alloc/pop frame helpers lower through the residual (i64xn)->i64
  call_indirect type family instead of the jit_call trampoline
  (residual_max_arity forced >= 2 when the CA arm is emitted).
- CA callee frames are libc-allocated jitframes registered with the
  shadow stack and recycled through a LIFO pool, replacing per-call
  old-gen allocation that forced back-to-back major collections
  (gc_majors 1704 -> 0 on fib_recursive).
- Frames are sized and gcmapped to FRAME_REF_HOME_FLOOR when bridges are
  enabled, mirroring execute_token.
- CA on a loop that already has bridges (or further chaining on a loop
  whose CA arm ran) is declined (diag slot 14 decl_ca_chain): composing
  the two on one recursion produces wrong output on
  recursion_memo_branch/generator_tree_recursion (task #7).

Inline fast paths in generated code:
- write barrier: test the TRACK_YOUNG_PTRS header flag inline and only
  call_indirect the helper when set (genop_discard_cond_call_gc_wb).
- nursery allocation (PYRE_WASM_INLINE_ALLOC=0 disables): New/
  NewWithVtable and constant-length NewArray/NewArrayClear bump
  nursery_free inline (header word + array length store) and only call
  the collecting helper on overflow, gated on the new
  type_alloc_is_plain trait (no destructor, not a weakref), size below
  max_nursery_object_size, and gc_stress off.

Diagnostics: collection_counts on the GC trait (MiniMarkGC override),
pyre_gc_minor/major_collections + pyre_jit_set_inline_alloc guest
exports, gc_minors/gc_majors in the runner's [jit-stats] line, runner
labels for the new MC_DIAG slots.

fib_recursive (fib 0..34) 24s -> 2.9s, fannkuch8 13.8s -> 0.56s; wasm
suite 168/169 (remaining fail is the pre-existing nbody libm ULP drift).

Assisted-by: Claude

* check.py: allow bounded float divergence for the nbody wasm bench only

The wasm float tolerance is now opt-in per bench via
run_bench(..., wasm_float_tol=True) rather than applied to every wasm
bench. Only nbody is marked; all other wasm benches require
byte-identical output. The comparison gate becomes
`backend == "wasm" and wasm_float_tol`, threaded from run_bench through
_run_backend_bench. Corrects the wasm_outputs_match root-cause note: the
divergence is libm's arch-specific pow building blocks, not FMA fusion
(FMA hypothesis tested and refuted).

Assisted-by: Claude

* ci: save rust-cache on main only, restore-only on PRs

Adds save-if: ${{ github.ref == 'refs/heads/main' }} to all five
Swatinem/rust-cache steps (prepare-charon-llbc, cargo-test, pyre-check,
cpython-tests, wasm-build). PR jobs become restore-only and pull main's
default-branch cache via restore-keys, so each PR no longer persists its
own ~500-600 MB target cache and multiplies the repo past GitHub's 10 GB
cache budget.

This is the still-applicable part of #209; that PR's other commit
(LLBC handoff to wasm-build via artifact) is already superseded on main,
where wasm-build restores LLBC through download-artifact.

Assisted-by: Claude

* check.py: add wasm to default backends when its target is installed

DEFAULT_BACKENDS gains "wasm" whenever `rustup target list --installed`
reports wasm32-unknown-unknown, the sole extra prerequisite for building
the wasm backend (the wasmtime runtime is embedded in pyre-wasm-runner,
not an external tool). On a machine without that target the probe returns
false and the defaults stay dynasm,cranelift, so a bare check.py still
runs only the native backends there.

Assisted-by: Claude

* optimizeopt/heap: gate out-of-universe descr invalidation on compute_bitstrings_has_run

The runtime-minted-descr invalidation added in 6091829 fired on any
descr whose effect_idx is the u32::MAX sentinel. In the unit tests
compute_bitstrings is skipped, so every cached descr carries u32::MAX and
an unrelated CallMayForce write bit wrongly invalidated it, breaking
test_call_may_force_uses_effectinfo_to_keep_unaffected_cached_fields,
test_call_may_force_keeps_unaffected_variable_index_array_cache and
test_default_pipeline_escaping_call_arg_flush_is_selective.

Gate the clause on compute_bitstrings_has_run() in both the field and
array loops: after bitstrings run a u32::MAX effect_idx genuinely means an
out-of-universe runtime descr (invalidate conservatively); before that it
just means the fixture never stamped the slot, so the existing identity
fallback is the correct arbiter.

Assisted-by: Claude

* ci: install wasm32 target on the Linux pyre/check.py leg only

check.py now adds wasm to its default backends when wasm32-unknown-unknown
is installed. The three pyre/check.py jobs share one steps anchor, so a
step guarded by runner.os == 'Linux' installs the target on the ubuntu leg
only — it builds and runs the wasm backend there, while macOS/Windows keep
no wasm32 target and stay on dynasm/cranelift. wasm output is
platform-independent, so exercising it on one OS is enough.

Assisted-by: Claude
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