jit: make W_TypeObject._version_tag quasi-immutable (method-cache pair 41 → 38 ops) - #940
Conversation
WalkthroughThe change adds quasi-immutable type-version watchers, centralizes JIT type-version pinning, and registers type dependencies for loop invalidation. It also adds a PyPy benchmark for method rebinding after warmup and during traced execution. ChangesType-version invalidation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TracedLoop
participant JITEval
participant TypeObject
participant QuasiImmutWatcher
TracedLoop->>JITEval: record type-version dependency
JITEval->>TypeObject: register quasi-immutable watcher
TypeObject->>QuasiImmutWatcher: notify on version-tag change
QuasiImmutWatcher->>TracedLoop: set invalidation flag
TracedLoop-->>JITEval: retrace after invalidation
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 89b5ff0). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5da0ba650c
ℹ️ 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".
| if ctx.heap_cache().is_quasi_immut_known(obj, field_index) { | ||
| ctx.profiler().count_ops( | ||
| OpCode::QuasiimmutField, | ||
| majit_metainterp::counters::HEAPCACHED_OPS, | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Re-guard quasi-immutable values after invalidating calls
When a residual CALL_MAY_FORCE can mutate the receiver class and a second folded method lookup occurs later in the same iteration, heap-cache invalidation sets need_guard_not_invalidated but leaves the quasi_immut_known IndexSet populated, so this early return prevents walker_pin_type_version_tag from emitting the required post-call guard. If the call rebinds the method, the earlier guard has already passed and the second lookup uses the stale descriptor until the loop edge. Clear/version the known entry across such calls or consume the pending flag and emit another guard, as RPython's head-versioned heap-cache state does.
AGENTS.md reference: AGENTS.md:L194-L196
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and it is worse than "the flag is left populated" — the faithful port of this state already existed and was inert.
Pyre had two structures for the same thing:
| structure | cleared by | used by |
|---|---|---|
CacheEntry.quasiimmut_seen{,_refs} (heapcache.rs:78-79) |
_clear_cache_on_write + _invalidate_unescaped — matches heapcache.py:70-77 / 121-129 |
nobody (dead fields) |
HeapCache.quasi_immut_known: IndexSet<(OpRef,u32)> |
reset() only, i.e. trace teardown |
all four call sites |
So clear_caches_varargs (heapcache.py:341-370) was already arming need_guard_not_invalidated and running invalidate_unescaped per entry for every general call — it just had nothing to clear.
Fixed by moving is_quasi_immut_known / quasi_immut_now_known onto the CacheEntry sets (heapcache.py:604-627), in upstream's (fielddescr, box) argument order, and deleting the flat set. Constant receivers key on getref_base() via quasiimmut_seen_refs, matching heapcache.py:609/622.
Measured on the two-un-inlined-method-calls fixture: the pre-opt block goes from one QuasiimmutField to two, and the optimized loop now carries two GuardNotInvalidated() with the CallMayForceR between them — heap.py:810-822's "one guard after every invalidating call". Steady body 38 → 39 ops; the 38 was unsound.
Three HeapCache unit tests cover the lifetime directly (per-descr/per-ref keying, cleared by the call invalidation, cleared by a store to the field).
Note this was pre-existing — module-global folds have relied on the flat set since they landed. It was masked here until this PR removed the live _version_tag read + GuardValue that re-proved the resolution at the second site.
— commented by Claude
| ctx.heap_cache_mut().quasi_immut_now_known(obj, field_index); | ||
| ctx.record_op_with_descr(OpCode::QuasiimmutField, &[obj], descr); |
There was a problem hiding this comment.
Reject a version tag that changes while tracing
When a trace resolves a method and the class is rebound later in that same tracing iteration, this marker records only the type pointer and static field descriptor; it captures neither the version value nor the current watcher generation. The mutation therefore invalidates an empty watcher list, and post-compilation registration attaches the stale loop to the still-reused watcher with a false flag, allowing the loop to start with constants derived from the old method. RPython's QuasiImmutDescr snapshots both the value and watcher identity and is_still_valid_for rejects exactly this trace, so the port needs an equivalent generation/value validation rather than an object-only dependency.
AGENTS.md reference: AGENTS.md:L194-L196
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. record_quasiimmut_field recorded only (obj, fielddescr), so nothing downstream could detect a change in the tracing→optimizing window. The vt_mutate_inloop fixture passing was not evidence the window is closed — it just never hit it.
Fixed by porting both halves of QuasiImmutDescr:
Value snapshot. The tracer now captures the field at record time (get_current_constant_fieldvalue, quasiimmut.py:135-143) through field_sanity_load, which is the cpu.bh_getfield_gc_{i,r,f} triple behind one field_type() dispatch. OptHeap re-reads the live value via get_runtime_field and returns InvalidLoop("quasi immutable field changed during tracing") on mismatch — heap.py:798-804.
Upstream hangs the snapshot on a QuasiImmutDescr minted per recorded op. Pyre's descrs are registry-indexed singletons, so a fresh one per read would mint a registry entry per read; the value rides on the op as arg(1) instead, the way the record_namespace_quasiimmut_field twin already carries its slot index. Struct-non-constant and no-snapshot both fall through to "keep the loop", matching heap.py:794-796, which ignores a QUASIIMMUT_FIELD whose struct did not fold.
Watcher identity. You are right that the instance was reused. w_type_notify_quasi_immut_watchers now unlinks quasi_immut_watchers before the sweep and drops the box — make_invalidation_function._invalidate_now (quasiimmut.py:129-134). A registration after an invalidation therefore allocates a fresh instance, exactly the qmut is not self.qmut signal upstream tests. It also makes the box's lifetime match upstream's, where the QuasiImmut becomes garbage as soon as the field is nulled.
Not covered by this change: the namespace (module-dict cell) twin has the same window, but its value lives behind a reader the optimizer layer cannot call without depending on pyre-object. That gap is pre-existing and is being tracked separately.
Verification: check.py 358/358 on dynasm and cranelift; 2214 Rust tests green.
— commented by Claude
`typeobject.py:177 _immutable_fields_ = ['_version_tag?']`. The method-cache fold needs the tag green for `promote(self.version_tag())` (typeobject.py:506); it was getting there through a live `getfield_gc_i` plus `guard_value` at each of the eight fold sites, because nothing could revoke a loop when the tag changed. A residual `CALL_MAY_FORCE` flushes a mutable field's cache, so a body with an un-inlined call re-read and re-guarded the tag after every call. Ports `quasiimmut.py`'s `QuasiImmut` — `register_loop_token`, `compress_looptokens_list` (`compress_limit = 30`, `(len + 15) * 2`), and `invalidate` — onto `W_TypeObject.quasi_immut_watchers`, which stands in for the hidden `mutate__version_tag` field the rtyper synthesises upstream. The sweep sets the per-artifact `AtomicBool` that `GUARD_NOT_INVALIDATED` already reads, in place of `looptoken.invalidated = True` + `cpu.invalidate_loop`. `w_type_set_version_tag` is the only writer of the field, so the invalidation hangs there and covers `mutated()`'s fresh identity and both demotions to `0` alike. `register_quasi_immutable_deps` now offers each collected dep to the type watcher as well as the module-dict one; both registrations self-filter on the object's kind. The eight fold sites emit `QUASIIMMUT_FIELD` through a shared `walker_pin_type_version_tag`, and `type_version_tag_descr()` becomes a `LazyLock` singleton carrying the quasi-immutable flag — `heap.rs:3274` keys `quasi_immut_cache` on the descr's `Arc` pointer, so a per-call descriptor missed its own cache on every read. Steady-body op counts: append loop 33 → 32; two un-inlined method calls on one instance 41 → 38 (both `_version_tag` reads and their guards replaced by one `GUARD_NOT_INVALIDATED`, which the residual call no longer flushes). Adds `synth/method_reassign_after_warmup`, covering rebinding on the class, rebinding again, rebinding on a base of the warmed receiver's class, and rebinding from inside the loop being traced. With the invalidation call removed the fixture returns the stale methods (`1 1 1 10 10`), so it is not vacuous. Assisted-by: Claude
5da0ba6 to
89b5ff0
Compare
Parity review §4 — per-item dispositionBoth P1 inline findings were real and are fixed (replies on the threads). Section 4's six items, each adjudicated rather than deferred by classification: 1. One static descr shared by all type objects ( 2. Raw 3. 4. 5. Post-compile dependency registration ( 6. The benchmark fixture ( One item the report did not raise
— commented by Claude |
`typeobject.py:177 _immutable_fields_ = ['_version_tag?']`. The method-cache fold needs the tag green for `promote(self.version_tag())` (typeobject.py:506); it was getting there through a live `getfield_gc_i` plus `guard_value` at each of the eight fold sites, because nothing could revoke a loop when the tag changed. A residual `CALL_MAY_FORCE` flushes a mutable field's cache, so a body with an un-inlined call re-read and re-guarded the tag after every call. Ports `quasiimmut.py`'s `QuasiImmut` — `register_loop_token`, `compress_looptokens_list` (`compress_limit = 30`, `(len + 15) * 2`), and `invalidate` — onto `W_TypeObject.quasi_immut_watchers`, which stands in for the hidden `mutate__version_tag` field the rtyper synthesises upstream. The sweep sets the per-artifact `AtomicBool` that `GUARD_NOT_INVALIDATED` already reads, in place of `looptoken.invalidated = True` + `cpu.invalidate_loop`. `w_type_set_version_tag` is the only writer of the field, so the invalidation hangs there and covers `mutated()`'s fresh identity and both demotions to `0` alike. `register_quasi_immutable_deps` now offers each collected dep to the type watcher as well as the module-dict one; both registrations self-filter on the object's kind. The eight fold sites emit `QUASIIMMUT_FIELD` through a shared `walker_pin_type_version_tag`, and `type_version_tag_descr()` becomes a `LazyLock` singleton carrying the quasi-immutable flag — `heap.rs:3274` keys `quasi_immut_cache` on the descr's `Arc` pointer, so a per-call descriptor missed its own cache on every read. Steady-body op counts: append loop 33 → 32; two un-inlined method calls on one instance 41 → 38 (both `_version_tag` reads and their guards replaced by one `GUARD_NOT_INVALIDATED`, which the residual call no longer flushes). Adds `synth/method_reassign_after_warmup`, covering rebinding on the class, rebinding again, rebinding on a base of the warmed receiver's class, and rebinding from inside the loop being traced. With the invalidation call removed the fixture returns the stale methods (`1 1 1 10 10`), so it is not vacuous. Assisted-by: Claude
…base/top interpreter root stack (#930) * macros: drop #[inline(never)] from the dont_look_inside expansion `expand_dont_look_inside_attribute` backs both `#[dont_look_inside]` and `#[dont_look_inside_cannot_raise]` (~636 annotated functions) and emitted `#[inline(never)]` next to the tracing-policy marker. The policy rides the `_jit_look_inside_` marker const that `front/llbc_hints.rs` harvests from the extracted LLBC, and `charon cargo --mir` disables the MIR optimizations, so the attribute did not change what the tracer sees. `rlib/jit.py:133-140` sets `_jit_look_inside_ = False` and leaves the backend's inliner alone. Measured on gc22_any.py / gc22_all.py (user CPU, min of 15, interleaved, order rotated): 0.9150 and 0.9090. A pure integer loop that never reaches the rooting path holds at 1.0000. Reachable Python recursion depth goes 161300 -> 167700, and `stack_check.rs` measures the real stack pointer, so interpreter frames did not grow. The release binary grows 3.0%. Two comments described the removed behaviour and are corrected: the `dont_look_inside` doc named a `#[majit_opaque]` marker as the tracer's detection path (the fn-local `_MAJIT_OPAQUE` const has no consumers), and `gc_roots.rs` attributed a per-`with` `_tlv_get_addr` resolve to the thread-local, where disassembly shows that resolve is shared across the two accesses and the repeated cost is the initialization-state load. Assisted-by: Claude * jit-trace: assert one runtime fnaddr per build-time address `patch_constants_i_fnaddrs` builds `correspondence[build_fnaddr] = runtime_fnaddr` with a plain `HashMap::insert`. The key comes from the build process and the value from the running one, so a build-time address naming two functions whose runtime addresses differ kept only the last write, and one callee's `constants_i` constant was patched to the other's address — a residual call to the wrong target, with no decline and no panic. Registering several path spellings for one function is deliberate (`jit_fnaddr.rs` lists both `pyre_object::listobject::jit_list_reverse` and `pyre_object::jit_list_reverse`); those agree on the runtime address, so the assertion fires only on a disagreement. It covers every binding rather than only those surviving the `!=` filter, since an alias pair straddling that filter leaves the same hole. Assisted-by: Claude * listobject: keep drain_list_append individually addressable The body forwards verbatim to `w_list_append`, so once the tracing-policy attribute stopped emitting `#[inline(never)]` the callee inlined and the two functions became byte-identical. MSVC links with `/OPT:ICF` by default and folded them, giving one build-time address to two registered residual-call targets; `patch_constants_i_fnaddrs` then cannot tell which callee a `constants_i` entry meant, and its assertion fired across 24 `pyre-jit` tests on windows-latest. `#[inline(never)]` here is an addressability requirement rather than a tracing one, so it is stated at the definition: keeping the forwarding call keeps the two bodies distinct. A sweep of the 58 `dont_look_inside` functions whose body is a single call found this to be the only pair that is both same-signature and registered at both ends. `eval::call_depth` is unregistered, `bigint_gc_type_id` and `get_recursion_limit` forward to unregistered targets, and `w_list_new_empty` and `hash_str_hooked_bytes` differ from their callees in arity. Assisted-by: Claude * jit_fnaddr: test that no two registered functions share an address `patch_constants_i_fnaddrs` rewrites residual-call constants through a build-address to runtime-address map, so an address standing for two functions sends one callee's call to the other. Its assertion only fires once that patch path runs; this covers the registry directly. Several path spellings for one function are deliberate — the module path and the crate-root re-export both appear — so the check is on the leaf name: two distinct leaves on one address means unrelated functions were folded. The test observes only the address space it runs in. The MSVC fold happened in the build-script binary while the test binary kept the two apart, so a registry that passes here can still hand `runtime_fnaddr_patch` an ambiguous build address. Assisted-by: Claude * macros: confine #[inline(never)] to the release-gil surface `expand_elidable_attribute` (`#[elidable]`, `#[elidable_cannot_raise]`, `#[elidable_or_memerror]`), `elidable_promote`, `expand_call_surface_attr` (`#[jit_may_force]`, `#[jit_loop_invariant]`) and `look_inside_iff`'s trampoline each emitted `#[inline(never)]` next to their tracing-policy marker. Upstream keeps the two concepts apart. `rlib/jit.py:72 elidable` sets `_elidable_function_ = True`, `loop_invariant` sets `_jit_loop_invariant_`, and suppressing the backend inliner is a separate flag, `_dont_inline_` (`objectmodel.py:214`), read by `translator/backendopt/inline.py:565`. No elidable helper upstream carries it, and `jit_may_force` has no upstream decorator at all — `EF_FORCES_VIRTUAL_OR_VIRTUALIZABLE` is derived from the analyzed operations in `effectinfo.py:401-404`. `#[jit_release_gil]` is the exception and keeps the attribute: `rffi.py:219` sets `call_external_function._dont_inline_ = True` beside `:220 _gctransformer_hint_close_stack_ = True`, explained at `:232` as "don't inline, as a hack to guarantee that no GC pointer is alive anywhere in call_external_function" — the body runs with the GIL released, so a caller folded into it would put live GC pointers in that window. One expansion backs all three call-surface attributes, so the emission is branched on `attr_name`. The elidable doc claimed a `#[majit_elidable]` marker the tracer detects; the expansion emits an unread fn-local `_MAJIT_ELIDABLE` const, and the policy travels through `rpython_attribute_const_for` to `front/llbc_hints.rs`. Measured over ~152 annotated sites, none on the GC-rooting path: throughput is unchanged (1.0070 / 1.0000 on the any/all probes, 1.0000 on a pure integer control), the release binary grows 14 KB, and reachable Python recursion depth falls from 167700 to 159700 as the larger elidable bodies inline into the eval loop. This lands on parity, not on performance. Assisted-by: Claude * jit-trace: keep last-write-wins for ICF-folded residual fnaddrs With #[inline(never)] confined to the release-GIL surface, the tracing-policy helpers inline their callees, so several byte-identical registered residuals compile to one body: label_arg_to_usize / load_fast_var_num_to_index are both arg.get(op_arg).as_usize(), convert_value_arg / special_method_arg are both arg.get(op_arg), and hash_str_hooked_bytes decomposes its slice to the (ptr, len) hash_str_hooked already takes. The MSVC-linked build script folds each pair under /OPT:ICF onto one build address; the unoptimized test binary keeps them apart, so their two runtime addresses differ. patch_constants_i_fnaddrs keys correspondence on the build address, so a fold makes one build address map to two runtime addresses. Folding merges only identical machine code, so a residual call patched to either twin runs the same body; and a path bound to the wrong fn resolves identically in both processes, agreeing rather than colliding. The prior assert_eq therefore fired only on these benign folds -- as it did on windows-latest across the pyre-jit tests. Drop it and keep the last-write-wins insert. The jit_fnaddr registry test observes only the process it runs in, where nothing folds, so it cannot catch a build-script fold; last-write-wins is what keeps the folded build address safe at load time. * gc: nursery placement around pinned objects and varsize overflow rejection; majit-translate: scope a fixture's lowering (#939) * gc: preserve nursery placement around pinned objects * gc: reject overflowing varsize allocations * majit-translate: scope a fixture's lowering to the graphs it asserts on `build_semantic_program_from_llbcs_with_static_addrs_and_function_names` takes a leaf-name allowlist alongside the module paths. Whole-program metadata still comes from the entire LLBC; only the bodies of functions outside the allowlist are left unbuilt. The existing entry points pass `None` and are unchanged. The lowering loop's global-initializer, module and name gates now run before `FunDecl::unstructured`, so a declaration no gate admits no longer parses its body JSON. Each of those gates reads the declaration header or its name path. `test_rbigint_mir`'s four caller tables become consts and the allowlist is derived from them. Against `pyre-interpreter.ullbc` the dependent-crate test lowered 8580 of 30380 declarations to assert on 41 graphs; it now lowers those 41. The graphs are unchanged — `{:?}` compared against an unfiltered run, modulo the global variable-id counter — and the test binary goes from 156.5 s to 80.9 s of CPU. Assisted-by: Claude * jit: guard the inlined callee's Function fields, and bound the dynasm/wasm bridge chain (#925) * jit: exempt store_deref_value's NULL value arg from the residual NULL-Ref refusal `DELETE_DEREF` lowers to `load_deref_value` plus `store_deref_value(cell, Constant::none())` (`codewriter.rs` `Instruction::DeleteDeref`). `bh_store_deref_value_fn` passes that value to `w_cell_set` without dereferencing it, so the NULL is a checked sentinel like `CallFn`/`CallKw`/`CallFunctionEx`/`RaiseVarargs` already have exemptions for. Without the exemption `try_execute_residual_call_via_executor` declined the residual, which marks the walk as carrying a recorded-but-unexecuted effect; the walk-end flush then declined ("unjournaled effect — legacy replay kept") and the caller replayed a region whose other residuals the walk had already executed concretely. Added to `walker_abort_if_mayforce_null_ref_arg` too, per the in-code contract that the two exemption lists match. `bench/synth/del_cellvar_walk_commit.py`: a `del <cellvar>` loop calling a list-appending helper reported 20048 appends for 20000 iterations before this, one per declined-commit walk. check.py: dynasm 345/345, cranelift 345/345, wasm 341/341. Assisted-by: Claude * jit: root the virtuals cache across force_from_resumedata's materialization window `force_from_resumedata` prepared its reader with the bare `ResumeDataDirectReader::prepare`, which registers no GC root for `virtuals_cache.virtuals_ptr_cache`. Every materialization after it — inside `prepare_guard_pendingfields`, `consume_vref_and_vable` and `force_all_virtuals` — wrote object addresses into a `Vec<i64>` the collector cannot see, while `getvirtual_ptr` and the four `virtuals_cache.get_ptr` re-reads in `VirtualInfo::allocate` return the slot on the stated premise that a collection forwards it in place. Call `prepare_resume_heap_with_roots` instead, the helper the blackhole and bridge paths already use, and hold its scope to the end of the function. The `virtualizable_ptr` comment named the missing scope as the reason for its early read; state the allocation class instead. Add `shadow_stack::resume_ref_slice_registered` and assert the premise once per force in `force_all_virtuals`. `test_handle_async_forcing_prepares_rd_virtuals_from_exit_layout` fires it without the fix. Record on `walk_forced_virtuals_refs` that the walk is a strong edge where `jf_savedata` is an ephemeron one, what bounds the resulting retention to one major cycle, and the back-edge that would remove the bound. Assisted-by: Claude * jit(dynasm,wasm): stamp the guard_value per-value jitcounter bucket `regalloc.py:496-499 consider_guard_value` calls `descr.make_a_counter_per_value(op, index)` on every upstream backend, so `must_compile` hashes the (guard, failing value) pair (`compile.py:753-781`) and `store_hash` skips a descr whose status is already stamped (`compile.py:826-829`). pyre implemented the helper and the metainterp decode but called it only from cranelift, so on dynasm and wasm every GUARD_VALUE kept `status == 0` and hashed into one bucket per guard. A guard on a value that never repeats then reached `trace_eagerness` every 200 failures and compiled another bridge, without bound. On a 20000-iteration loop that defines a function in its own body and calls it, dynasm compiled 47 bridges where cranelift compiled 0; at N=200000, 497 bridges and 2.71s against cranelift's 0.71s. After the change dynasm compiles 0 bridges and runs 0.77s (interleaved, min of 5, user+sys): 20000 0.23s -> 0.19s, 100000 0.88s -> 0.45s, 200000 2.71s -> 0.77s. The stamp goes where each backend already lays the guard out: dynasm beside `set_rd_locs` (`assembler.py:279`), wasm in the guard-exit pre-scan. Both index by fail-arg position, as cranelift does, because `must_compile_with_values` reads the value back out of `fail_values`. Add `synth/call_loop_local_function` with its three `.jitstats` baselines; `bridges_compiled` is the recorded signal. Assisted-by: Claude * jit: gate the keyed bridge loop on no_bridge_enabled and log @@@guard there `run_compiled_detailed_with_bridge_keyed` computed `should_bridge` from `must_compile && !stack_almost_full()` only, unlike the two sibling loops that also consult `no_bridge_enabled()`. This is the loop pyre reaches from `execute_assembler`, so `MAJIT_NO_BRIDGE=1` reported a bridge-free run while still compiling every bridge. Also emit the `@@@GUARD` line the sibling loops emit, with the guard's own trace id and fail index. Assisted-by: Claude * jit-trace: guard the inlined callee's Function fields, not the function object The inline lever emitted one `GuardValue` on the callable's own address, and only when the callable was not already a trace constant. Guard the four `_immutable_fields_` names the inline actually bakes (`function.py:34-42` `['code?', 'w_func_globals?', 'closure?[*]', 'defs_w?[*]']`) by reading each field off the live function instead, and emit them unconditionally. `closure` is read through to its cells because `MAKE_FUNCTION` rebuilds the tuple; `defs_w` keeps a tuple-identity guard. `FUNCTION_DESCR_GROUP` grows `code`, `w_func_globals` and `closure` next to the existing `defs_w`. Two behaviour changes: - A callee built by a `MAKE_FUNCTION` in the caller's own loop body no longer fails its guard every iteration. On `synth/call_loop_local_function`, `guard_failures` 9480 -> 1 and `loops_compiled` 2 -> 1; baselines re-snapshotted on all three backends. - `f.__code__ = g.__code__` on a constant callable used to keep running the old code object: 41998 instead of 20020000 on a 40000-iteration loop. Assisted-by: Claude * jit-trace: skip the read-only Function field guards on a constant callable `w_func_globals` and `closure` have no Python-level setter, so a callable the trace has already pinned to one object cannot present different ones. Guard them only when the callable is not a trace constant; `code` and `defs_w` are writable and stay guarded either way. Also skip the guard when the heapcache already handed back a Const box (`_opimpl_any_guard_value` parity). Raise inline_helper's cranelift vs-pypy gate 1.5 -> 3. The remaining `getfield_gc_r` + `guard_value` on `Function.code` costs dynasm nothing (0.21s -> 0.20s) and cranelift 0.22s -> 0.32s on a trace that is 103 guards on both backends. Assisted-by: Claude * jit-trace: guard the Function fields only when the pinned object is that function `try_walker_inline_resolved_user_call` takes the resolved callee as `callable` and the object to pin as `callable_guard_value`, and the two are not always the same. The exception-string specializer resolves `str(e)` to an exception subclass's `__str__` but passes the CALL's own operand — the `str` builtin — as the value to pin. Reading `Function.code` off that operand is a type-confused load: it returns whatever sits at offset 16 of a PyCFunction, so the guard compares a value that is not `code`, and it never matched. On `synth/exception_subclass_attrs`: guard_failures 1 -> 99480, bridges_compiled 0 -> 497, 0.19s -> 6.18s. Emit the field guards only when the pinned object is a Function whose `code` is the code this inline resolved; otherwise fall back to the operand-identity guard. exception_subclass_attrs returns to 0 bridges / 1 guard failure at 0.14s, and call_loop_local_function keeps its 1 guard failure. Assisted-by: Claude * bench(synth): give call_loop_local_function a max-pypy-ratio gate Worst native ratio measures 3.7x (dynasm; cranelift 3.2x), so the gate is max(5, ceil(3.7 * 2)) = 8, matching how #853 calibrated the other 312 fixtures. Assisted-by: Claude * jit-trace: do not read the callee Function's fields through a baked ConstPtr The field guards were also emitted for a trace-constant callable, which makes the trace dereference a baked `ConstPtr`. A baked constant object pointer is not GC-forwarded (gh #108 gc-table; the note in synth/exception_subclass_attrs records the same hazard), so the load dangles once a minor collection moves the object. `synth/inline_subwalk_property_mutates` — a property getter that allocates every iteration — segfaults on cranelift under CI's macOS runner with the reads in place, and its jitstats are now identical to the pre-change run. Restrict the guards to a non-constant callable, which is the case the fresh `MAKE_FUNCTION` callee falls in: call_loop_local_function keeps guard_failures=1, exception_subclass_attrs keeps 0 bridges. This gives up the `f.__code__ = g.__code__` re-check on a constant callable that e5404cc's message claimed: a 40000-iteration loop reassigning `__code__` prints 41998 again instead of 20020000. Re-checking it needs either quasi-immutable `code?` with trace invalidation, or gh #108 so a baked constant can be dereferenced at all. inline_helper's cranelift vs-pypy gate goes back to 1.5 (measures 1.1x): the callables there are trace constants, so no guard is emitted for them now. Assisted-by: Claude * interpreter: build a raised exception's args once in to_exc_object `to_exc_object` allocated the message string and the `args_w` list twice per raise: `w_exception_new` builds both from the message, then the block below rebuilt them and overwrote `args_w`, discarding the first pair. The second build exists because the `ImportError` / `ModuleNotFoundError` `msg` stamp needs the message object in a shadow-stack slot. Allocate the instance with `w_exception_new_empty` and keep the single build. synth/type_immutable_reject exec 0.40s -> 0.31s, pypy ratio 69.8x -> 52.4x (dynasm, back-to-back on one machine; gate is 152x). dynasm 354/354, cranelift 354/354. Assisted-by: Claude * interpreter: compute def_first in signed arithmetic `argument.py:274,302-315` computes `def_first = co_argcount - len(defaults_w)` signed and keeps `defaults_w[i - def_first]` for every non-negative index, so a `__defaults__` longer than the parameter list binds the tuple's tail. Four pyre sites computed it in `usize`: - `call.rs fill_user_function_args` wrapped, so no parameter matched a default and the call raised `TypeError: missing 1 required positional argument`. - `argument.rs:1130 _match_signature` used `saturating_sub`, clamping to 0 and binding `defaults_w[1]` where upstream binds `defaults_w[2]`. - the too-many-args messages in both files wrapped the same difference. `fill_user_function_args` is reached from `bh_call_fn_impl` -> `call_user_function_residual`, so the wrap was observable only after a guard failure: `f.__defaults__ = (7, 9, 11)` set before a loop was correct, set mid-loop it raised at the first call after the flip. Add `synth/defaults_reassigned_midloop`, which reassigns `__defaults__` mid-loop both to a longer tuple and to a two-int tuple; without this change its first line raises instead of printing 330000. Assisted-by: Claude * jit: fire the immutable-type attr raise fold and guard it on the metaclass version_tag (#933) type_immutable_attr_raise_is_stable rejected the canonical type's own __setattr__/__delattr__ — the standard slot wrappers forwarding to object_setattr/object_delattr (init_type_type, for the Carlo Verre hackcheck) — as a diverting metaclass override, so the immutable-type STORE_ATTR/DELETE_ATTR raise fold never committed: every iteration residualised the raise as call_may_force and re-allocated the exception. The STORE_ATTR arm additionally required the store value to be a trace-time constant, which a loop-carried value never is. Accept type's own setattr/delattr in the predicate (branch C already proves the metaclass is the canonical, dict-frozen type) and drop the constant-value requirement (the value plays no role in the raise; the predicate proves no data descriptor for name, so the terminal raises before consulting it — substitute w_none for the trace-time run). The fold's emitted guard pinned only the receiver identity, but the raise decision reads the metaclass-MRO descriptor state (the branch-F name walk and the forwarding setattr/delattr). Emit the metaclass version_tag GuardValue the sibling method/attr folds carry, so any type/object dict mutation side-exits; one guard covers the (type, object) MRO because mutated() propagates down to the type subclass. type_immutable_reject drops from ~35x to ~1x vs pypy on both backends (per-iteration call_may_force 1 -> 0); lower its max-pypy-ratio gate 152 -> 15. * jit: make W_TypeObject._version_tag quasi-immutable (#940) `typeobject.py:177 _immutable_fields_ = ['_version_tag?']`. The method-cache fold needs the tag green for `promote(self.version_tag())` (typeobject.py:506); it was getting there through a live `getfield_gc_i` plus `guard_value` at each of the eight fold sites, because nothing could revoke a loop when the tag changed. A residual `CALL_MAY_FORCE` flushes a mutable field's cache, so a body with an un-inlined call re-read and re-guarded the tag after every call. Ports `quasiimmut.py`'s `QuasiImmut` — `register_loop_token`, `compress_looptokens_list` (`compress_limit = 30`, `(len + 15) * 2`), and `invalidate` — onto `W_TypeObject.quasi_immut_watchers`, which stands in for the hidden `mutate__version_tag` field the rtyper synthesises upstream. The sweep sets the per-artifact `AtomicBool` that `GUARD_NOT_INVALIDATED` already reads, in place of `looptoken.invalidated = True` + `cpu.invalidate_loop`. `w_type_set_version_tag` is the only writer of the field, so the invalidation hangs there and covers `mutated()`'s fresh identity and both demotions to `0` alike. `register_quasi_immutable_deps` now offers each collected dep to the type watcher as well as the module-dict one; both registrations self-filter on the object's kind. The eight fold sites emit `QUASIIMMUT_FIELD` through a shared `walker_pin_type_version_tag`, and `type_version_tag_descr()` becomes a `LazyLock` singleton carrying the quasi-immutable flag — `heap.rs:3274` keys `quasi_immut_cache` on the descr's `Arc` pointer, so a per-call descriptor missed its own cache on every read. Steady-body op counts: append loop 33 → 32; two un-inlined method calls on one instance 41 → 38 (both `_version_tag` reads and their guards replaced by one `GUARD_NOT_INVALIDATED`, which the residual call no longer flushes). Adds `synth/method_reassign_after_warmup`, covering rebinding on the class, rebinding again, rebinding on a base of the warmed receiver's class, and rebinding from inside the loop being traced. With the invalidation call removed the fixture returns the stale methods (`1 1 1 10 10`), so it is not vacuous. Assisted-by: Claude * majit: address PR #932 review — restrict as_ptr fold to slices, fix getslice fixed-result dst_items (#941) as_ptr identity fold (front/mir.rs): drop the `alloc::vec::<Impl>::as_ptr` arm. A resized-list / Vec reaches its items buffer through `ll_items(l) = l.items` (rlist.py:368, a getfield), not the receiver, so aliasing a Vec::as_ptr result to the receiver contradicts the layout; only the fixed-array `<[T]>::as_ptr` matches `ll_fixed_items(l) = l` (rlist.py:399). The two Vec::as_ptr callers (IntArray/FloatArray::from_vec) are host builtins residualised to their compiled bodies, so the Vec arm had no traced consumer — census is unchanged (1151). getslice dst_items (rtyper/rlist.rs): emit_listslice_alloc_and_copy hardcoded `getfield(new_lst, "items")` for the destination, which is wrong for a FixedSizeListRepr result (ll_newlist returns a bare Ptr(GcArray) with no items field). Branch dst_items on the result layout, read off result_ptr_lltype, mirroring the source-layout branch of src_items. Add a fixed-result getslice test asserting no `items` getfield, plus a non-nonneg-start rejection test. Also: fix a stale `simplify_and_finalize` cross-reference in the dormant slice_index note, align slice_index's consumer gate FieldWrite base with its removal sweep (`base == range`), and add a negative-length malloc_varsize llinterp test. Assisted-by: Claude * jit: fix a lost conditional store in an inlined callee, widen the method-form inline (#942) * majit: keep the nonstandard vable element heapcache in sync Two halves of the same gap on the NONSTANDARD virtualizable array path (a force-materialized inline-callee frame's locals_cells_stack_w): - Every `getarrayitem_vable` / `setarrayitem_vable` arm recorded its own fresh `GetfieldGcR` for the array base. The per-array element cache is keyed by that base OpRef, so reads and writes landed in different submaps. Route all four arms through a new `nonstandard_vable_array_base`, which forwards the cached field box on a hit and publishes the recorded op with `getfield_now_known`, the way `_opimpl_getfield_gc_any_pureornot` does. - The nonstandard store recorded `SETARRAYITEM_GC` without updating the heapcache. `execute_setarrayitem_gc` (pyjitpl.py) records and then calls `heapcache.setarrayitem`; add that pairing as `execute_setarrayitem_gc` and call it from the nonstandard arm. `gen_store_back_in_vable` keeps using the raw `vable_setarrayitem_descr`, matching its direct op recording upstream. With both, a local written inside an inlined callee no longer keeps reading the value the element cache was seeded with at frame construction. Assisted-by: Claude * jit: inline a method-form callee whose body reads self.attr `try_walker_inline_user_call` passed `allow_method_load_attr = false`, so `method_form_callee_body_supported` declined every `b.at(i)` whose callee body contains a `LoadAttr` residual — the ordinary accessor shape `def at(self, i): return self.v + i`. The same callee already inlined when stored as a bound method (`m = b.at; m(i)`). Pass `true` there, and document what the helper declines. Measured on the call-shape micro (400k iterations, dynasm): `b.at(i)` 1.54s -> 0.08s, matching the plain-function and bound-method forms; `synth/sre_pattern_methods` prints 280000, the pypy3 value. Assisted-by: Claude * jit: decline the widened method-form inline for raise-bearing and FOR_ITER-deferred callees `try_walker_inline_user_call` passes `allow_method_load_attr = true`, which admits an unbound method-form callee whose body reads `self.attr`. Two body shapes reached that way discarded the enclosing loop instead of falling back to a residual call: - A body containing a `raise`. The sub-walk records into the handler region, and a guard whose resume coordinate lands on the `Reraise` asks `collect_callee_active_boxes` for ref registers the recorded path never wrote. That decline arrives mid-recording on an opcode that is not effect-free, so it has no mid-body carrier and the trace is thrown away. - A FOR_ITER `CalleeReplaySafety::DeferredCall` admission whose deferred call resolves to a builtin: `fbw_abort_nested_unjournaled_residual` spends one abort and then denies the callee. Decline both at the callsite. Measured, median of 3: dynasm sre_pattern_methods 0.90s -> 0.67s, sre_wasm_min 0.55s -> 0.43s, sre_wasm_min1 0.45s -> 0.33s, type_metatype_method_call 0.35s -> 0.28s cranelift the same four return to 0.72s / 0.47s / 0.36s / 0.23s, matching their times with `allow_method_load_attr = false` `loops_compiled`, `bridges_compiled` and `loops_aborted` on all four match the `allow_method_load_attr = false` counters. `inline_subwalk_mutating_residual` keeps the widening's gain at 13.0x pypy3 (dynasm) / 18.3x (cranelift), so its gate moves 200 -> 40, and `inline_subwalk_property_mutates` 80 -> 50. check.py: dynasm 354/354, cranelift 354/354. Assisted-by: Claude * call: translator-safe __call__-slot guard + descriptor tests; operator countOf/indexOf GC fix (#944) * call: guard __call__-slot self-dispatch with call-depth instead of black_box std::hint::black_box is not recognized by the majit translator (front/mir.rs only lowers core::hint::n and core::convert::identity), so in the call-dispatch graph it stayed an unregistered FunctionPath. Replace it at all three slot sites with an increment_call_depth() guard: its drop after the recursive call keeps the self-dispatch off the tail, so a self-referential A.__call__ = A() still recurses natively for stack_check, and it counts the dispatch level. synth: add a descriptor whose __get__ resolves to a separate callable (positional and keyword), covering the get() branch of resolve_user_call_slot. Assisted-by: Claude * operator: move countOf app-level, route indexOf through space.sequence_index Add `baseobjspace::sequence_index` mirroring `descroperation.py:538`, pinning the iterator and needle on the shadow stack so they survive a collection triggered by `__next__`/`__eq__`. Remove the hand-rolled `op_countof`/`op_indexof` loops that held those references in raw locals: `countOf` becomes an `app_operator.py` function (`moduledef.py` `app_names`) and `indexOf` delegates to `sequence_index` (`interp_operator.py:58`). Assisted-by: Claude * Close the PR #937 review queue: GC rooting fixes, os.kill unwrap_spec, and the frame root-walk floor (#946) * _multiprocessing: port W_SemLock's recursion state, release checks and classmethod _rebuild `SemLock` kept no per-lock state, so `_count()` returned 0, `_is_mine()` returned false, `acquire` had no `RECURSIVE_MUTEX` re-entry check and went straight to `sem_wait` (a second acquire on an RLock blocks on a value-0 semaphore), `release` neither checked ownership nor decremented a recursion depth, and `_after_fork` had no state to reset. `count` and `last_tid` (interp_semaphore.py:462,465) now live in the instance dict beside `kind`, `maxvalue`, `handle` and `name`, and `acquire`/`release` follow interp_semaphore.py:506-545. `_is_zero` reported whether the handle was null rather than whether the semaphore was available; it and the new `_get_value` follow interp_semaphore.py:443-455 and :431-441, taking the `sem_trywait` fallback where `sem_getvalue` is broken (darwin, :86-89). `release` gained the maxvalue check of :407-427. `__enter__` returned `self` after an unchecked `sem_wait`; it now returns `acquire()`'s boolean and `__exit__` goes through `release()` (:563-567). `__new__` accepted any `kind`; it now raises `ValueError("unrecognized kind")` (:574-575). `_rebuild` rejected `name=None` and always reopened by name, and allocated on `type_object()` rather than the class it was called through; it is now a classmethod (:606) that falls back to `handle_w` on a nameless semaphore (:550-557, :223-224). Assisted-by: Claude * mapdict: write _mapdict_pop_attribute's map through the rooted receiver Both arms of `_mapdict_pop_attribute` can trigger a collection — the unboxed arm through `_mapdict_write_storage`, the other through the `grow_instance_items_block` shrink — and the trailing `self.map = map` ran through the `&mut self` address captured before them. A moving collection in either arm left that write targeting the stale address, so the live instance kept its old map and its storage index into a block that no longer has the slot. The receiver is now published for the whole function and both the storage write and the `map` write go through the reloaded address, matching `_set_mapdict_increase_storage1` (mapdict.rs:2406) and `_set_mapdict_storage_and_map` (:2427). Assisted-by: Claude * mapdict: treat a null "dict" SPECIAL slot as absent and root the receiver across the three allocating dict paths `_obj_getdict` returned `instance_get_dict_slot`'s answer unfiltered, so a slot holding NULL was reported as the instance's `__dict__`. `mapdict.py:828-830` reads that as "no dict yet" — RPython's `read` answers `None` both for an absent slot and for one holding `None` — and rebuilds the view. Returning the null instead makes `getdict` answer empty, which is exactly the state `descr__setattr__` turns into `"'%T' object attribute '%s' is read-only"` (descroperation.py:58-67): the sole way `_obj_getdict` can produce null, and the reported failure of `self._loop = loop` on a fresh `asyncio.futures.Future` (`_loop` is a class variable, futures.py:53, so the descriptor lookup always hits and the read-only branch is always armed). The sibling `instance_get_weakref_slot` already carries the same filter. Three receivers were also captured before an allocation and used after it: - `_obj_getdict` — `w_dict_new_with` allocates, so both the erased `dstorage` back-pointer baked into the new view and the `instance_set_dict_slot` target were pre-move addresses. `walk_gc_refs` only forwards `dstorage` from the collection after the wrapper is reachable, so the birth allocation is not covered; the back-pointer is now restated from the post-allocation address. - `materialize_dict` — `node_materialize_dict` allocates boxed names, dict stores and carrier transitions before the transplant wrote through the pre-walk `inst`. - `_obj_setdict` — `_obj_getdict` and `mapdict_switch_to_object_strategy` both allocate ahead of `instance_set_dict_slot(self_ref, w_dict)`. Assisted-by: Claude * mapdict: cover the null "dict" SPECIAL slot rebuild with a unit test; rustfmt The test nulls the slot in place — the state a write that never landed leaves behind — and asserts `_obj_getdict` builds a fresh view backed by the instance. It fails without the `!w_dict.is_null()` guard ("a null SPECIAL slot must rebuild the view") and passes with it. Assisted-by: Claude * posix: unwrap os.kill/os.killpg arguments through c_int_w interp_posix.py:1386 and :1394 declare `@unwrap_spec(pid=c_int, signal=c_int)`; both arguments were read with `w_int_get_value`, a raw payload read with no type check. `is_int` compares `ob_type` for exact identity, so no non-`int` argument took a checked path: `os.kill(pid, None)`, `os.kill(pid, [30])` and `os.kill("x", 30)` passed whatever bit pattern sat at that offset to `kill(2)` instead of raising TypeError, and an out-of-range value was truncated rather than reported. Assisted-by: Claude * baseobjspace: include the operand type in space_int's unwrap error baseobjspace.py:323 raises through `_typed_unwrap_error(space, "integer")`, whose body at :316-318 is `"expected %s, got %T object"`. The message here was `"expected integer"`, dropping the operand type. Assisted-by: Claude * mapdict: resolve the unboxed storage slot without allocating `boxed_storage_indices` built a `vec![false; len]` and a result `Vec` on every call. `instance_walk_boxed_storage` calls it from inside `object_object_custom_trace`, so the collector allocated twice per traced instance while marking, and `_mapdict_write_storage` called it through `storage_index_is_boxed` on every attribute store to answer a single-slot question. `_compute_storageindex_listindex` (mapdict.py:549-562) breaks at the first `UnboxedPlainAttribute` ancestor and reuses its `storageindex`, setting `firstunwrapped` only when the walk finds none, so a chain owns at most one unboxed slot. `unboxed_storage_index` returns that slot without allocating; the index list is now derived from it and remains only for `_set_mapdict_storage_and_map`, which needs the list itself. Assisted-by: Claude * posix: build the sysconf_names value before reading the rooted dict Call arguments evaluate left to right, so `w_dict_setitem_str` read the dict out of the shadow stack before `w_int_new` allocated the value: a collection in that window moved the dict and the store wrote through the pre-move address. The slot index is now taken once after `pin_root` instead of being respelled as `shadow_stack_len() - 1` inside the loop. Assisted-by: Claude * eval: floor the frame root walk at the locals/cells prefix Both walks over `locals_cells_stack_w` took `valuestackdepth.min(len)` as the walk length. `valuestackdepth` is an absolute index that starts at `stack_base()` (pyframe.rs:2136) and `pop` refuses to go below it, so for a running frame that already covers the locals/cells prefix — `PyFrame::descr_clear` is the exception: it rebinds every cell slot to a fresh `w_cell_new` and then sets `valuestackdepth = 0`. Both walks then scanned zero slots for a cleared frame, so cells reachable only through the array were neither kept nor forwarded while the array still pointed at them. The length now floors at `stack_base()`, which only ever widens the walk. The SAFETY comment claiming the walk covers the full fixed-length array described neither the previous behaviour nor this one. Assisted-by: Claude * _multiprocessing, thread: reload the receiver past getdict_native; drop a repeated fork-child reinit `semlock_instance` pinned `obj` and then passed the pre-pin copy to `getdict_native`, which materialises the instance dict and can therefore collect and move it; the receiver is now read back from its slot, as the stores in the same function already do. `after_fork_child` called `setobject::set_locks_after_fork_child()` twice. Assisted-by: Claude * gc_roots, executioncontext, posix, _socket: root operands held across collecting calls `pin_roots` did not carry `pin_root`'s debug-only `assert_shadow_stack_not_walking`, so a caller moving from one to the other lost the guard against publishing roots mid-walk. `_call_finalizer` read `__del__` out of the type dict and passed the pre-call pointer to `report_error` after running arbitrary Python. `posix_spawn`'s env loop kept the mapping and the key vector in plain locals across `getitem` and two `fsencode_bytes_w` calls. `sendmsg` pinned each data item and then iterated the unrooted vector, handing pre-pin copies to `simple_buffer_bytes`, which looks up `__buffer__` and builds a memoryview. Assisted-by: Claude * mapdict: scan the whole chain for unboxed slots instead of the first hit The allocation-free rewrite returned at the first `firstunwrapped` node, resting on `_compute_storageindex_listindex` (mapdict.py:549-562) giving a chain at most one. The collector is the wrong place to depend on that: under-reporting one unboxed slot hands the marker a raw `Vec<i64>` pointer. `storage_index_is_unboxed` now scans the chain for a matching storageindex, which restores the exact set the pre-rewrite `boxed_storage_indices` computed while keeping both allocations out of `object_object_custom_trace`. Assisted-by: Claude * types: complete the UnionType slot surface; functools function exports; check.py startup median (#936) * types: expose UnionType rich comparison slots * types: complete UnionType identity and iteration slots * functools: export reduce and cmp_to_key as functions * check.py: take the empty-program startup median over five samples The measured startup is subtracted from every timed run, so it is the divisor for short benches. One high sample collapses that denominator: a macOS runner measured pypy startup at 0.031s where the same job had measured 0.013s, and three benches failed their gates in that run while their pyre exec times had gone down. Assisted-by: Claude * jit,interp: lower DELETE_NAME/GLOBAL, drop invalidated celldict watchers (#935) * interp: decline only a metatype data descriptor in the classmethod fold `classmethod_on_type_fast_path` declined every name the metatype defines. `typeobject.py:813-823 descr_getattribute` orders the two lookups the other way: the metatype entry preempts the class's own MRO only when it is a data descriptor, and otherwise `self.lookup(name)` is what gets selected. Since the fold already requires the class MRO lookup to answer a classmethod, the class-MRO-miss arm is unreachable, so a non-data metatype attribute is provably never the selected value. Swap the check to `type_lookup_is_data_descr`, which is the existing `space.is_data_descr(space.lookup(...))` analogue in the same module. The check was dead on main until the metatype pin landed: it read `&TYPE_TYPE` as an object pointer, whose first field is an `AtomicI64`, so `lookup_in_type`'s `is_type` gate rejected it for every name. Pinning the real metatype made it live, and with it the divergence. Reach is one name: the walker declines every dunder up front, and `mro` is the only non-dunder `type` defines. `Cls.mro()` with `mro` a classmethod now folds, matching pypy3 and python3. Assisted-by: Claude * comments: name the real blockers behind three declined ports No behaviour change. `w_pytraceback_get_lineno` justified the eager `offset2lineno` stamp with "PyFrame is not a GC-traced W_Root, the frame may already be freed". That stopped being true: `pytraceback_object_custom_trace` forwards the `frame` edge, and `w_code` — which is `frame.pycode` — is forwarded unconditionally, so a lazy resolve off `w_code` and `lasti` would be safe. What actually blocks the port is the JIT fold, which reads the slot and declines on the sentinel, so every fresh node would decline on its first read. Record that, and the two `tb_lineno` answers the eager stamp diverges on: `tb.tb_lasti = N` before the first read, and a sentinel written back through `TracebackType` or the setter. The `TbLineno` fold named one sentinel producer; add the other two. `FBW_FORITER_DEFERRED_DENY` lacked the per-thread paragraph its sibling `FBW_HAZARDOUS_INLINE_DENY` carries, so nothing said that it is only a memo and that the live abort is what keeps a deferred body honest. Assisted-by: Claude * builtins: read the pinned iterator back from its slot `collect_iterator`, `builtin_any` and `builtin_all` pinned the iterator and then ran the dict-view type check and the backing-dict read off the raw local. `pin_root` normalizes through `try_gc_current_object_address` and writes the result into the shadow-stack slot only, so the slot — not the local — is the authoritative value once it returns. The loop below already reloaded per call; these three lines were the outliers, and the sibling `min_max_sequence` in the same file already reads the slot for exactly this. No live path reaches the difference today: every builtin iterator is `allocate_stable` and a dict-view iterator is off-GC `malloc_typed`, so neither moves, and nothing collects between `iter(...)` and the pin at any of the three sites. Assisted-by: Claude * interp: read the frame builtin through get_builtin in LOAD_GLOBAL `_load_global` (pyopcode.py) reaches the builtins fallback through `self.get_builtin()`. `load_global_value` and the `jit_load_name_from_namespace` extern read the `w_builtin` slot directly instead. `pyframe.py` assigns `self.builtin` only under `honor__builtins__`, which `baseobjspace::HONOR_BUILTINS` leaves off, so `get_builtin` answering `space.builtin` for an unset slot is the only route that resolves a builtin at all for a frame nobody wrote the slot on -- among them the inlined-callee frames the tracer emits, which follow the same constructor and leave it at the allocation's zero-fill. Before this, forcing one of those frames made every builtin name raise NameError. Both frame emitters' trailing comments now list `w_builtin` alongside the other class-level defaults they deliberately do not store. Assisted-by: Claude * dynasm: take getenv and String building off the entry path `execute_token` read `PYRE_DYNASM_EXEC_DIAG` once and `PYRE_GC_FREELIST_DIAG` twice per compiled-trace entry, each an uncached `std::env::var_os` that takes the global env lock and walks the env table. It also built the `format!("before/after trace {id}")` site strings unconditionally, so a diagnostic that is off still cost two heap allocations per entry. `dynasm_debug_validate_oldgen_freeblocks` paid the same getenv plus one `format!` per residual call. Cache both flags in `LazyLock` bools next to `majit_log_enabled`, and take `std::fmt::Arguments` in `debug_validate_oldgen_freeblocks` so the site name is only materialized once the flag is on. Sampling `synth/gc_bug_bridge_flavor_traceback_names` (5s, dynasm): samples under `run_compiled_detailed_with_values_at_dispatch_key` 244 -> 113, with the `__findenv_locked` and `debug_validate_oldgen_freeblocks` frames gone from the entry path. Assisted-by: Claude * jit: lower DELETE_NAME and DELETE_GLOBAL to residual calls Both opcodes were emitted as a static `abort_permanent`, which discards the enclosing loop trace permanently. A module-level `except X as e:` compiles its implicit cleanup to `e = None; del e`, and at module scope `del e` is DELETE_NAME, so any module loop containing an except-as clause could never be traced. Function scope uses DELETE_FAST, which was already lowered. pyopcode.py DELETE_NAME is `space.delitem(w_locals, w_varname)` and DELETE_GLOBAL the same against `w_globals`; neither is an unsupported opcode upstream. Adds the `delete_name` / `delete_global` HLOps in the frame-receiver call shape of `store_name`, minus the value operand: two Ref operands, void result, zero stack effect. They lower to `residual_call_r_v` with `PyreHelperKind::None` (there is no fold for delete) and are registered in `graph_op_can_raise` — DELETE_NAME raises NameError when the binding is absent. `bh_delete_name_fn` / `bh_delete_global_fn` delegate to the existing `OpcodeStepExecutor` methods on PyFrame and publish into both exception cells on error, matching `bh_store_name_fn`. SetupAnnotations stays on `abort_permanent`. loops_aborted -> 0 on the three benches whose hot module loops reached the op: exception_reraise_tb_depth_hot (30 -> 0, bridges_compiled 0 -> 9), exception_reraise_tb_depth_jitstress (30 -> 0), and exception_reentry_guard_finally_residual (5 -> 0). exception_reraise_tb_depth_hot: 692ms -> 267ms CPU, 6.5x -> 2.5x of pypy. check.py dynasm 354/354, cranelift 354/354. Assisted-by: Claude * celldict: drop version watchers once they are invalidated `sweep_version_watchers` used `Vec::retain` and returned true for every watcher it could still upgrade, so a live loop flag stayed in the list after being flipped. `QuasiImmut.invalidate` (quasiimmut.py) instead takes the list and empties it — `wrefs = self.looptokens_wrefs; self.looptokens_wrefs = []` — so a loop is invalidated exactly once and then drops out. Retaining them is redundant and unbounded: the flag is already true, so re-storing it changes nothing, and the list grows by one entry per compiled loop that folded a module-global. A module-level `except X as e:` runs `del e` every iteration and `delitem` calls `mutated()`, so each iteration walked every loop ever compiled in that module. Dropping is safe because registration is per compiled artifact: `last_compiled_artifact_invalidation_flag` registers a fresh flag on every compile, and a flipped flag is permanently invalid. On synth/exception_metadata_hot at N=250000 this was the largest single non-interpreter profile leaf (629 samples, ahead of eval_loop_jit) and is absent from the profile afterwards; 13244ms -> 12019ms CPU, 8.31x -> 7.54x of pypy. It does not change loops_compiled (309 either way) — the remaining gap is the version churn itself, not the walk. Also corrects the `_setitem_str_cell_known` doc comment, which claimed `write_cell` was stubbed to identity; it ports the MutableCell family and absorbs a same-cell rewrite without touching the version. cargo test pyre-object + pyre-interpreter green, check.py cranelift 354/354, dynasm 353/354 with synth/jit_reg_const_pool_256_slot_decline a gate-boundary flake (passes 3/3 solo; both binaries measure 984ms vs 988ms on it). Assisted-by: Claude * codewriter: reformat two delete-opcode let bindings The DeleteName / DeleteGlobal arms sit at a shallower indentation after the rebase onto main, so rustfmt joins both `Constant::string` bindings onto one line. Whitespace only; `cargo fmt --all --check` is clean again. Assisted-by: Claude
…nline moved `#942` passes `allow_method_load_attr = true` at `try_walker_inline_user_call`, so an unbound method-form callee whose body reads `self.attr` inlines into the caller's loop instead of running as a residual call. Three fixtures carry that shape (`c.add(i & 7)`, `self.process_word()`) and lose the callee's separately compiled loop: class_attrs_methods loops_compiled 2 -> 1 (dynasm, cranelift) foriter_inplace_immutable loops_compiled 2 -> 1 (cranelift) inline_subwalk_mutating_residual loops_compiled 4 -> 3 (cranelift) Every counter that moves with them falls: `class_attrs_methods` cranelift `guard_failures` 202 -> 1 and `bridges_compiled` 1 -> 0; `inline_subwalk_mutating_residual` cranelift `guard_failures` 12316 -> 603 and `loops_aborted` 1 -> 0, wasm `loops_aborted` 4 -> 2. The measured ratio for that fixture is 13.1x dynasm / 17.2x cranelift, matching the 13.0x / 18.3x `#942` records. `method_reassign_after_warmup` arrived with `#940` and had no baseline, which the missing-baseline gate reported on all three backends. check.py: dynasm 359/359, cranelift 359/359, wasm 355/355. Assisted-by: Claude
….py: arm the jit-stats floor across the synthetic suite (#947) * jit: count the deduped set in intern_liveness' liveness record header `intern_liveness` keyed its dedup table on the sorted, deduped form of its three `&[u8]` arguments but took the record's count bytes and encoded bitsets from the raw slices. `encode_liveness` dedups, so a repeated register index made the count exceed the bitset's cardinality. `LivenessIterator` is driven by that count, so decoding such a record read past its own bitset into the following record's bytes and swallowed it. Read the counts and the bytes off the canonical form already computed for the key, matching `majit-translate`'s producer, which counts the `VecSet` it encodes from (codewriter/assembler.rs:1060). Assisted-by: Claude * majit: read the diagnostic env gates once, and build their messages behind them `execute_token` brackets every entry into compiled code with `debug_validate_oldgen_freeblocks`, which read `PYRE_GC_FREELIST_DIAG` with `std::env::var_os` on each call, and it read `PYRE_DYNASM_EXEC_DIAG` the same way — three `getenv`s per trace entry, each taking the environment lock and scanning the array. The two bracket calls also built their `site` string with `format!` before the gate, so a disabled diagnostic still allocated and formatted one per trace entry, and `dynasm_debug_validate_oldgen_freeblocks` one per residual call site. Cache both gates in a `LazyLock<bool>` as `majit_log_enabled` does, take `site` as `Arguments` so the string is materialised only past the gate, and do the same for `MAJIT_LOG` in `do_collect_nursery` / `finish_incremental_cycle` and `PYRE_GC_FREELIST_DIAG` in `dynasm_nursery_slowpath`. On a 20x-scaled `recursion_memo_branch` the startup-subtracted user-CPU exec time goes 0.7609s to 0.6566s (0.863x, min of 7 interleaved rounds, same host), output unchanged; `__findenv_locked` and `std::env::__var_os` fall from 43 of 388 working samples to 0 of 581. Also correct the comment above the `should_disable` application: it cited `trace_opcode.rs:3044-3049` as a second site applying `disable_noninlinable_function`, where no such symbol exists. Assisted-by: Claude * jit: emit the CALL's null_or_self slot in the outer-call stack overrides `collect_call_stack_overrides` had no source for the `null_or_self` operand a `PUSH_NULL` leaves under a `CALL`: the walk holds no box for that slot, no live color names it, and the virtualizable shadow reads back the same NULL an unmirrored slot does. The slot stayed absent, so the outer-call flush that follows a `LoopBearingCalleeInlineUnsupported` abort declined on "stack override missing for a live slot" and fell back to the legacy rollback+replay, where `fbw_foriter_inflight_take` refuses in-flight FOR_ITER delivery and the whole iteration is dropped. Name the slot from the CALL's own operand layout — `[callable, null_or_self, arg0 .. arg_{argc-1}]` ends at the caller's stack top, so the sentinel sits `argc + 1` below it — and push an explicit null override. A coordinate that does not invert to a plain `CALL` keeps the existing decline. The added `bench/synth/foriter_call_resume_drops_iteration.py` printed `491088 8999` before this change and `491130 9000` after, matching CPython, PyPy and `PYRE_NO_JIT=1`. Assisted-by: Claude * check.py: gate guard_failures on a bounded rise against the jit-stats baseline _jit_stats_regression_floor now iterates JITSTATS_RISE_BOUNDED_FIELDS in addition to JITSTATS_BADNESS_FIELDS. A badness counter still fails on any rise above its baseline; a rise-bounded counter fails only past `base + max(base // 4, 2)`. A field absent from the baseline keeps reading as 0 for the badness counters, whose healthy value is 0, but leaves a rise-bounded counter unpinned: the wasm [jit-stats] line reports no guard_failures, so every *.wasm.jitstats baseline omits it. guard_failures moves out of the informational snapshot surface into that gate. Assisted-by: Claude * check.py: track the synthetic .jitstats baselines pyre/.gitignore ignored /bench/synth/*.jitstats with three `!` negations, so the regression floor read a committed baseline for 3 of 340 synthetic fixtures and skipped the other 337. Recorded with `pyre/check.py --snapshot` (dynasm 355/355, cranelift 355/355, wasm 351/351): 340 dynasm, 340 cranelift, 339 wasm — getframe_caller_locals_nested_compiled_callee carries skip-backends=wasm. Of the 1019 synthetic baselines, 872 record all-zero badness counters, 147 record a nonzero loops_aborted, and none record a nonzero descr_set_* or internal_compile_panics. Assisted-by: Claude * check.py: record the synthetic baselines the rebase left missing or stale defaults_reassigned_midloop and del_cellvar_walk_commit arrived from origin/main with no .jitstats baseline, so the regression floor skipped them. type_metatype_method_call moves loops_compiled 4 -> 2, bridges_compiled 47 -> 0 and guard_failures 9480 -> 1 on the native backends, the effect #925 records in its own message. comprehension_object_append_hot.dynasm moves guard_failures 3611 -> 3612. Assisted-by: Claude * check.py: gate loops_compiled on a fall, and fail a benchmark with no baseline JITSTATS_FALL_FIELDS gates a counter whose defect direction is down. loops_compiled is its first member: a pre-trace decline aborts nothing, so loops_aborted holds, and it removes the compiled guards that used to fail, so guard_failures falls — both existing gates read the loss as an improvement while the hot loop runs interpreted. The fall is gated exactly rather than through a band. Diffing the dynasm and cranelift baselines across the 341 synthetic fixtures, the two code generators agree on loops_compiled for 340 of them (99.7%), against 97.4% for guard_failures. A benchmark that emits a [jit-stats] line but has no committed baseline now fails instead of passing. Every gate above compares against nothing when the file is absent, so "never recorded" and "recorded and clean" print the same PASS — which is how the floor came to cover 3 of 340 synthetic fixtures. Assisted-by: Claude * check.py: re-record the jit-stats baselines the widened method-form inline moved `#942` passes `allow_method_load_attr = true` at `try_walker_inline_user_call`, so an unbound method-form callee whose body reads `self.attr` inlines into the caller's loop instead of running as a residual call. Three fixtures carry that shape (`c.add(i & 7)`, `self.process_word()`) and lose the callee's separately compiled loop: class_attrs_methods loops_compiled 2 -> 1 (dynasm, cranelift) foriter_inplace_immutable loops_compiled 2 -> 1 (cranelift) inline_subwalk_mutating_residual loops_compiled 4 -> 3 (cranelift) Every counter that moves with them falls: `class_attrs_methods` cranelift `guard_failures` 202 -> 1 and `bridges_compiled` 1 -> 0; `inline_subwalk_mutating_residual` cranelift `guard_failures` 12316 -> 603 and `loops_aborted` 1 -> 0, wasm `loops_aborted` 4 -> 2. The measured ratio for that fixture is 13.1x dynasm / 17.2x cranelift, matching the 13.0x / 18.3x `#942` records. `method_reassign_after_warmup` arrived with `#940` and had no baseline, which the missing-baseline gate reported on all three backends. check.py: dynasm 359/359, cranelift 359/359, wasm 355/355. Assisted-by: Claude * codex-review: withhold generated .jitstats baselines from the changed-file list The workflow appends `git diff --name-only upstream/main` to the review prompt as the authoritative definition of "this patch". On a PR that re-records check.py's jit-stats baselines in bulk that list reached 1027 entries and Codex returned no report at all (exit 1); CodeRabbit skipped the same PR for being 927 files over its own 100-file limit. `.jitstats` files are generated golden data with no RPython/PyPy counterpart, so no parity finding can cite one. Exclude them from the list, report the withheld count in the prompt header and in the empty-diff message, and put the same exclusion in the prompt's own scope-discipline instruction, which is what the local `/codex-review` path re-derives from. On this branch the list goes from 1132 to 112 files. Assisted-by: Claude * check.py: re-record the exception-reraise baselines main's trace shape moved `origin/main` alone reproduces the counters exactly, so this is not our branch: exception_reraise_tb_depth_hot 40 loops / 0 guards / 30 aborts / 0 bridges -> 4 loops / 1803 guards / 0 aborts / 9 bridges exception_reraise_tb_depth_jitstress 900 / 0 / 30 / 0 -> 805 / 1798 / 0 / 1 Both gated directions fired at once (`loops_compiled` down, `guard_failures` up) and neither is a defect on the native backends. Interleaved user+sys CPU time, min of 5, `e1e97e61d32` vs `origin/main`, same host: `_hot` 0.750s -> 0.370s (2.03x faster), `_jitstress` 0.490s -> 0.490s. The 40 specialized loops with 30 aborts were the worse shape. wasm moves the other way on the same two fixtures: `loops_aborted` 30 -> 44 and 30 -> 1198. Recording those pins the higher value, so the rise is stated here rather than absorbed silently. check.py, per fixture: dynasm 1/1, cranelift 1/1, wasm 1/1. Assisted-by: Claude
Follow-up to #931. That PR closed the
w_classhalf of the LOAD_METHOD method-cache pair by deduplicating a descr split; this closes the_version_taghalf, and it turned out not to be a descr problem at all.The hypothesis I started with was wrong
The plan was "apply the same bridge shape — the parentless descr blocks the fold".
type_version_tag_descr()really did mint a freshArcon every call, at eight emit sites, withname: "",index_in_parent: 0,parent_descr: None.It cost nothing. The read is always off the pinned type constant, and the constant path does not key on
descr_identity; it slots byOptHeap::field_slot_index→Descr::index()→stable_field_index(offset, field_size, field_type, signed), which is content-derived. TwoArcs describing the same field land in the same slot, so the fold hit anyway. Measured on two natively-foldedxs.appendin one body: 2_version_tagreads before opt → 1 in the steady body. Already deduped.What it actually was
The pair survived only across a residual
CALL_MAY_FORCE— and that flush is correct, because pyre modelled_version_tagas a plain mutable field. Two un-inlined method calls on one instance gave a 41-op steady body with ops 8/11-15 repeated verbatim as 22-27.The control that settles it: op 26 in that repeat is
W_ObjectObject.map, a group-backed descr with a real parent — and it duplicated too. The cause is the call flush, not the parentless spelling.Upstream pays none of this:
typeobject.py:177 _immutable_fields_ = ['_version_tag?']. The read becomes aQUASIIMMUT_FIELDmarker plus oneGUARD_NOT_INVALIDATEDper trace, which no residual call can flush.type_version_tag_descr's own doc comment already named this as the convergence path, "once pyre wires that write barrier intomutated()".The blocker was one function
QuasiimmutFieldis fully ported and live —state.rs:3394emits it,heap.rs:3236-3280records the dep andRemoves the op, module globals already use it. Butregister_quasi_immutable_deps(pyre-jit/src/eval.rs) sent every dep tomodule_dict_register_version_watcher, which early-returns on a non-dict. A type-object dep had nowhere to register, so the flag was never flipped — which is exactly why the field had to stay mutable.What this does
Ports
rpython/jit/metainterp/quasiimmut.py'sQuasiImmut—register_loop_token,compress_looptokens_list(compress_limit = 30,(len + 15) * 2), andinvalidate(which empties the list) — onto a newW_TypeObject.quasi_immut_watchers. That field stands in for the hiddenmutate__version_tagthe rtyper synthesises upstream; pyre has no rtyper to synthesise it, so it is spelled out, allocated on first registration exactly like its siblingweak_subclasses. Pyre's per-artifactAtomicBoolreplaceslooptoken.invalidated = True+cpu.invalidate_loop.Four seams, deliberately one commit — the invalidation has to exist before the read is folded away:
type_version_tag_descr()→ aLazyLocksingleton carrying the quasi-immutable flag. Not optional:heap.rs:3274keysquasi_immut_cacheonfield_cache_identity, which is theArcpointer, so a per-call descriptor missed its own cache on every read.walker_pin_type_version_tag— no load, noGUARD_VALUE.w_type_set_version_tag, the field's only writer, so it coversmutated()'s fresh identity and both demotions to0without asking each caller to remember.register_quasi_immutable_depsoffers each dep to the type watcher as well as the module-dict one; both registrations self-filter on the object's kind.Measurements
v_build)Both
_version_tagreads and both their guards are replaced by a singleGuardNotInvalidated(), which sits inside the steady body — so it re-checks every iteration — and which the residual call between the two call sites no longer flushes.Regression coverage, and proof it is not vacuous
New
synth/method_reassign_after_warmup, four phases: rebind on the class, rebind again (so a stale second-generation trace is caught), rebind on a base of the warmed receiver's class (mutated()'s subclass walk), and rebind from inside the loop being traced. That last one is the case the old live read +guard_valueused to catch for free; it is closed because the guard is per-iteration.Commenting out the single
w_type_notify_quasi_immut_watcherscall makes the fixture return1 1 1 10 10— the stale methods survive. So the fixture is not vacuous and the invalidation is exactly load-bearing.Its
max-pypy-ratiois deliberately loose (60) and the header says why: pypy folds this loop to 0.01s at every N tried, so the denominator is collapsed and the ratio measures pypy's constant folding. The differential output is the gate.Verification
check.py: dynasm 355/355, cranelift 355/355cargo test --release -p pyre-object -p pyre-jit-trace -p majit-metainterp: 2152 passed, 0 failed, including two newQuasiImmutunit tests (invalidate flips + clears; register compresses dead tokens)w_subclass2,retag_force,v_buildunchanged on both backends and underPYRE_NO_JITorigin/main(jit-trace: bridge the codewriter's PyObject.w_class read to the walker descr (append loop 40 → 33 ops) #931 merged asf0627903782) and re-built, re-measured and re-gated on that baseKnown, unchanged by this PR
W_TypeObjecthas no teardown path in this tree —weak_subclassesis never freed either — so the watcher box outlives a collected heap type.compress_looptokens_listbounds the growth that matters. Registration and notify are unsynchronisedVecoperations, matchingModuleDictStrategy::register_version_watcherandw_type_add_subclass.str_len_descr()still mints a freshArcper call. Same latent shape, currently unobservable — the reads get hoisted out of the loop entirely — so it is left alone rather than changed on speculation.— authored by Claude
Summary by CodeRabbit
Bug Fixes
Performance
Tests