jit, _abc: carry the recorded QuasiImmut instance on the marker descr, and let a missing _abc collection raise

[youknowone]

Two independent changes on one branch.

quasiimmut: record the resolved QuasiImmut instance on the marker descr

record_quasiimmut_field now builds a per-read majit_ir::QuasiImmutDescr binding
the QuasiImmut instance resolved at record time together with the owner pointer,
the way QuasiImmutDescr.__init__ does upstream. The heap optimizer's
QuasiimmutField arm runs the full is_still_valid_for test against that descr —
owner pointer, instance identity, current field value — and answers InvalidLoop
when the marker carries no instance or the test fails. The dependency it records is
the instance itself rather than an (owner pointer, descr index) pair the compile
side re-resolves.

What made the pair necessary was an objection recorded in the tree — that pyre's
descrs are registry-indexed singletons, so a record-time value cannot ride on one.
That does not hold: record_op_with_descr only stores the Arc<dyn Descr> and pyre
traces are never serialized, so a per-read wrapper descr costs one Arc.

Dropping the pair does lose something, and this is where the InvalidLoop comes
from: re-resolution also covered the case where the record side cannot resolve the
instance but the optimizer can fold the owner to a constant, and it registered the
watcher late. Carrying the instance instead would leave that fold with no watcher
behind it, so the optimizer now refuses the loop rather than standing one — which is
how heap.py:814's assert reads as a verdict.

QuasiImmut becomes Arc-shared and carries an unlinked flag, so an instance
already swept answers is_current() == false and its register_loop_token sets the
flag immediately instead of storing a weak ref nothing will visit. That is what makes
the window safe here: upstream's GIL spans optimize→compile and pyre's does not.

QuasiImmutField::get_current_qmut_instance resolves-or-creates under the field lock
and returns that Arc, which collapses the twelve *_install_* /
*_register_*_watcher accessor pairs into one *_current_*_qmut each and reduces
pyre-jit's register_quasi_immutable_deps from an 11-arm re-resolution chain to
registering the artifact's invalidation flag on the recorded instances.

pyre-object gains no majit dependency: the handle is a majit-ir trait
(QuasiImmutHandle) with a newtype adapter in pyre-jit-trace, which already sees
both crates.

quasi_immutable_deps is Vec<Arc<dyn QuasiImmutHandle>> through optimizer, unroll
and pyjitpl; dedup is by Arc::ptr_eq.

Also repairs 51 citations in the touched regions that named quasiimmut.py and
heap.py lines the current sources no longer hold — quasiimmut.py:116-126 names a
different function; get_current_qmut_instance is at 17-27.

_abc: let a missing collection raise instead of reading as an empty one

A missing _abc_impl collection was read as empty. It now raises, as upstream does.

Gates

Run on the pre-rebase base e4fb0a308c6:

gate	result
cargo check --all --no-default-features --features dynasm	rc=0, no warnings
pyre/check.py --backend dynasm	445/445, no jitstats counter movement
parity (--dynasm-only)	all pass
cpython_tests	207 PASS / 3 FAIL
cargo test --all --no-default-features --features dynasm	rc=0, 158 test binaries ok

The three cpython_tests failures — test_robotparser, test_smtpnet, test_urllib2 —
all died on the same DNS error (gaierror (8, 'nodename nor servname provided, or not known')) and each passes on an isolated --filter re-run. Reporting the batch result
as it stood.

check.py at 445/445 with no counter movement is the evidence that neither the added
identity check nor the new InvalidLoop arm costs a loop. It also retires the one
open concern: QuasiImmutDescr.struct_ptr is a raw u64 the GC does not update, so
an owner that moved between record and optimize would mismatch and lose the loop —
safe, but a cliff. Nothing moved.

The branch was then rebased onto 01b740aedaf, picking up #1362, #1370 and #1380.
The rebase was clean and cargo check --all is rc=0 on the new base, but the gate
table above predates it — CI on this PR is the verdict for the current base.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MLkGH6Ee8dMtvQqFYU8k5Q

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 27 minutes

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1c3d7a81-b89a-4c76-8bec-b8f7faf6bdf8

📥 Commits

Reviewing files that changed from the base of the PR and between 01b740a and 23a0308.

📒 Files selected for processing (23)

• majit/majit-ir/src/descr.rs
• majit/majit-ir/src/lib.rs
• majit/majit-metainterp/src/optimizeopt/heap.rs
• majit/majit-metainterp/src/optimizeopt/mod.rs
• majit/majit-metainterp/src/optimizeopt/optimizer.rs
• majit/majit-metainterp/src/optimizeopt/unroll.rs
• majit/majit-metainterp/src/pyjitpl.rs
• pyre/pyre-interpreter/src/function.rs
• pyre/pyre-interpreter/src/jit_fnaddr.rs
• pyre/pyre-interpreter/src/module/_abc/mod.rs
• pyre/pyre-interpreter/src/module/sys/vm.rs
• pyre/pyre-interpreter/src/objspace/std/mapdict.rs
• pyre/pyre-jit-trace/src/descr.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
• pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
• pyre/pyre-jit-trace/src/state.rs
• pyre/pyre-jit/src/eval.rs
• pyre/pyre-object/src/celldict.rs
• pyre/pyre-object/src/descriptor.rs
• pyre/pyre-object/src/dictmultiobject.rs
• pyre/pyre-object/src/function.rs
• pyre/pyre-object/src/quasiimmut.rs
• pyre/pyre-object/src/typeobject.rs

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/youknowone/pyre/blob/23a03083de40e8a86a31d1f977425bf69a902b32/pyre-jit-trace/src/state.rs#L5215-L5218
Deduplicate by the underlying quasi-immutable instance

When one trace records the same ? field again after a heap-cache invalidation, each call wraps the same qmut in a new Arc<RecordedQuasiImmut>. The optimizer and unroller deduplicate with Arc::ptr_eq on this outer allocation, so these logically identical dependencies never collapse: compilation registers the same artifact flag repeatedly, and the weak entries remain live until invalidation, making storage and invalidation work proportional to repeated reads rather than unique fields. Preserve a stable handle or compare the underlying QuasiImmut identity instead.

AGENTS.md reference: AGENTS.md:L81-L88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 23a0308).
Updated: 2026-08-20T16:36:42.409Z

Files in the reviewed diff

majit/majit-ir/src/descr.rs
majit/majit-ir/src/lib.rs
majit/majit-metainterp/src/optimizeopt/heap.rs
majit/majit-metainterp/src/optimizeopt/mod.rs
majit/majit-metainterp/src/optimizeopt/optimizer.rs
majit/majit-metainterp/src/optimizeopt/unroll.rs
majit/majit-metainterp/src/pyjitpl.rs
pyre/pyre-interpreter/src/function.rs
pyre/pyre-interpreter/src/jit_fnaddr.rs
pyre/pyre-interpreter/src/module/_abc/mod.rs
pyre/pyre-interpreter/src/module/sys/vm.rs
pyre/pyre-interpreter/src/objspace/std/mapdict.rs
pyre/pyre-jit-trace/src/descr.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
pyre/pyre-jit-trace/src/state.rs
pyre/pyre-jit/src/eval.rs
pyre/pyre-object/src/celldict.rs
pyre/pyre-object/src/descriptor.rs
pyre/pyre-object/src/dictmultiobject.rs
pyre/pyre-object/src/function.rs
pyre/pyre-object/src/quasiimmut.rs
pyre/pyre-object/src/typeobject.rs

1. Regressions to PyPy parity introduced by this patch

• majit/majit-metainterp/src/optimizeopt/optimizer.rs:1741 ↔ rpython/jit/metainterp/optimizeopt/heap.py:821 — dependency dedup compares the separately allocated Arc<RecordedQuasiImmut> wrappers (Arc::ptr_eq), not the underlying QuasiImmut. pyre/pyre-jit-trace/src/state.rs:5215 creates one wrapper per marker, so repeated markers for one upstream qmut register the same loop repeatedly. PyPy’s dict[qmutdescr.qmut] = None retains exactly one entry. Main’s (object_ptr, field_index) key did deduplicate this case.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

None.

4. Structural adaptations

• pyre/pyre-object/src/quasiimmut.rs:98 ↔ rpython/jit/metainterp/quasiimmut.py:72 — Rust uses Arc, atomics, and a mutex to keep record/compile/invalidate safe across free-threaded execution; PyPy relies on its GIL-protected object lifecycle. This preserves the same invalidation contract.
• pyre/pyre-jit-trace/src/state.rs:5087 ↔ rpython/jit/metainterp/quasiimmut.py:119 — PyPy stores constantfieldbox on QuasiImmutDescr; Pyre carries it as the second QUASIIMMUT_FIELD operand because Rust descriptors are shared trait objects rather than per-op Python objects.