From a1ab82a8dfadf2f17996bb5b46ab41737186a147 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 17 Aug 2026 14:42:51 +0900 Subject: [PATCH 01/24] jit: admit the bound-method folds at an inline depth whose guards resume in the callee `try_walker_specialize_load_bound_method_attr`, `try_walker_specialize_load_ classmethod_attr`, and the class pin in `try_walker_fold_load_method_self` declined for the whole of an inlined callee sub-walk, on the grounds that a guard emitted there resumes at the caller's CALL and re-runs the callee from its entry. That is the single-frame collapse; when the paused-caller chain covers the full inline depth the multi-frame snapshot resumes the callee at its own coordinate instead. The condition `walker_capture_snapshot_for_last_guard_impl` fires the multi-frame path under is factored out as `walker_inline_guard_resumes_in_callee` and is now the predicate the three folds consult. synth/inlined_helper_mutation moves 39.4x -> 15.1x (dynasm), 70.2x -> 27.7x (cranelift), 60.8x -> 26.4x (wasm); its ceiling goes 145 -> 60. Assisted-by: Claude --- pyre/bench/synth/inlined_helper_mutation.py | 14 +++-- .../src/jitcode_dispatch/resume_snapshot.rs | 58 +++++++++++-------- .../src/jitcode_dispatch/specialize.rs | 37 +++++++----- 3 files changed, 66 insertions(+), 43 deletions(-) diff --git a/pyre/bench/synth/inlined_helper_mutation.py b/pyre/bench/synth/inlined_helper_mutation.py index 9228c6d48fa..84124d9dc24 100644 --- a/pyre/bench/synth/inlined_helper_mutation.py +++ b/pyre/bench/synth/inlined_helper_mutation.py @@ -1,8 +1,14 @@ -# pyre-check: max-pypy-ratio=145 +# pyre-check: max-pypy-ratio=60 # The trip count now puts pypy above the startup-subtraction floor, so this -# ratio is a measurement rather than pyre divided by the floor constant. The -# ceiling is twice the slowest of the three backends observed unclamped -# (71.1x on wasm); the previous 45 was fitted against the clamp and fails. +# ratio is a measurement rather than pyre divided by the floor constant; a +# ceiling fitted against the clamp (the 45 this bench once carried) fails. +# The bound is twice the slowest of the three backends, rounded up. +# +# `push` binds `a.append` inside an inlined callee, and the folds that shape a +# bound-method load used to decline for the whole of such a sub-walk. They now +# decline only where a guard would collapse its resume to the caller's CALL, +# so the binding folds here: the ratio fell from 39.4x/70.2x/60.8x to +# 15.1x/27.7x/26.4x (dynasm/cranelift/wasm). # Inlined-callee shared-heap mutation parity, in both helper orderings. # # A tiny helper mutates a caller-owned list/instance inside a hot while-loop, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index eb5b6f34c65..01edb56704e 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -299,6 +299,27 @@ fn walker_capture_inline_nonstandard_vable_guard_inner( Ok(()) } +/// Whether a guard emitted at the current inline depth resumes at its own +/// callee coordinate instead of collapsing to the caller's CALL boundary. +/// +/// The multi-frame snapshot below fires only when the paused-caller chain +/// covers the full inline depth — one parent per active inlined callee. A +/// shorter chain falls through to the single-frame collapse, whose resume +/// re-executes the entire call, so a guard emitted under it re-runs every side +/// effect the inline region sequenced before it. A fold that must not be +/// re-run consults this before emitting its guards. +pub(crate) fn walker_inline_guard_resumes_in_callee( + ctx: &WalkContext<'_, '_, Sym>, +) -> bool { + let session = ctx.session.borrow(); + let n_parents = session + .framestack + .iter() + .filter(|frame| frame.parent.is_some()) + .count(); + n_parents > 0 && n_parents == session.framestack.len() +} + pub(crate) fn walker_capture_snapshot_for_last_guard_impl( ctx: &mut WalkContext<'_, '_, Sym>, op_pc: usize, @@ -385,31 +406,22 @@ pub(crate) fn walker_capture_snapshot_for_last_guard_impl( // populates the chain; straight-line callees keep the empty chain + the // single-frame collapse below. if inline_subwalk { - // Fire the multi-frame snapshot only when the paused-caller chain - // covers the FULL current inline depth: framestack levels with parents - // must have one entry per active inlined callee. A nested - // straight-line callee inlined under a multiframe ancestor (e.g. - // `add3` inside a multiframe `mix`) pushes NO parent frame, so its own - // guards see a SHORTER chain than the callee depth — fall through to - // the single-frame collapse (the strict callee's resume-at-CALL - // behavior) rather than emit a chain that skips the intermediate frame. - let (n_parents, n_callees, parent_frames) = { + let parent_frames = { let session = ctx.session.borrow(); - ( - session - .framestack - .iter() - .filter(|frame| frame.parent.is_some()) - .count(), - session.framestack.len(), - session - .framestack - .iter() - .filter_map(|frame| frame.parent.clone()) - .collect::>(), - ) + session + .framestack + .iter() + .filter_map(|frame| frame.parent.clone()) + .collect::>() }; - if n_parents > 0 && n_parents == n_callees { + // Fire the multi-frame snapshot only when the paused-caller chain + // covers the FULL current inline depth. A nested straight-line callee + // inlined under a multiframe ancestor (e.g. `add3` inside a multiframe + // `mix`) pushes NO parent frame, so its own guards see a SHORTER chain + // than the callee depth — fall through to the single-frame collapse + // (the strict callee's resume-at-CALL behavior) rather than emit a + // chain that skips the intermediate frame. + if walker_inline_guard_resumes_in_callee(ctx) { // A STRICT straight-line callee (gh#420) whose own frame is not // MF-snapshot-able (a kept operand-stack temp the sub-walk does not // mirror) propagates the `Unsupported` error the same as the branch diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 7e87180c0cf..c619ee8edc6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -3299,11 +3299,11 @@ pub(crate) fn try_walker_specialize_load_method_attr( /// type as `cls`, and the following `CALL` inlines `__func__(cls, ...)` — the /// instance-method shape with the class in the receiver slot. /// -/// Restricted to the top full-body frame for the reason -/// [`try_walker_specialize_load_bound_method_attr`] carries: a fold guard -/// inside an inlined callee sub-walk resumes at the caller's CALL, re-running -/// side effects. The `getattr` residual resumes past the call, so declining -/// there re-runs nothing. +/// Carries the inline-depth restriction +/// [`try_walker_specialize_load_bound_method_attr`] documents: under the +/// single-frame collapse a fold guard inside an inlined callee sub-walk +/// resumes at the caller's CALL, re-running side effects. The `getattr` +/// residual resumes past the call, so declining there re-runs nothing. #[allow(clippy::too_many_arguments)] pub(crate) fn try_walker_specialize_load_classmethod_attr( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3317,7 +3317,7 @@ pub(crate) fn try_walker_specialize_load_classmethod_attr( if !ctx.is_authoritative_executor || dst_bank != 'r' { return Ok(None); } - if ctx.fbw_mode.inline_subwalk { + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { @@ -3537,12 +3537,14 @@ pub(crate) fn try_walker_specialize_load_type_attr( /// Returns `None` (fall through to the residual, SAFE) for every shape /// [`pyre_interpreter::baseobjspace::bound_method_attr_fast_path`] declines. /// -/// Restricted to the top full-body frame for the reason -/// [`try_walker_orthodox_list_append`] documents: inside an inlined callee -/// sub-walk a fold's guards collapse their resume to the caller's CALL -/// boundary, so a guard failure re-runs the callee from its entry and doubles -/// any side effect it sequenced before this `LOAD_ATTR`. The residual resumes -/// past the call instead, so declining here re-runs nothing extra. +/// Inside an inlined callee sub-walk the fold is restricted to a depth whose +/// guards resume at their own callee coordinate +/// ([`walker_inline_guard_resumes_in_callee`]). Under the single-frame +/// collapse the reason [`try_walker_orthodox_list_append`] documents applies: a +/// guard resumes at the caller's CALL boundary, so a failure re-runs the callee +/// from its entry and doubles any side effect it sequenced before this +/// `LOAD_ATTR`. The residual resumes past the call instead, so declining there +/// re-runs nothing extra. #[allow(clippy::too_many_arguments)] pub(crate) fn try_walker_specialize_load_bound_method_attr( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3553,7 +3555,10 @@ pub(crate) fn try_walker_specialize_load_bound_method_attr( dst: usize, dst_bank: char, ) -> Result, DispatchError> { - if !ctx.is_authoritative_executor || dst_bank != 'r' || ctx.fbw_mode.inline_subwalk { + if !ctx.is_authoritative_executor || dst_bank != 'r' { + return Ok(None); + } + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { @@ -3661,10 +3666,10 @@ pub(crate) fn try_walker_fold_load_method_self( let method_type_addr = &pyre_object::function::METHOD_TYPE as *const _ as i64; let class_pinned = attr.is_constant() || ctx.trace_ctx.heap_cache().is_class_known(attr); if !class_pinned { - // A guard here would resume at the caller's CALL inside an inlined - // callee sub-walk, re-running whatever that callee already did; + // Under the single-frame collapse a guard here would resume at the + // caller's CALL, re-running whatever that callee already did; // leave those to the residual (which resumes past the call). - if ctx.fbw_mode.inline_subwalk { + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let type_const = ctx.trace_ctx.const_int(method_type_addr); From b6e825541f74e6817f18df78e04bbba0d96ed522 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 17 Aug 2026 16:32:54 +0900 Subject: [PATCH 02/24] jit: inline the receiver type's __getattr__ hook for a missing attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `descroperation.py:242-245` reaches the hook only after the descriptor protocol has raised, so a hooked access cost one opaque residual holding the whole `object_getattr_miss` walk — the `__dict__` / `__doc__` / `__class__` special names, the metaclass loops, the terminal miss — and then a fresh interpreter frame for the hook, on every iteration. Every other user dunder already has a resolver into `try_walker_inline_resolved_user_call` (`__getitem__`, `__add__`, `__hash__`, `__index__`, `property.__get__`, `__eq__`); `__getattr__` had none. `mapdict::getattr_hook_fast_path` is the miss twin of `load_attr_fast_path`: it answers with the type's version tag and the instance map, which are what make "the name resolves nowhere and `__getattr__` is this one" a constant of the trace. `try_walker_inline_getattr_hook` emits those pins through `walker_guard_mapdict_instance_shape` and enters the hook. All three spellings `get_and_call_function` binds are folded — a plain `Function` leads with the receiver, a `classmethod` with the class, a `staticmethod` with nothing; a custom-descriptor hook stays on the residual. The name argument is an interned immortal block, the shape `pyopcode.py LOAD_ATTR` passes (`co_names_w[oparg]`, one object per code object). Per-access cost at N=600000, dynasm: plain hook 0.346s -> 0.088s, classmethod 0.410s -> 0.080s, staticmethod 0.347s -> 0.076s, each from one residual to none (an existing attribute reads 0.074s). synth/getattr_hook_binding moves 48.6x -> 7.4x (dynasm), 10.4x (cranelift), 8.7x (wasm); its ceiling goes 90 -> 25. `extra_tests/parity_tests/getattr_hook_inline_deopt.py` breaks each pin in turn mid-loop — a store that puts the name on the instance, a reassigned `__getattr__`, a class attribute that shadows the hook — and covers a raising hook, an inherited classmethod hook's bound class, and a hook that installs the attribute itself. Assisted-by: Claude --- pyre/bench/synth/getattr_hook_binding.py | 8 +- .../parity_tests/getattr_hook_inline_deopt.py | 165 +++++++++++++++++ .../src/objspace/std/mapdict.rs | 67 +++++++ .../src/jitcode_dispatch/inline_call.rs | 168 ++++++++++++++++++ .../src/jitcode_dispatch/residual_call.rs | 16 ++ 5 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py diff --git a/pyre/bench/synth/getattr_hook_binding.py b/pyre/bench/synth/getattr_hook_binding.py index 0ed24321dbd..482c636d0a4 100644 --- a/pyre/bench/synth/getattr_hook_binding.py +++ b/pyre/bench/synth/getattr_hook_binding.py @@ -1,8 +1,14 @@ -# pyre-check: max-pypy-ratio=90 +# pyre-check: max-pypy-ratio=25 # objspace.py:710 get_and_call_function: a __getattr__ (or __getattribute__) # defined as a classmethod or staticmethod must be bound through __get__ before # being called, exactly like any other special method, so it receives the # arguments the descriptor protocol gives it. +# +# Each of the three accesses below used to cost one opaque residual holding the +# whole `object_getattr_miss` walk plus a fresh frame for the hook. Inlining +# the hook against the version-tag and map pins that make the miss constant +# dropped the ratio from 48.6x/59.5x (dynasm/wasm) to 7.4x/10.4x/8.7x +# (dynasm/cranelift/wasm); the bound is twice the slowest of those, rounded up. class ClassmethodGetattr: diff --git a/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py b/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py new file mode 100644 index 00000000000..6d8bcc47100 --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py @@ -0,0 +1,165 @@ +# CPython-suite gap: the suite exercises __getattr__ semantics but never runs a +# hooked access hot enough to be compiled, so nothing covers the compiled form. +# parity-tests reason: this targets the pyre-specific guards a compiled +# __getattr__ hook rests on. + +"""A compiled `__getattr__` hook answers to the two pins that admitted it. + +`objspace.py:710 get_and_call_function` reaches the hook only after the +attribute resolves nowhere, so the compiled form pins the receiver's type +version tag (the type keeps lacking the name, and keeps this hook) and the +instance map (the receiver keeps lacking the name). Each loop below runs long +enough to be compiled and then invalidates exactly one of those pins mid-loop: +the values recorded before and after must differ at the iteration the pin was +broken, which is what proves the guard deopts rather than the compiled answer +being reused. + +The AttributeError case is here for the same reason: a hook that raises for an +unknown name is an ordinary outcome of an inlined body, not a shape the fold may +quietly turn into a returned value. +""" + +N = 40000 +SWAP = N // 2 + + +class Instance: + def __getattr__(self, name): + return "hook:" + name + + +class Hooked: + @classmethod + def __getattr__(cls, name): + return "cm:%s:%s" % (cls.__name__, name) + + +class Static: + @staticmethod + def __getattr__(name): + return "sm:" + name + + +class Raiser: + def __getattr__(self, name): + if name == "absent": + raise AttributeError("no " + name) + return "ok:" + name + + +class Installer: + def __getattr__(self, name): + # The hook itself gives the instance the attribute, so every later + # access must read the instance rather than hook again. + self.installed = "real" + return "hook:" + name + + +def instance_shadow(): + """A store during the loop puts the name on the instance.""" + obj = Instance() + seen = [] + i = 0 + while i < N: + seen.append(obj.later) + if i == SWAP: + obj.later = "instance" + i += 1 + assert seen[0] == "hook:later", seen[0] + assert seen[SWAP] == "hook:later", seen[SWAP] + assert seen[SWAP + 1] == "instance", seen[SWAP + 1] + assert seen[-1] == "instance", seen[-1] + + +def hook_replaced(): + """Reassigning `__getattr__` bumps the type's version tag.""" + + class Swapped(Hooked): + pass + + obj = Swapped() + seen = [] + i = 0 + while i < N: + seen.append(obj.zed) + if i == SWAP: + Swapped.__getattr__ = classmethod(lambda cls, name: "replaced") + i += 1 + assert seen[0] == "cm:Swapped:zed", seen[0] + assert seen[SWAP + 1] == "replaced", seen[SWAP + 1] + + +def name_shadowed_on_type(): + """A class attribute added during the loop wins over the hook.""" + + class Shadowed(Static): + pass + + obj = Shadowed() + seen = [] + i = 0 + while i < N: + seen.append(obj.zed) + if i == SWAP: + Shadowed.zed = "class" + i += 1 + assert seen[0] == "sm:zed", seen[0] + assert seen[SWAP + 1] == "class", seen[SWAP + 1] + + +def bound_argument_follows_the_receiver_type(): + """A classmethod hook binds the receiver's own class, not the base.""" + + class Sub(Hooked): + pass + + base = Hooked() + sub = Sub() + i = 0 + while i < N: + assert base.q == "cm:Hooked:q" + assert sub.q == "cm:Sub:q" + i += 1 + + +def hook_raises(): + """An AttributeError out of the hook reaches the caller every iteration.""" + obj = Raiser() + hits = 0 + misses = 0 + i = 0 + while i < N: + hits += len(obj.present) + try: + obj.absent + except AttributeError as exc: + assert str(exc) == "no absent", exc + misses += 1 + i += 1 + assert hits == N * len("ok:present"), hits + assert misses == N, misses + + +def hook_installs_the_attribute(): + obj = Installer() + seen = [] + i = 0 + while i < N: + seen.append(obj.installed) + i += 1 + assert seen[0] == "hook:installed", seen[0] + assert seen[1] == "real", seen[1] + assert seen[-1] == "real", seen[-1] + + +def main(): + instance_shadow() + hook_replaced() + name_shadowed_on_type() + bound_argument_follows_the_receiver_type() + hook_raises() + hook_installs_the_attribute() + print("OK") + + +main() diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 2cdda23fa3c..a9c2634fd80 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -1760,6 +1760,73 @@ pub unsafe fn load_attr_fast_path( Some((w_type, version_tag, map, p.storageindex)) } +/// The miss twin of [`load_attr_fast_path`]: return the ingredients for +/// inlining the receiver type's `__getattr__` hook when `name` resolves +/// nowhere. +/// +/// `baseobjspace::instance_getattr_hook_or_err` is the tail this stands in for +/// (`descroperation.py:242-245`): once the descriptor protocol has produced an +/// AttributeError, the type's `__getattr__` is looked up and called with the +/// receiver and the name. Reaching that tail is what the two returned pins +/// prove, and both are guards the caller owes: +/// +/// * `version_tag` — the class lookup stays constant, so `name` keeps +/// resolving to nothing on the type and `__getattr__` keeps resolving to +/// the returned hook; +/// * `map` — the instance shape stays constant, so `name` keeps being absent +/// from this receiver's own storage. +/// +/// Together they make the whole `object_getattr_miss` walk a compile-time +/// answer, which is the work the fold removes; the hook itself is what the +/// caller then inlines. +/// +/// Returns `None` for every shape those two guards cannot cover: a non-mapdict +/// receiver, a custom `__getattribute__`, an uncacheable `version_tag`, a name +/// the type or the instance actually owns, or a type with no `__getattr__`. +/// +/// # Safety +/// `w_obj` must be a live object. +pub unsafe fn getattr_hook_fast_path( + w_obj: PyObjectRef, + name: &str, +) -> Option<(PyObjectRef, u64, MapRef, PyObjectRef)> { + // mapdict.py:1495 `if map is not None:` — also filters non-instances. + let map = unsafe { mapdict_map_or_null(w_obj) }; + if map.is_null() { + return None; + } + // mapdict.py:1496 `w_type = map.terminator.w_cls`. + let w_type = unsafe { (*(*map).terminator()).as_terminator() }.w_cls; + if w_type.is_null() { + return None; + } + // mapdict.py:1497-1499 — a custom `__getattribute__` runs its own lookup, + // which neither pin describes. + if unsafe { crate::baseobjspace::getattribute_if_not_from_object(w_type) }.is_some() { + return None; + } + // mapdict.py:1500-1501 `version_tag = w_type.version_tag(); if is not None:`. + let version_tag = unsafe { crate::baseobjspace::w_type_version_tag(w_type) }; + if version_tag == 0 { + return None; + } + // The miss itself. A type-level hit is refused before the map is consulted: + // `classify_attr` reads a `__slots__` member under the `"slot"` name rather + // than its own, so a descriptor found here says nothing about what + // `find_map_attr(name)` below would answer. + if unsafe { crate::baseobjspace::lookup_in_type_where(w_type, name) }.is_some() { + return None; + } + // `classify_attr(w_type, None, false)` answers `(DICT, false)` — the + // no-descriptor arm (mapdict.py:1509-1510) — so this is the same + // `find_map_attr` call the hit path makes, read for its absence. + if unsafe { find_map_attr(map, Wtf8::new(name), DICT) }.is_some() { + return None; + } + let w_getattr = unsafe { crate::baseobjspace::lookup_in_type_where(w_type, "__getattr__") }?; + Some((w_type, version_tag, map, w_getattr)) +} + /// The [`load_attr_fast_path`] twin for a receiver that keeps its attributes in /// a `newdict(instance=True)` dictionary rather than in header mapdict storage /// (`mapdict.py:1299-1303 make_instance_dict`). It applies the same diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 3059d98e4e8..a03ac3a7dbd 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -6189,6 +6189,174 @@ pub(crate) fn try_walker_inline_property_get( ) } +/// Inline the receiver type's `__getattr__` hook for an attribute the type and +/// the instance both lack — the miss twin of [`try_walker_inline_property_get`]. +/// +/// `descroperation.py:242-245` reaches the hook only after the descriptor +/// protocol has raised, so pyre runs the whole `object_getattr_miss` walk (the +/// `__dict__` / `__doc__` / `__class__` special names, the metaclass loops, the +/// terminal miss) and then a fresh interpreter frame for the hook, on every +/// access, behind one opaque `CALL_MAY_FORCE`. PyPy traces through all of it, +/// so the miss const-folds and only the hook body is left. +/// [`getattr_hook_fast_path`](pyre_interpreter::objspace::std::mapdict::getattr_hook_fast_path) +/// is the oracle for that fold: the version-tag and map pins it asks for are +/// what make the miss a compile-time answer. +/// +/// All three spellings `get_and_call_function` binds are folded, because the +/// version-tag pin makes the binding decision itself constant: a plain +/// `Function` takes `funccall(w_obj, w_name)`, a `classmethod` is entered with +/// the class the descriptor would bind, and a `staticmethod` with the name +/// alone. A custom-descriptor hook stays on the residual. +/// +/// The name argument is an interned immortal block rather than the fresh +/// `w_str_new` the residual path allocates per access, which is the shape +/// `pyopcode.py LOAD_ATTR` passes (`space.getattr(w_obj, w_name)` hands over +/// `co_names_w[oparg]`, one object for the life of the code object). +/// +/// A branching, raising body is admitted: a hook that raises `AttributeError` +/// for an unknown name is the shape worth inlining, not an edge case. Same +/// loop-header and top-frame restrictions as the sibling routes; every other +/// shape declines to the residual (SAFE — no acceleration, unchanged +/// semantics). +/// The leading argument `get_and_call_function` binds ahead of the attribute +/// name, one variant per descriptor spelling of a `__getattr__` hook. +enum HookLeading { + /// Plain `Function`: `funccall(w_obj, w_name)` leads with the receiver. + Receiver, + /// `ClassMethod.__get__` leads with the class. + Class, + /// `StaticMethod.__get__` binds nothing; the name is the only argument. + None, +} + +#[allow(clippy::too_many_arguments)] +pub(crate) fn try_walker_inline_getattr_hook( + ctx: &mut WalkContext<'_, '_, Sym>, + op: &DecodedOp, + code: &[u8], + r_args: &[OpRef], + call_descr: &dyn majit_ir::descr::CallDescr, + obj: OpRef, + w_code_ptr: usize, + name_idx: usize, + dst: usize, + dst_bank: char, +) -> Result, DispatchError> { + if !ctx.is_authoritative_executor || dst_bank != 'r' || ctx.fbw_mode.inline_subwalk { + return Ok(None); + } + let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { + return Ok(None); + }; + let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { + return Ok(None); + }; + let Some((w_type, version_tag, map, w_getattr)) = (unsafe { + pyre_interpreter::objspace::std::mapdict::getattr_hook_fast_path(concrete_obj, &name) + }) else { + return Ok(None); + }; + // `get_and_call_function` leads the positionals with the receiver only for + // a plain `Function`; every other descriptor is bound through `get` first + // and called with the name alone. The version-tag pin makes that binding + // decision a constant of the trace, so each spelling resolves to its own + // function and leading argument here instead of declining. + let (w_func, leading) = unsafe { + if pyre_object::function::is_classmethod(w_getattr) { + ( + pyre_object::function::w_classmethod_get_func(w_getattr), + HookLeading::Class, + ) + } else if pyre_object::function::is_staticmethod(w_getattr) { + ( + pyre_object::function::w_staticmethod_get_func(w_getattr), + HookLeading::None, + ) + } else { + (w_getattr, HookLeading::Receiver) + } + }; + if w_func.is_null() { + return Ok(None); + } + let Some((w_code, nparams, has_closure)) = (unsafe { resolve_inlinable_callee(w_func) }) else { + return Ok(None); + }; + // The name, plus the bound leading argument when the descriptor supplies + // one. Any other arity is a shape the call would reject before the body + // runs. + if nparams != usize::from(!matches!(leading, HookLeading::None)) + 1 { + return Ok(None); + } + // Decided once per callee on its jitcode payload; `None` means no body or + // descr pool, which this route declines on either way. + let Some(body_facts) = sub_jitcode_body_facts_for_code(w_code) else { + return Ok(None); + }; + if body_facts.owns_loop_header { + return Ok(None); + } + + // Both pins the oracle asked for, plus the layout guard its map read needs. + walker_guard_mapdict_instance_shape(ctx, op.pc, obj, concrete_obj, w_type, version_tag, map)?; + + let name_obj = + pyre_object::unicodeobject::box_str_constant(rustpython_wtf8::Wtf8::new(name.as_str())) + as pyre_object::PyObjectRef; + let name_const = ctx.trace_ctx.const_ref(name_obj as i64); + let leading_arg = match leading { + // The live receiver box: baking it would collapse instances that share + // this shape but not this identity. + HookLeading::Receiver => Some((obj, concrete_obj)), + // The class the `w_class` guard above already pinned. + HookLeading::Class => Some((ctx.trace_ctx.const_ref(w_type as i64), w_type)), + HookLeading::None => None, + }; + // `[__getattr__, , ?, name]`: the method-form + // call header the inline plumbing expects, then the positional args. + let mut arg_concretes = vec![ConcreteValue::Ref(w_func), ConcreteValue::Null]; + let mut callee_args = Vec::with_capacity(2); + let mut callee_arg_concretes = Vec::with_capacity(2); + if let Some((arg, concrete)) = leading_arg { + arg_concretes.push(ConcreteValue::Ref(concrete)); + callee_args.push(arg); + callee_arg_concretes.push(ConcreteValue::Ref(concrete)); + } + arg_concretes.push(ConcreteValue::Ref(name_obj)); + callee_args.push(name_const); + callee_arg_concretes.push(ConcreteValue::Ref(name_obj)); + let getattr_const = ctx.trace_ctx.const_ref(w_func as i64); + try_walker_inline_resolved_user_call( + ctx, + op, + code, + getattr_const, + r_args, + call_descr, + 'r', + dst, + w_func, + getattr_const, + w_func, + arg_concretes, + callee_args, + callee_arg_concretes, + true, + None, + w_code, + nparams, + has_closure, + // The class and version pins are already emitted above, alongside the + // map pin this route additionally owes. + None, + None, + // The same LOAD_ATTR entry [`try_walker_inline_property_get`] admits. + true, + false, + None, + ) +} + /// Inline a `property` setter store (`obj.value = x`) after the plain-attribute /// mapdict store fold declines because the attribute is a data descriptor — the /// setter twin of [`try_walker_inline_property_get`]. Pin the receiver class + diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index 80c4d809108..5d5f64effff 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -6706,6 +6706,22 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( )? { return Ok(inlined); } + // The name resolves nowhere and the type defines `__getattr__`: + // inline the hook in place of the miss walk plus its frame. + if let Some(inlined) = try_walker_inline_getattr_hook( + ctx, + op, + code, + &r_args, + call_descr, + obj_opref, + w_code_ptr, + namei as usize, + dst, + dst_bank, + )? { + return Ok(inlined); + } // A type receiver whose class-MRO value needs no descriptor // binding folds to that value under receiver + version pins. if spec_gate("load_type_attr", || { From 56a9aecc313ed29078be7e8cb72383e43f81f96d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 17 Aug 2026 20:47:24 +0900 Subject: [PATCH 03/24] type call inline: admit the call_fn spelling's PY_NULL receiver slot `try_walker_inline_type_call` read any concrete shadow in `r_args[1]` as a populated receiver. `call_fn` fills that slot with the checked `PY_NULL` sentinel rather than leaving it empty the way `call_kw` does, so every ordinary `C(...)` declined and the emit only ever ran for the `call_kw` spelling. The gate now rejects the slot only when it holds something that is neither null nor `PY_NULL`. Four bails that returned `Ok(None)` silently now report through `type_call_decline`, and the `is_authoritative_executor` / `inline_subwalk` / `dst_bank` pre-filter names its reason as well, resolving the callable for that only while the reasons are being collected. `[inline-entry]` prints `dst_bank`. `type_call_diag_enabled` is split out of `type_call_decline`, and both helpers move above `try_walker_inline_type_call`'s doc comment and its `#[allow(clippy::too_many_arguments)]`, which an earlier insertion had left attached to `type_call_decline`. The `str(exc)` / `repr(exc)` summary line at the head of that doc comment moves to `try_walker_inline_exception_string_override`, which it describes. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 67 ++++++++++++++----- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index a03ac3a7dbd..0ca2ddfdad3 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -2007,7 +2007,7 @@ pub(crate) fn try_walker_inline_user_call( } if fbw_inline_diag_enabled() { eprintln!( - "[inline-entry] pc={} helper={:?} nrefargs={} subwalk={}", + "[inline-entry] pc={} helper={:?} nrefargs={} subwalk={} dst_bank={dst_bank}", op.pc, pyre_helper, r_args.len(), @@ -5500,7 +5500,19 @@ fn try_walker_inline_resolved_user_call_inner( } } -/// Route `str(exc)` / `repr(exc)` through an app-level exception override. +/// Whether the instantiation emit's decline reasons are being collected. +fn type_call_diag_enabled() -> bool { + std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() +} + +/// Report why the instantiation emit declined, under `PYRE_FBW_INLINE_DIAG`. +fn type_call_decline(reason: &str) -> Result, DispatchError> { + if type_call_diag_enabled() { + eprintln!("[type-call-decline] {reason}"); + } + Ok(None) +} + /// Instantiate a user-defined class inside the trace instead of leaving `P()` /// an opaque `bh_call_fn` residual that re-enters `type_descr_call_impl`, /// `object.__new__` and an interpreted `__init__` frame every iteration. @@ -5518,14 +5530,6 @@ fn try_walker_inline_resolved_user_call_inner( /// `new_with_vtable` is a virtual, so a constructor whose result never escapes /// the loop optimizes away entirely, as it does upstream. #[allow(clippy::too_many_arguments)] -/// Report why the instantiation emit declined, under `PYRE_FBW_INLINE_DIAG`. -fn type_call_decline(reason: &str) -> Result, DispatchError> { - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { - eprintln!("[type-call-decline] {reason}"); - } - Ok(None) -} - pub(crate) fn try_walker_inline_type_call( ctx: &mut WalkContext<'_, '_, Sym>, op: &DecodedOp, @@ -5537,15 +5541,44 @@ pub(crate) fn try_walker_inline_type_call( dst: usize, ) -> Result, DispatchError> { if !ctx.is_authoritative_executor || ctx.fbw_mode.inline_subwalk || dst_bank != 'r' { + // These three reject far more calls than the instantiations this emit is + // about, so name the reason only for a call that does resolve to a + // class, and only while the reasons are being collected — the extra + // resolution below is diagnostic cost, not tracing cost. + if type_call_diag_enabled() + && r_args.len() >= 2 + && walker_concrete_ref_object(ctx, r_args[1]).is_none() + && walker_concrete_ref_object(ctx, r_args[0]) + .is_some_and(|w_type| unsafe { pyre_object::is_type(w_type) }) + { + return type_call_decline(if !ctx.is_authoritative_executor { + "not the authoritative executor" + } else if ctx.fbw_mode.inline_subwalk { + "inline sub-walk" + } else { + "destination is not a ref register" + }); + } return Ok(None); } // `[callable, null_or_self, args...]`. A method-form call (`null_or_self` // populated) never names a class as its callable. - if r_args.len() < 2 || walker_concrete_ref_object(ctx, r_args[1]).is_some() { + if r_args.len() < 2 { return Ok(None); } + // "No receiver" has two spellings in that slot: `call_kw` leaves it with no + // concrete shadow at all, `call_fn` fills it with the checked `PY_NULL` + // sentinel. Reading a present shadow as a receiver rejects the whole + // `call_fn` spelling, which is the one an ordinary `C(...)` lowers to. + if walker_concrete_ref_object(ctx, r_args[1]) + .is_some_and(|null_or_self| !null_or_self.is_null() && null_or_self != pyre_object::PY_NULL) + { + return type_call_decline("receiver slot is populated"); + } let Some(w_type) = walker_concrete_ref_object(ctx, r_args[0]) else { - return Ok(None); + // Whether this even was an instantiation is unknowable without the + // callable, so the reason is reported as the open question it is. + return type_call_decline("callable is not a concrete ref"); }; if !unsafe { pyre_object::is_type(w_type) } { return Ok(None); @@ -5586,7 +5619,7 @@ pub(crate) fn try_walker_inline_type_call( } let w_object = pyre_interpreter::typedef::w_object(); if w_object.is_null() { - return Ok(None); + return type_call_decline("object type unavailable"); } // Only `object.__new__` allocates the plain `[ob_type | w_class | map | // storage]` instance this emit builds; any other `__new__` picks its own @@ -5616,9 +5649,9 @@ pub(crate) fn try_walker_inline_type_call( let mut arg_concretes = vec![ConcreteValue::Ref(w_type), ConcreteValue::Null]; let mut callee_arg_concretes = Vec::with_capacity(r_args.len() - 1); - for &arg in &r_args[2..] { + for (i, &arg) in r_args[2..].iter().enumerate() { let Some(concrete) = walker_concrete_ref_object(ctx, arg) else { - return Ok(None); + return type_call_decline(&format!("argument {i} is not a concrete ref")); }; arg_concretes.push(ConcreteValue::Ref(concrete)); callee_arg_concretes.push(ConcreteValue::Ref(concrete)); @@ -5660,7 +5693,7 @@ pub(crate) fn try_walker_inline_type_call( instance, &pyre_object::pyobject::INSTANCE_TYPE as *const _ as i64, ); - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if type_call_diag_enabled() { eprintln!( "[type-call-inline] pc={} class={} init={}", op.pc, @@ -5719,6 +5752,8 @@ pub(crate) fn try_walker_inline_type_call( Ok(inlined) } +/// Route `str(exc)` / `repr(exc)` through an app-level exception override. +/// /// Pyre's exact `str` type call follows `str_descr_new` → `builtin_str` → /// `exc_user_dunder_obj`; the builtin `repr` follows `builtin_repr` → /// `py_repr_obj`. Both paths look up the receiver dunder before builtin From 16872aaff12890f46aa7314647bdbec74a840b70 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 17 Aug 2026 22:36:36 +0900 Subject: [PATCH 04/24] inline diag: list the scanned callee body beside a replay-dirty verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `pc` a `[replay-dirty]` line names is an offset into the callee's jitcode, and no per-function dump covers a callee — `PYRE_DUMP_PERFN_JITCODE` emits only portal frames — so the number could not be matched against any op. `PYRE_FBW_REPLAY_DIRTY_BODY=1`, under the existing `PYRE_FBW_INLINE_DIAG`, lists each body as `fbw_callee_body_replay_safety` scans it, so the verdict line that follows a listing names an op within it. Residual calls carry the helper kind the verdicts turn on, since the opname alone does not separate a deferred `call_fn` from an untagged helper that declines the whole body. Assisted-by: Claude --- .../src/jitcode_dispatch/fbw_state.rs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index 53309ccc351..655c75af2c8 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -2366,6 +2366,41 @@ macro_rules! replay_dirty { }}; } +/// The `pc` a `[replay-dirty]` line names is an offset into the scanned +/// callee's jitcode, and no per-function dump covers a callee — so on its own +/// the number cannot be matched against any op. `PYRE_FBW_REPLAY_DIRTY_BODY=1` +/// lists each body as it is scanned, so the verdict line that follows a listing +/// names an op within it. +fn replay_safety_dump_body(body_code: &[u8], callee_descr_refs: &[DescrRef]) { + if !fbw_inline_diag_enabled() || std::env::var_os("PYRE_FBW_REPLAY_DIRTY_BODY").is_none() { + return; + } + eprintln!( + "[replay-dirty-body] === scanning body len={} ===", + body_code.len() + ); + for d in crate::jitcode_runtime::decoded_ops(body_code) { + // Every residual verdict below turns on the helper kind, and the opname + // alone does not separate a deferred `call_fn` from an untagged helper + // that declines the whole body — so name it. + let helper = if d.opname.starts_with("residual_call") { + residual_call_descr_index_in_body(body_code, &d) + .and_then(|i| callee_descr_refs.get(i)) + .and_then(|descr| descr.as_call_descr()) + .map_or_else( + || " helper=".to_string(), + |cd| format!(" helper={:?}", cd.get_extra_info().pyre_helper), + ) + } else { + String::new() + }; + eprintln!( + "[replay-dirty-body] pc={:>4} {}/{}{}", + d.pc, d.opname, d.argcodes, helper + ); + } +} + pub(crate) fn fbw_callee_body_replay_safety( body_code: &[u8], exact_numeric_args: &[ExactNumericArg], @@ -2376,6 +2411,7 @@ pub(crate) fn fbw_callee_body_replay_safety( callee_descr_refs: &[DescrRef], method_form_deferred_helpers: bool, ) -> CalleeReplaySafety { + replay_safety_dump_body(body_code, callee_descr_refs); let Some(branch_targets) = body_branch_targets(body_code) else { replay_dirty!("BranchTargetsUndecodable", 0, "-"); }; @@ -2547,6 +2583,12 @@ pub(crate) fn fbw_callee_body_replay_safety( // reads the same value again. Its writing twin // `SetCurrentException` is not here — it is journalled, and so // reaches the `deferred_call` arm below instead. + // `load_deref` is that same shape once more, and it is the one every + // closure body carries: `bh_load_deref_value_fn` dereferences a cell + // and returns its contents, writing nothing, so a replay reads the + // same cell again. Its raise on an unbound free variable is no + // barrier — `load_global` above raises `NameError` too, and a replay + // raises the same one. Its writing twin `StoreDeref` is not here. let replay_safe_read = matches!( ei.pyre_helper, majit_ir::PyreHelperKind::LoadConst From 54d71dd2a1b31a57092b231ce1ff143bba7a5fe8 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 00:24:53 +0900 Subject: [PATCH 05/24] _abc: port the positive/negative subclass caches `app_abc.py:15-44` defines `SimpleWeakSet` and `_abc_init` installs an `_abc_cache`, an `_abc_negative_cache` and an `_abc_negative_cache_version` alongside the registry; `_abc_subclasscheck` consults both before running the subclass hook, the registry walk and the `__subclasses__` walk, and records its verdict in the matching one. This module had only the registry and the invalidation counter. - `abc_init` installs the two caches and the version, per class for the same reason the registry is per class. - `subclass_of` checks the positive cache, then discards the negative cache when the counter has moved past its recorded version and otherwise consults it, and records the verdict at a single site after the walks. - The cache entries are `_weakref.ref` objects reached through `get_or_make_weakref`, and membership goes through `space.contains_w` and the set's own `add` rather than the raw set primitives: a weakref hashes by running interpreter-level code, which is the case those primitives document as the caller's to pre-hash. - `_reset_caches` clears both caches instead of bumping the counter, which only `_abc_register` does (`app_abc.py:100-101, 188-191`). Not ported: the `ref()` callback upstream's `add` passes, so a spent weakref stays a member of the set it was recorded in. `isinstance(1, numbers.Rational)` over 64000 iterations: 0.1855s -> 0.0518s. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_abc/mod.rs | 382 ++++++++++++++----- 1 file changed, 287 insertions(+), 95 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index 847c9353150..cc46d6e585a 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -4,19 +4,135 @@ //! `_abc_subclasscheck` walk `__mro__` for direct inheritance and the //! per-class `_abc_registry` list populated by `_abc_register` for //! virtual subclasses. Mirrors `pypy/module/_abc/app_abc.py`'s -//! `_abc_register` / `_abc_subclasscheck` flow (registry-based virtual -//! lookups, no negative cache). +//! `_abc_register` / `_abc_subclasscheck` flow, including its +//! positive/negative caches: without them every check that is not a direct +//! `__mro__` hit re-runs the subclass hook, the registry walk and the +//! `__subclasses__` walk, all recursively, on every single call. use pyre_object::*; use std::sync::atomic::{AtomicU64, Ordering}; -// `abc_invalidation_counter` (`_abcmodule.c`): bumped by every successful -// `_abc_register` and by `_reset_caches`, and read by `get_cache_token`. -// The positive/negative object caches themselves remain omitted as an -// optimisation — only this token is tracked, so a bump makes any cached -// token stale. +// `abc_invalidation_counter` (`app_abc.py:47`): bumped by every successful +// `_abc_register` — and by nothing else — and read by `get_cache_token`. A +// negative cache recorded before a bump no longer describes the registry, so +// `_abc_negative_cache_version` is compared against this on every check. static INVALIDATION_COUNTER: AtomicU64 = AtomicU64::new(0); +/// `ref(cls)` as `SimpleWeakSet` spells it (`app_abc.py:20`). This is the +/// interpreter-level `weakref.ref` object, not [`weakref::w_weakref_new`]'s +/// bare GC struct: the caches are ordinary sets, so an entry has to be a real +/// object with a type — one whose `__hash__` and `__eq__` go by referent, which +/// is what lets a probe find an entry recorded earlier. +/// +/// `get_or_make_weakref` returns the one weakref a class already has, so only +/// the first probe of a given class allocates. +/// +/// `None` for a class that cannot be weak-referenced at all; the caller reads +/// that as "not cacheable" rather than raising, since the answer to the +/// subclass question does not depend on whether it can be remembered. +fn class_weakref(cls: PyObjectRef) -> Option { + use crate::module::_weakref::interp__weakref as wr; + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + // Both calls below allocate, and an allocation can claim root slots of its + // own — so every slot index comes back from the `publish` that made it + // rather than from arithmetic on an earlier one, and every argument is read + // out of its slot rather than from a local copy. + let type_slot = roots.publish(&[wr::weakref_type()]); + let lifeline = wr::getlifeline(roots.get(cls_slot)).ok()?; + let lifeline_slot = roots.publish(&[lifeline]); + Some(wr::get_or_make_weakref( + roots.get(lifeline_slot), + roots.get(type_slot), + roots.get(cls_slot), + )) +} + +/// `app_abc.py:33-38 SimpleWeakSet.__contains__` — `ref(item) in self.data`, a +/// set whose members are weakrefs, probed with a weakref to the same class. +/// +/// A missing or non-set attribute reads as "not cached" rather than raising: +/// `_abc_init` installs both caches, but an ABC built before this module (a +/// pickled class, a hand-rolled `ABCMeta` subclass that skips `_abc_init`) +/// has neither, and such a class must still answer subclass checks. +/// +/// Takes the class and the attribute name rather than the set itself, so that +/// the set is read only after the probe exists: making the probe allocates, and +/// a reference read across an allocation names where the object used to be. +/// +/// `wr in self.data` goes through the membership protocol rather than +/// [`w_set_contains`]: a weakref hashes by running interpreter-level code +/// (`_weakref.ref.__hash__` hashes the referent and memoises the result), and +/// the raw set primitives document that a caller whose element hashes that way +/// owes them a digest taken while the operands are still rooted. +fn weak_cache_contains( + cls: PyObjectRef, + name: &str, + item: PyObjectRef, +) -> Result { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let Some(probe) = class_weakref(item) else { + return Ok(false); + }; + let probe_slot = roots.publish(&[probe]); + let cache = cache_attr(roots.get(cls_slot), name); + if cache.is_null() || !unsafe { is_set(cache) } { + return Ok(false); + } + crate::baseobjspace::contains(cache, roots.get(probe_slot)) +} + +/// `app_abc.py:39-40 SimpleWeakSet.add` — `self.data.add(ref(item))`. Silently +/// declines a class with no cache slot, for the same reason +/// [`weak_cache_contains`] reads one as a miss, and calls the set's own `add` +/// for the same reason it uses the membership protocol. +/// +/// Upstream's `add` passes `ref()` a callback that discards the entry once the +/// referent dies; this does not, so a checked class that is later collected +/// leaves its spent weakref behind as a member. Such an entry answers no +/// probe — a weakref compares by referent and this one has none — and it keeps +/// no class alive, so what it costs is the weakref itself. +fn weak_cache_add(cls: PyObjectRef, name: &str, item: PyObjectRef) -> Result<(), crate::PyError> { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let Some(entry) = class_weakref(item) else { + return Ok(()); + }; + let entry_slot = roots.publish(&[entry]); + let cache = cache_attr(roots.get(cls_slot), name); + if cache.is_null() || !unsafe { is_set(cache) } { + return Ok(()); + } + let add = crate::baseobjspace::getattr_str(cache, "add")?; + let add_slot = roots.publish(&[add]); + crate::call::call_function_impl_result(roots.get(add_slot), &[roots.get(entry_slot)])?; + Ok(()) +} + +/// The named cache attribute of `cls`, or null when it has none. Read fresh +/// at every use: the walks between two reads run arbitrary Python, which can +/// rebind the attribute and can move the set. +fn cache_attr(cls: PyObjectRef, name: &str) -> PyObjectRef { + match crate::baseobjspace::getattr_str(cls, name) { + Ok(cache) => cache, + Err(_) => std::ptr::null_mut(), + } +} + +/// The registry generation `cls`'s negative cache was recorded against. A +/// class with no version attribute, or one holding something other than an +/// `int`, reports generation 0, which is below every counter value a +/// registration produces — so its negative cache is discarded rather than +/// trusted. +fn negative_cache_version(cls: PyObjectRef) -> u64 { + let version = cache_attr(cls, "_abc_negative_cache_version"); + if version.is_null() || !unsafe { is_int(version) } { + return 0; + } + unsafe { w_int_get_value(version) }.max(0) as u64 +} + // `_py_abc.ABCMeta.__new__` (`_py_abc.py:48`) gives every ABC its OWN // `_abc_registry`. Create it here as a per-class list so the registry is not // inherited: without an own entry `register`/`subclass_of` would resolve @@ -27,6 +143,17 @@ fn abc_init(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { let fresh = w_list_new(vec![]); crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; + // `app_abc.py:75-77` — the caches are per-class for the same reason the + // registry is: resolved up the MRO, one base's caches would answer for + // every descendant ABC, and a hit on `Rational` would satisfy + // `Integral`. Each value is built before the call that stores it, so + // that no allocation happens between reading `cls` and using it. + let cache = w_set_new(); + crate::baseobjspace::setattr_str(cls, "_abc_cache", cache)?; + let negative_cache = w_set_new(); + crate::baseobjspace::setattr_str(cls, "_abc_negative_cache", negative_cache)?; + let version = w_int_new(INVALIDATION_COUNTER.load(Ordering::Relaxed) as i64); + crate::baseobjspace::setattr_str(cls, "_abc_negative_cache_version", version)?; let mut abstract_names = Vec::new(); let bases = unsafe { w_type_get_bases(cls) }; if !bases.is_null() && unsafe { is_tuple(bases) } { @@ -118,7 +245,10 @@ fn register(args: &[PyObjectRef]) -> Result { unsafe { w_list_append(registry, subclass); } - // Invalidate any outstanding cache token. + // `app_abc.py:100-101` — invalidate every negative cache. A class this + // registration now makes a subclass may already be recorded as a non-match + // somewhere, and only the counter can reach those entries: they live on + // arbitrary other ABCs, not on `cls`. INVALIDATION_COUNTER.fetch_add(1, Ordering::Relaxed); // `app_abc.py:102-105` — an ABC that carries a structural-match marker // hands it to the registered class and its descendants @@ -161,11 +291,14 @@ fn set_collection_flag_recursive(w_type: PyObjectRef, flag: u8) { } } -// `_py_abc.ABCMeta.__subclasscheck__` (`_py_abc.py:108-147`): the subclass -// hook first, then a direct `__mro__` test, then the recursive registry and -// subclass walks. The positive/negative caches are a pure optimisation and -// are omitted; `issubclass` re-dispatches through `__subclasscheck__` so a -// registered or descendant ABC applies its own hook in turn. +// `_py_abc.ABCMeta.__subclasscheck__` (`_py_abc.py:108-147`): the caches +// first, then the subclass hook, then a direct `__mro__` test, then the +// recursive registry and subclass walks. `issubclass` re-dispatches through +// `__subclasscheck__` so a registered or descendant ABC applies its own hook +// in turn — which is also why the caches are load-bearing rather than a +// refinement: an uncached miss re-runs all three walks at every level of that +// recursion, so one `isinstance` against a deep ABC costs a walk of the whole +// ABC graph. fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result { // _py_abc.py:110-111 — `if not isinstance(subclass, type): raise // TypeError('issubclass() arg 1 must be a class')`. The `__mro__`/registry @@ -187,94 +320,136 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result scls, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, - Err(err) => return Err(err), - }; - let item_roots = pyre_object::gc_roots::push_roots(); - let scls_slot = item_roots.base(); - item_roots.pin_root(scls); - if crate::baseobjspace::issubclass(roots.get(subclass_slot), item_roots.get(scls_slot))? { - return Ok(true); + // _py_abc.py:140-144 — `for scls in cls.__subclasses__():`. This must go + // through normal attribute lookup, call, and iteration. Reading the + // internal type subclass vector directly hides user overrides and their + // TypeError/custom exceptions, which are observable ABCMeta semantics. + let subclasses_method = + crate::baseobjspace::getattr_str(roots.get(cls_slot), "__subclasses__")?; + let walk_roots = pyre_object::gc_roots::push_roots(); + let method_slot = walk_roots.base(); + walk_roots.pin_root(subclasses_method); + let subclasses = crate::call::call_function_impl_result(walk_roots.get(method_slot), &[])?; + let subclasses_slot = method_slot + 1; + walk_roots.pin_root(subclasses); + let iterator = crate::baseobjspace::iter(walk_roots.get(subclasses_slot))?; + let iterator_slot = subclasses_slot + 1; + walk_roots.pin_root(iterator); + loop { + let scls = match crate::baseobjspace::next(walk_roots.get(iterator_slot)) { + Ok(scls) => scls, + Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) => return Err(err), + }; + let item_roots = pyre_object::gc_roots::push_roots(); + let scls_slot = item_roots.base(); + item_roots.pin_root(scls); + if crate::baseobjspace::issubclass(roots.get(subclass_slot), item_roots.get(scls_slot))? + { + break 'decide true; + } } - } - Ok(false) + false + }; + + // `app_abc.py:144-163` records at each of its own arms; one site here covers + // all of them. + let recorded = if verdict { + "_abc_cache" + } else { + "_abc_negative_cache" + }; + weak_cache_add(roots.get(cls_slot), recorded, roots.get(subclass_slot))?; + Ok(verdict) } fn instancecheck(args: &[PyObjectRef]) -> Result { @@ -316,6 +491,25 @@ fn reset_registry(args: &[PyObjectRef]) -> Result { Ok(w_none()) } +/// `_abc._reset_caches(cls)` (`app_abc.py:188-191`): empty both of this ABC's +/// caches, leaving the registry and the invalidation counter untouched — a +/// cleared cache is answered by re-running the walks, which is not a change of +/// answer, so no token needs to expire. +/// +/// Cleared in place rather than rebound, so anything already holding the set +/// sees the clear. +fn reset_caches(args: &[PyObjectRef]) -> Result { + if let Some(&cls) = args.first() { + for name in ["_abc_cache", "_abc_negative_cache"] { + let cache = cache_attr(cls, name); + if !cache.is_null() && unsafe { is_set(cache) } { + unsafe { w_set_clear(cache) }; + } + } + } + Ok(w_none()) +} + crate::py_module! { "_abc", functions: { @@ -326,8 +520,6 @@ crate::py_module! { "_abc_subclasscheck" / 2 = subclasscheck, "_get_dump" / 1 = |_| Ok(w_tuple_new(vec![])), "_reset_registry" / 1 = reset_registry, - // Pyre keeps no object caches to clear; bumping the token invalidates - // any outstanding `get_cache_token` value. - "_reset_caches" / 1 = |_| { INVALIDATION_COUNTER.fetch_add(1, Ordering::Relaxed); Ok(w_none()) }, + "_reset_caches" / 1 = reset_caches, }, } From 325218cdf98c800864b2b0f2215e3b29f207a3f7 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 07:35:13 +0900 Subject: [PATCH 06/24] type-call diag: report the fold that gets rewound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `[type-call-inline]` is emitted before the `__init__` sub-walk, because the class and the `init` shape are what it names and both are known there. When that sub-walk declines, `inline_call.rs` cuts the trace back to `pre_fold_pos` and the instantiation stays a residual, with no line saying so — the diagnostic reads as a completed fold on a trace that carries none. Print `[type-call-rewind]` beside the `cut_trace`. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 0ca2ddfdad3..0a9a2f05bb6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -5746,6 +5746,19 @@ pub(crate) fn try_walker_inline_type_call( Some((instance, ConcreteValue::Ref(concrete_instance))), )?; if inlined.is_none() { + // The `[type-call-inline]` line above is printed before this sub-walk is + // attempted, because that is where the class and the `init` shape are + // known — so on its own it reports that the fold *began*, not that it + // stood. Say so when it does not: without this line the diagnostic + // reads as a successful fold on a trace that ends up carrying the whole + // instantiation as a residual. + if type_call_diag_enabled() { + eprintln!( + "[type-call-rewind] pc={} class={} why=__init__ sub-walk declined", + op.pc, + unsafe { pyre_object::w_type_get_name(w_type) }, + ); + } ctx.trace_ctx.cut_trace(pre_fold_pos); ctx.trace_ctx.heap_cache_mut().reset(); } From 77aefd2a0cf8371a246e092f698e7d56eb286c94 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 08:18:37 +0900 Subject: [PATCH 07/24] abort resume: restrict the residual-operand stack image to CALL-family entries `reconstructed_all_ref_call_stack` reads the aborting residual's Ref operand list as the caller's Python operand stack at the entry pc. That identity holds for the CALL-family helpers, whose list is `[callable, null_or_self, args...]`. It does not hold for the other entries the inline lever serves: `load_attr_fn(obj, code, name_idx)` encodes `r_args = [obj, code]` and `store_attr_fn` encodes `[obj, value, code]`, whose `code` operand is a code object that was never on the Python stack. Publishing that list resumed the interpreter with the code object in the receiver slot. `__getattr__`-hook and `property`-getter folds both enter from LOAD_ATTR, so a sub-walk abort inside either raised `AttributeError: 'code' object has no attribute ` for a name the descriptor answers, under a FOR_ITER caller at N above the trace threshold. Take the residual's helper kind and decline outside `CallFn` / `CallKw` / `CallFunctionEx`; the non-CALL entries source their operand image from the per-slot resume sources instead. Adds the regression test for both LOAD_ATTR routes. Assisted-by: Claude --- .../load_attr_inline_abort_operand_stack.py | 59 +++++++++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 30 +++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py diff --git a/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py b/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py new file mode 100644 index 00000000000..ddb7c03faac --- /dev/null +++ b/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py @@ -0,0 +1,59 @@ +# CPython-suite gap: no suite test resumes a JIT frame at a LOAD_ATTR whose +# inlined descriptor body aborted. +# parity-tests reason: this is a pyre trace-abort operand-stack regression. + +# The operand stack an aborted inline sub-walk hands back to the interpreter, +# for an inline entered from LOAD_ATTR rather than from CALL. +# +# A `__getattr__` hook and a `property` getter are both inlined in place of the +# attribute residual, so both enter the inline lever from LOAD_ATTR. When the +# sub-walk gives up, the caller's frame is flushed at that opcode and the +# interpreter re-executes it, which means the flush has to rebuild the operand +# stack the LOAD_ATTR pops. One of the sources it rebuilds from is the encoded +# residual's Ref operand list. For a CALL that list is exactly the stack image +# (`[callable, null_or_self, args...]`); for `load_attr_fn(obj, code, name_idx)` +# it is `[obj, code]`, whose `code` is a code object the Python stack never +# held. Publishing it resumed the LOAD_ATTR with the code object as receiver: +# `AttributeError: 'code' object has no attribute 'missing'` for an attribute +# the hook answers. +# +# Reaching the abort needs all three of: a FOR_ITER caller (the same body under +# `while` is admitted through an arm that does not abort), a body admitted as +# deferred-call safe, and a residual inside that body which does not inline — +# the string concatenations below. + +N = 3000 + + +class HookOwner: + def __getattr__(self, name): + return "hook:" + name + + +class PropertyOwner: + def __init__(self): + self._value = "v" + + @property + def value(self): + return "prop:" + self._value + + +def read_hook(owner): + last = None + for _ in range(N): + last = owner.missing + return last + + +def read_property(owner): + last = None + for _ in range(N): + last = owner.value + return last + + +assert read_hook(HookOwner()) == "hook:missing" +assert read_property(PropertyOwner()) == "prop:v" + +print("OK") diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 0a9a2f05bb6..2a529f00bf6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -1587,11 +1587,32 @@ pub(crate) fn emit_walker_loop_callee_call_assembler( /// pure-leaf callee resume to the caller's CALL boundary via the inherited /// single-frame snapshot (`entry_py_pc` / `outer_active_boxes`), which is /// sound for side-effect-free leaves (re-execute the whole call on deopt). +/// +/// That layout is a property of the CALL-family helpers alone, so the residual +/// this reads from has to be one of them. Every other entry the inline lever +/// serves passes its own receiver-plus-metadata list, not an operand-stack +/// image: `load_attr_fn(obj, code, name_idx)` is `r_args = [obj, code]` and +/// `store_attr_fn` is `[obj, value, code]`, whose `code` operand is a code +/// object the Python stack never held. Publishing it as a stack slot resumes +/// the interpreter with the code object where the receiver belongs — the +/// `__getattr__`/`property` folds returned `AttributeError: 'code' object has +/// no attribute ` for an attribute their own hook answers. Decline for +/// those instead; their operand image comes from the per-slot resume sources +/// ([`reconstructed_call_stack_from_resume_sources`]). pub(crate) fn reconstructed_all_ref_call_stack( code: &[u8], op: &DecodedOp, ctx: &WalkContext<'_, '_, Sym>, + call_descr: &dyn majit_ir::descr::CallDescr, ) -> Option> { + if !matches!( + call_descr.get_extra_info().pyre_helper, + majit_ir::PyreHelperKind::CallFn + | majit_ir::PyreHelperKind::CallKw + | majit_ir::PyreHelperKind::CallFunctionEx + ) { + return None; + } // The Ref list is NOT at a fixed offset: the method-form `CALL` helpers // this leg latches for lower through the mixed `iIRd>r` shape, whose // leading Int list shifts it (`dispatch_residual_call_iIRd_kind` reads it @@ -3002,6 +3023,7 @@ fn latch_abort_call_resume( code: &[u8], op: &DecodedOp, ctx: &WalkContext<'_, '_, Sym>, + call_descr: &dyn majit_ir::descr::CallDescr, is_top_inline: bool, unjournaled_before_subwalk: bool, executed_effects_before: usize, @@ -3016,7 +3038,7 @@ fn latch_abort_call_resume( let Some((outer_jitcode_index, call_jitcode_pc)) = abort_flush_call_jitcode_coord else { return; }; - if let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx) { + if let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx, call_descr) { fbw_set_abort_call_resume(outer_jitcode_index, call_jitcode_pc, stack); } } @@ -5284,6 +5306,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5314,6 +5337,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5357,6 +5381,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5385,6 +5410,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5468,7 +5494,7 @@ fn try_walker_inline_resolved_user_call_inner( // while the fallback rewind remains provably disabled. if let Some((outer_jitcode_index, call_jitcode_pc)) = abort_flush_call_jitcode_coord && let Some(stack) = - reconstructed_all_ref_call_stack(code, op, ctx).or_else(|| { + reconstructed_all_ref_call_stack(code, op, ctx, call_descr).or_else(|| { reconstructed_call_stack_from_resume_sources(ctx, call_jitcode_pc) }) { From 2c59d76c752a3dc44327b13dc2655843b49caeb0 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 08:45:27 +0900 Subject: [PATCH 08/24] getattr hook inline: test the descriptor type exactly and guard w_function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fold resolves a `__getattr__` hook's descriptor spelling at record time and unwraps `classmethod` / `staticmethod` to the callable inside, in place of invoking `__get__`. Two things it assumed: `is_classmethod` / `is_staticmethod` are `py_type_check`, an `ob_type` compare, where `descroperation.py:169-187 get_and_call_function` takes the descriptor shortcut only on the exact type and routes every other one through `space.get`. A `classmethod` subclass overriding `__get__` was unwrapped as if it were the base, so the compiled trace called the wrapped function while the interpreter called the override. Adds `is_exact_classmethod` / `is_exact_staticmethod`, which compare `w_class` as `is_exact_tuple` does — `classmethod_descr_new` calls `store_subclass_tag` only for a subclass, so that word separates them. `w_function` was baked as a constant, but `function.py:673` and `:720` mark it `_immutable_fields_ = ['w_function?']`; the `?` registers an invalidation pyre's setters do not force, and re-initialising an installed descriptor changes no type's version tag, which was the fold's only pin over it. Adds descr groups for both wrappers and reads the slot live behind a `GuardValue`, the stand-in `FUNCTION_DESCR_GROUP` already documents for `code?`. Both showed as a stale answer from the compiled trace only: `PYRE_JIT=0` and CPython 3.14 agree with each other. Adds the regression test, and states the rounding in two bench ratio rationales that read as exact arithmetic. Assisted-by: Claude --- pyre/bench/synth/getattr_hook_binding.py | 3 +- pyre/bench/synth/inlined_helper_mutation.py | 3 +- .../getattr_hook_descriptor_binding.py | 88 +++++++++++++++++++ pyre/pyre-jit-trace/src/descr.rs | 88 +++++++++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 60 ++++++++++--- pyre/pyre-object/src/function.rs | 40 +++++++++ 6 files changed, 269 insertions(+), 13 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py diff --git a/pyre/bench/synth/getattr_hook_binding.py b/pyre/bench/synth/getattr_hook_binding.py index 482c636d0a4..89966d49b9c 100644 --- a/pyre/bench/synth/getattr_hook_binding.py +++ b/pyre/bench/synth/getattr_hook_binding.py @@ -8,7 +8,8 @@ # whole `object_getattr_miss` walk plus a fresh frame for the hook. Inlining # the hook against the version-tag and map pins that make the miss constant # dropped the ratio from 48.6x/59.5x (dynasm/wasm) to 7.4x/10.4x/8.7x -# (dynasm/cranelift/wasm); the bound is twice the slowest of those, rounded up. +# (dynasm/cranelift/wasm); the bound is twice the slowest of those (10.4x), +# rounded up to the next multiple of five. class ClassmethodGetattr: diff --git a/pyre/bench/synth/inlined_helper_mutation.py b/pyre/bench/synth/inlined_helper_mutation.py index 84124d9dc24..7b6c54e1189 100644 --- a/pyre/bench/synth/inlined_helper_mutation.py +++ b/pyre/bench/synth/inlined_helper_mutation.py @@ -2,7 +2,8 @@ # The trip count now puts pypy above the startup-subtraction floor, so this # ratio is a measurement rather than pyre divided by the floor constant; a # ceiling fitted against the clamp (the 45 this bench once carried) fails. -# The bound is twice the slowest of the three backends, rounded up. +# The bound is twice the slowest of the three backends (27.7x), rounded up to +# the next multiple of ten. # # `push` binds `a.append` inside an inlined callee, and the folds that shape a # bound-method load used to decline for the whole of such a sub-walk. They now diff --git a/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py b/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py new file mode 100644 index 00000000000..fd68fc3c837 --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py @@ -0,0 +1,88 @@ +# CPython-suite gap: no suite test rebinds a descriptor under a hot JIT loop +# that folded it, and none installs a classmethod subclass as `__getattr__`. +# parity-tests reason: these are pyre JIT descriptor-binding regressions. + +# The inline of a type's `__getattr__` hook resolves the descriptor spelling at +# record time — a plain function is called with the receiver, a `classmethod` or +# `staticmethod` is unwrapped to the callable inside it. Two things that +# resolution must not assume, both of which produced a stale answer from the +# compiled trace while the interpreter answered correctly: +# +# * `get_and_call_function` (`descroperation.py:169-187`) takes the descriptor +# shortcut only for the EXACT type and routes everything else through +# `space.get`. A `classmethod` subclass overriding `__get__` binds through +# that override, so unwrapping its `w_function` calls the wrong callable. +# +# * `function.py:673`/`:720` `_immutable_fields_ = ['w_function?']`. The `?` +# registers the invalidation an assignment owes, so re-initialising an +# installed wrapper has to be observed. It changes no type's version tag, +# which is the only pin the fold holds over the descriptor. +# +# The rebinds happen INSIDE each loop, because a read after the loop is +# interpreted and would not consult what the trace baked. Every hook body +# returns a constant so the fold stands rather than aborting on a residual. + +N = 12000 +SWITCH = N // 2 + + +class Subclassed(classmethod): + def __get__(self, obj, objtype=None): + return lambda name: 2 + + +def first(cls_or_name, name=None): + return 1 + + +def second(cls_or_name, name=None): + return 2 + + +def exact_type_is_required(): + class Owner: + __getattr__ = Subclassed(first) + + owner = Owner() + last = None + for _ in range(N): + last = owner.miss + return last + + +def rebind(wrapper): + class Owner: + __getattr__ = wrapper(first) + + owner = Owner() + seen = [] + for index in range(N): + value = owner.miss + if index == SWITCH: + Owner.__dict__['__getattr__'].__init__(second) + elif index in (SWITCH - 1, N - 1): + seen.append(value) + return seen + + +def rebind_plain(): + class Owner: + __getattr__ = first + + owner = Owner() + seen = [] + for index in range(N): + value = owner.miss + if index == SWITCH: + Owner.__getattr__ = second + elif index in (SWITCH - 1, N - 1): + seen.append(value) + return seen + + +assert exact_type_is_required() == 2, 'overridden __get__ was bypassed' +assert rebind(classmethod) == [1, 2], 'classmethod w_function stayed baked' +assert rebind(staticmethod) == [1, 2], 'staticmethod w_function stayed baked' +assert rebind_plain() == [1, 2], 'rebound plain hook stayed baked' + +print("OK") diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index c275137ffe2..847e97bbacd 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -1536,6 +1536,79 @@ static W_METHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { ) }); +/// `pypy/interpreter/function.py:673` / `:720` +/// `_immutable_fields_ = ['w_function?']` for `StaticMethod` and `ClassMethod`. +/// The `?` is what makes the wrapped callable a constant, and it registers the +/// invalidation an assignment owes; pyre's setters do not force that yet, so +/// the field stays LIVE/MUTABLE here and the read is paired with a +/// `GuardValue`, the same pre-invalidation stand-in +/// [`FUNCTION_DESCR_GROUP`] documents for `code?`. +/// +/// Both censuses are COMPLETE — `w_dict` is listed even though nothing reads +/// it, because a field the struct declares but a group omits has no +/// `index_in_parent` to rederive and the two sides that mint its descr then +/// disagree on the number. `PyObject.w_class` is absent because no emit +/// allocates either wrapper, so the analyzer's count is the whole answer. +static W_STATICMETHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { + use pyre_object::function::{ + STATICMETHOD_W_DICT_OFFSET, STATICMETHOD_W_FUNCTION_OFFSET, W_STATICMETHOD_GC_TYPE_ID, + W_STATICMETHOD_OBJECT_SIZE, + }; + let field = |key, offset| { + ( + key, + offset, + std::mem::size_of::(), + Type::Ref, + false, + false, + false, + ) + }; + build_object_descr_group_with_def_path( + W_STATICMETHOD_OBJECT_SIZE, + W_STATICMETHOD_GC_TYPE_ID, + &pyre_object::function::STATICMETHOD_TYPE as *const _ as usize, + &[ + field("w_function", STATICMETHOD_W_FUNCTION_OFFSET), + field("w_dict", STATICMETHOD_W_DICT_OFFSET), + ], + "StaticMethod", + "function::StaticMethod", + ) +}); + +/// The `classmethod` twin of [`W_STATICMETHOD_DESCR_GROUP`]; see it for why +/// `w_function` is mutable and why `w_dict` is listed. +static W_CLASSMETHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { + use pyre_object::function::{ + CLASSMETHOD_W_DICT_OFFSET, CLASSMETHOD_W_FUNCTION_OFFSET, W_CLASSMETHOD_GC_TYPE_ID, + W_CLASSMETHOD_OBJECT_SIZE, + }; + let field = |key, offset| { + ( + key, + offset, + std::mem::size_of::(), + Type::Ref, + false, + false, + false, + ) + }; + build_object_descr_group_with_def_path( + W_CLASSMETHOD_OBJECT_SIZE, + W_CLASSMETHOD_GC_TYPE_ID, + &pyre_object::function::CLASSMETHOD_TYPE as *const _ as usize, + &[ + field("w_function", CLASSMETHOD_W_FUNCTION_OFFSET), + field("w_dict", CLASSMETHOD_W_DICT_OFFSET), + ], + "ClassMethod", + "function::ClassMethod", + ) +}); + /// `pypy/objspace/std/typeobject.py:26-34 ObjectMutableCell`. The single /// `w_value` field is read LIVE on the module-global cell fast path: a /// frequently-rewritten global mutates the cell payload in place without @@ -2663,6 +2736,19 @@ pub fn method_w_function_descr() -> DescrRef { field_descr_from_group(&W_METHOD_DESCR_GROUP, 0) } +/// Live `StaticMethod.w_function` — the callable a descriptor fold unwraps in +/// place of invoking `__get__`. Read live and pinned by a `GuardValue`; see +/// [`W_STATICMETHOD_DESCR_GROUP`] for why it is not a constant. +pub fn staticmethod_w_function_descr() -> DescrRef { + field_descr_from_group(&W_STATICMETHOD_DESCR_GROUP, 0) +} + +/// Live `ClassMethod.w_function` — the `classmethod` twin of +/// [`staticmethod_w_function_descr`]. +pub fn classmethod_w_function_descr() -> DescrRef { + field_descr_from_group(&W_CLASSMETHOD_DESCR_GROUP, 0) +} + /// Resolve one [`FUNCTION_DESCR_GROUP`] field by byte offset, so the accessors /// below stay correct however the census is ordered. fn function_field_descr(offset: usize) -> DescrRef { @@ -6753,6 +6839,8 @@ pub(crate) fn publish_runtime_descr_groups() { &*W_ZIP_DESCR_GROUP, &*RANGE_DESCR_GROUP, &*W_METHOD_DESCR_GROUP, + &*W_STATICMETHOD_DESCR_GROUP, + &*W_CLASSMETHOD_DESCR_GROUP, &*W_OBJECT_MUTABLE_CELL_DESCR_GROUP, &*W_LIST_DESCR_GROUP, &*W_TUPLE_DESCR_GROUP, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 2a529f00bf6..8ca97b08ba0 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -5493,8 +5493,8 @@ fn try_walker_inline_resolved_user_call_inner( // pre-subwalk count so it can source the preferred rebuild // while the fallback rewind remains provably disabled. if let Some((outer_jitcode_index, call_jitcode_pc)) = abort_flush_call_jitcode_coord - && let Some(stack) = - reconstructed_all_ref_call_stack(code, op, ctx, call_descr).or_else(|| { + && let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx, call_descr) + .or_else(|| { reconstructed_call_stack_from_resume_sources(ctx, call_jitcode_pc) }) { @@ -6303,6 +6303,23 @@ enum HookLeading { None, } +/// The wrapper slot a descriptor spelling of the hook unwrapped, so the fold +/// can pin the value it read. `function.py:673`/`:720` +/// `_immutable_fields_ = ['w_function?']`. +enum WrapperField { + ClassMethod, + StaticMethod, +} + +impl WrapperField { + fn descr(&self) -> majit_ir::DescrRef { + match self { + Self::ClassMethod => crate::descr::classmethod_w_function_descr(), + Self::StaticMethod => crate::descr::staticmethod_w_function_descr(), + } + } +} + #[allow(clippy::too_many_arguments)] pub(crate) fn try_walker_inline_getattr_hook( ctx: &mut WalkContext<'_, '_, Sym>, @@ -6330,24 +6347,35 @@ pub(crate) fn try_walker_inline_getattr_hook( }) else { return Ok(None); }; - // `get_and_call_function` leads the positionals with the receiver only for - // a plain `Function`; every other descriptor is bound through `get` first - // and called with the name alone. The version-tag pin makes that binding - // decision a constant of the trace, so each spelling resolves to its own - // function and leading argument here instead of declining. - let (w_func, leading) = unsafe { - if pyre_object::function::is_classmethod(w_getattr) { + // `get_and_call_function` (`descroperation.py:169-187`) leads the + // positionals with the receiver only for an exact `Function`; every other + // descriptor goes through `space.get` first and is called with the name + // alone. The version-tag pin makes that binding decision a constant of the + // trace, so each spelling resolves to its own function and leading argument + // here instead of declining. + // + // The type tests are EXACT. Upstream is explicit that they have to be + // ("isinstance(typ, Function) would not be correct here … because a builtin + // function binds differently than a normal function"), and the same holds + // for the two wrappers: a `classmethod` subclass overriding `__get__` binds + // through that override, so unwrapping `w_function` in its place calls the + // wrong callable. `wrapper_field` names the slot that unwrapping read, for + // the guard below; the plain arm reads no field. + let (w_func, leading, wrapper_field) = unsafe { + if pyre_object::function::is_exact_classmethod(w_getattr) { ( pyre_object::function::w_classmethod_get_func(w_getattr), HookLeading::Class, + Some(WrapperField::ClassMethod), ) - } else if pyre_object::function::is_staticmethod(w_getattr) { + } else if pyre_object::function::is_exact_staticmethod(w_getattr) { ( pyre_object::function::w_staticmethod_get_func(w_getattr), HookLeading::None, + Some(WrapperField::StaticMethod), ) } else { - (w_getattr, HookLeading::Receiver) + (w_getattr, HookLeading::Receiver, None) } }; if w_func.is_null() { @@ -6373,6 +6401,16 @@ pub(crate) fn try_walker_inline_getattr_hook( // Both pins the oracle asked for, plus the layout guard its map read needs. walker_guard_mapdict_instance_shape(ctx, op.pc, obj, concrete_obj, w_type, version_tag, map)?; + // The pins above make the DESCRIPTOR a constant; they say nothing about the + // callable inside it. Re-initialising an installed wrapper swaps + // `w_function` without touching the owner type's version tag, which is the + // only thing those pins hold, so read the slot live and pin the value this + // fold unwrapped — the stand-in [`walker_guard_function_field`] already + // makes for a quasi-immutable field pyre's setters do not invalidate. + if let Some(field) = wrapper_field { + let wrapper = ctx.trace_ctx.const_ref(w_getattr as i64); + walker_guard_function_field(ctx, op.pc, wrapper, field.descr(), w_func as i64)?; + } let name_obj = pyre_object::unicodeobject::box_str_constant(rustpython_wtf8::Wtf8::new(name.as_str())) diff --git a/pyre/pyre-object/src/function.rs b/pyre/pyre-object/src/function.rs index 8a510ab6de2..747e912a90c 100644 --- a/pyre/pyre-object/src/function.rs +++ b/pyre/pyre-object/src/function.rs @@ -177,6 +177,12 @@ pub struct StaticMethod { pub w_dict: PyObjectRef, } +/// Field offsets of the inline `PyObjectRef` slots within `StaticMethod`, +/// consumed by `pyre-jit-trace/src/descr.rs` on the same footing as the +/// `METHOD_*` consts above. +pub const STATICMETHOD_W_FUNCTION_OFFSET: usize = std::mem::offset_of!(StaticMethod, w_function); +pub const STATICMETHOD_W_DICT_OFFSET: usize = std::mem::offset_of!(StaticMethod, w_dict); + pub fn w_staticmethod_new(func: PyObjectRef) -> PyObjectRef { // `gct_fv_gc_malloc` bracket pattern (`framework.py:853-856`): pin the // wrapped function across the GC malloc and read its relocated address. @@ -271,6 +277,23 @@ pub unsafe fn is_staticmethod(obj: PyObjectRef) -> bool { py_type_check(obj, &STATICMETHOD_TYPE) } +/// An exact `staticmethod`, excluding subclasses — the test a caller needs +/// before it may unwrap `w_function` in place of invoking `__get__`. +/// `descroperation.py:169-187 get_and_call_function` takes its descriptor +/// shortcut only on the exact type and routes every other one through +/// `space.get`, so a subclass that overrides `__get__` binds differently. +/// Compares the user-visible class object, as [`is_exact_tuple`] does, because +/// a subclass instance keeps the base layout in `ob_type` and retags `w_class`. +#[inline] +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_staticmethod(obj: PyObjectRef) -> bool { + unsafe { + is_staticmethod(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&STATICMETHOD_TYPE)) + } +} + // ── ClassMethod ────────────────────────────────────────────────────── // PyPy: pypy/interpreter/function.py ClassMethod // @@ -285,6 +308,11 @@ pub struct ClassMethod { pub w_dict: PyObjectRef, } +/// Field offsets of the inline `PyObjectRef` slots within `ClassMethod`, the +/// `classmethod` twin of the `STATICMETHOD_*` consts above. +pub const CLASSMETHOD_W_FUNCTION_OFFSET: usize = std::mem::offset_of!(ClassMethod, w_function); +pub const CLASSMETHOD_W_DICT_OFFSET: usize = std::mem::offset_of!(ClassMethod, w_dict); + pub fn w_classmethod_new(func: PyObjectRef) -> PyObjectRef { // `gct_fv_gc_malloc` bracket pattern (`framework.py:853-856`): pin the // wrapped function across the GC malloc and read its relocated address. @@ -378,6 +406,18 @@ pub unsafe fn is_classmethod(obj: PyObjectRef) -> bool { py_type_check(obj, &CLASSMETHOD_TYPE) } +/// An exact `classmethod`, excluding subclasses — the `classmethod` twin of +/// [`is_exact_staticmethod`], for the same reason. +#[inline] +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_classmethod(obj: PyObjectRef) -> bool { + unsafe { + is_classmethod(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&CLASSMETHOD_TYPE)) + } +} + #[cfg(test)] mod tests { use super::*; From eb21dc467c39a5e75fde69a32d9d0394dcd66a47 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 10:57:13 +0900 Subject: [PATCH 09/24] getattr_hook_fast_path: decline a devolved receiver `find_map_attr(name, DICT)` is read here for its ABSENCE, to prove no instance attribute shadows the name before folding the read to the type's `__getattr__`. mapdict.py:1534-1536 states that call always returns None for a map rooted at a `DevolvedDictTerminator`, so the answer carries no information for a devolved instance and the fold ran the hook for a name the instance's dictionary holds. The map `GuardValue` the fold emits does not separate the two cases: the devolved terminator is a per-class singleton, so every devolved instance of the class guards the same map word and a later `obj. = ...` does not change it. `LOAD_METHOD_mapdict_fill_cache_method` is upstream's own case of pinning a map to cache a negative instance lookup, and it refuses the shape at mapdict.py:1569. Add the same decline; the twin over a dict-backed receiver already had it (mapdict.rs:1880). New parity test `getattr_hook_devolved_dict.py`, which failed on the unfixed binary with `{'real', 'hook'}` where CPython 3.14 and PYRE_JIT=0 answer `{'real'}`. Assisted-by: Claude --- .../getattr_hook_devolved_dict.py | 64 +++++++++++++++++++ .../src/objspace/std/mapdict.rs | 15 +++++ 2 files changed, 79 insertions(+) create mode 100644 pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py diff --git a/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py b/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py new file mode 100644 index 00000000000..a9763dfabff --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py @@ -0,0 +1,64 @@ +# CPython-suite gap: no suite test reads an attribute of a devolved instance of +# a class that also defines `__getattr__`, under a loop hot enough to specialize. +# parity-tests reason: this is a pyre JIT `__getattr__`-fold regression. + +# The fold that replaces `obj.name` with the type's `__getattr__` proves the +# name is absent from the instance by asking `find_map_attr(name, DICT)` and +# taking None for absence. mapdict.py:1534-1536 states that call "will always +# return None if attrkind==DICT" once the map is rooted at a +# `DevolvedDictTerminator`, so for a devolved instance the answer is the same +# whether or not the attribute is there. Upstream's own case of pinning a map +# to cache a negative instance lookup — `LOAD_METHOD_mapdict_fill_cache_method` +# — refuses the shape outright (mapdict.py:1569). +# +# The map guard cannot stand in: the devolved terminator is a per-class +# singleton, so the pinned map word is identical for every devolved instance of +# the class and unchanged by a later attribute assignment. + +N = 12000 + + +class Hooked: + def __getattr__(self, name): + return 'hook' + + +def non_string_key_devolves(): + obj = Hooked() + # A non-str `__dict__` key forces the object strategy at any attribute + # count, without waiting for the attribute-count limit. + obj.__dict__[1] = 'sentinel' + obj.__dict__['present'] = 'real' + seen = set() + for _ in range(N): + seen.add(obj.present) + assert seen == {'real'}, 'devolved instance answered from the hook: %r' % (seen,) + + +def assignment_after_devolving_is_seen(): + obj = Hooked() + obj.__dict__[1] = 'sentinel' + seen = [] + for i in range(N): + seen.append(obj.later) + if i == N // 2: + obj.later = 'assigned' + assert seen[0] == 'hook', 'absent attribute did not reach the hook: %r' % (seen[0],) + assert seen[-1] == 'assigned', ( + 'assignment on a devolved instance was not seen: %r' % (seen[-1],) + ) + + +def hook_still_answers_a_real_miss(): + obj = Hooked() + obj.__dict__[1] = 'sentinel' + seen = set() + for _ in range(N): + seen.add(obj.missing) + assert seen == {'hook'}, 'the decline swallowed the hook: %r' % (seen,) + + +non_string_key_devolves() +assignment_after_devolving_is_seen() +hook_still_answers_a_real_miss() +print("OK") diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index a9c2634fd80..81d3b6ccfd4 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -1817,6 +1817,21 @@ pub unsafe fn getattr_hook_fast_path( if unsafe { crate::baseobjspace::lookup_in_type_where(w_type, name) }.is_some() { return None; } + // A devolved instance keeps its attributes in a real dictionary, and + // mapdict.py:1534-1536 states that `find_map_attr` "will always return + // None if attrkind==DICT" for such a map. The hit path reads that call + // for a Some, so a None costs it only the fold; this path reads it for its + // ABSENCE and would take an always-None answer as proof the name is not on + // the instance. `LOAD_METHOD_mapdict_fill_cache_method` — upstream's own + // case of pinning a map to cache a negative instance lookup — refuses the + // shape outright (mapdict.py:1569 `if map is None or + // isinstance(map.terminator, DevolvedDictTerminator): return`), and the + // map pin cannot stand in: the devolved terminator is a per-class + // singleton, so the guarded map word is the same for every devolved + // instance and unchanged by a later `obj. = ...`. + if unsafe { map_is_devolved(map) } { + return None; + } // `classify_attr(w_type, None, false)` answers `(DICT, false)` — the // no-descriptor arm (mapdict.py:1509-1510) — so this is the same // `find_map_attr` call the hit path makes, read for its absence. From a9403ffb047a3da1e9534120fbfb03f9f764a5ba Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 10:57:40 +0900 Subject: [PATCH 10/24] property: take the accessor shortcut only for the exact type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get`/`set`/`delete` called `fget`/`fset`/`fdel` in place of `type(w_descr).__get__` behind `is_property`, which is a `py_type_check`, i.e. an `ob_type` compare. A `property` subclass keeps the base layout and retags only `w_class`, so the test admitted it and an overridden `__get__` never ran. `descroperation.py:169-176 get_and_call_function` is where upstream draws that line, for its own shortcut: `typ = type(w_descr)` then `if typ is Function or typ is FunctionWithFixedCode`, with "isinstance(typ, Function) would not be correct here". Everything else reaches its accessor through `space.get`. `is_exact_property` compares `w_class` against `get_instantiate(&PROPERTY_TYPE)`, the `is_exact_classmethod` spelling. It separates the two by construction: `property_descr_new` allocates through `w_property_new`, which sets `w_class` to `property`, and calls `tag_subclass_instance` — the only writer of that word — solely for a subclass. `set` and `delete` resolved the descriptor's type for their MRO fallback only when it was a `GetSetProperty` or an instance, so a subclass falling through reached no lookup at all; resolve it through `crate::typedef::r#type` for every descriptor kind, as `get`'s tail already did. The JIT's `property_descr_fast_path` gate takes the same narrowing: it calls the accessor directly, so it is licensed by the same exact type. New parity test `property_subclass_descriptor_protocol.py`, which failed on the unfixed binary under both `PYRE_JIT=0` and the JIT with `{'wrapped-get'}` where CPython 3.14 answers `{'override-get'}`. Assisted-by: Claude --- .../property_subclass_descriptor_protocol.py | 94 +++++++++++++++++++ pyre/pyre-interpreter/src/baseobjspace.rs | 48 +++++----- .../src/objspace/std/mapdict.rs | 6 +- pyre/pyre-object/src/descriptor.rs | 23 +++++ 4 files changed, 143 insertions(+), 28 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py diff --git a/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py b/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py new file mode 100644 index 00000000000..07393fffd95 --- /dev/null +++ b/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py @@ -0,0 +1,94 @@ +# CPython-suite gap: the suite's property tests subclass `property` to add +# methods, never to override `__get__` / `__set__` / `__delete__`. +# parity-tests reason: this is a pyre descriptor-dispatch divergence, shared by +# the interpreter and the JIT fold over it. + +# `get_and_call_function` (`descroperation.py:169-176`) takes a descriptor +# shortcut only for the EXACT type — "isinstance(typ, Function) would not be +# correct here" — and routes everything else through `space.get`, i.e. +# `type(w_descr).__get__` off the MRO. A `property` subclass keeps the base +# layout and retags only its class word, so a layout test admits it and calls +# the wrapped `fget` in place of the override. +# +# The hot loop is here because the JIT's property fold applies the same +# resolution: it must decline for a subclass rather than bake the wrapped +# accessor. + +N = 12000 + + +class Overriding(property): + def __get__(self, obj, objtype=None): + return 'override-get' + + def __set__(self, obj, value): + obj.recorded = 'override-set' + + def __delete__(self, obj): + obj.recorded = 'override-del' + + +def base_getter(self): + return 'wrapped-get' + + +def base_setter(self, value): + self.recorded = 'wrapped-set' + + +def base_deleter(self): + self.recorded = 'wrapped-del' + + +class WithOverride: + recorded = None + x = Overriding(base_getter, base_setter, base_deleter) + + +class Plain: + # A subclass that overrides nothing still reaches `property`'s own + # `__get__` through the MRO. + recorded = None + x = type('Inert', (property,), {})(base_getter, base_setter, base_deleter) + + +def overridden_accessors_run(): + obj = WithOverride() + seen = set() + for _ in range(N): + seen.add(obj.x) + assert seen == {'override-get'}, 'overridden __get__ was bypassed: %r' % (seen,) + + for _ in range(N): + obj.x = 1 + assert obj.recorded == 'override-set', ( + 'overridden __set__ was bypassed: %r' % (obj.recorded,) + ) + + del obj.x + assert obj.recorded == 'override-del', ( + 'overridden __delete__ was bypassed: %r' % (obj.recorded,) + ) + + +def inert_subclass_still_works(): + obj = Plain() + seen = set() + for _ in range(N): + seen.add(obj.x) + assert seen == {'wrapped-get'}, 'inert subclass lost its getter: %r' % (seen,) + + obj.x = 1 + assert obj.recorded == 'wrapped-set', ( + 'inert subclass lost its setter: %r' % (obj.recorded,) + ) + + del obj.x + assert obj.recorded == 'wrapped-del', ( + 'inert subclass lost its deleter: %r' % (obj.recorded,) + ) + + +overridden_accessors_run() +inert_subclass_still_works() +print("OK") diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 038ac5fbe48..66c31bdda70 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -10938,8 +10938,13 @@ pub(crate) unsafe fn get( } } - // property: PyPy W_Property.get → call fget(obj) - if is_property(descr) { + // property: PyPy W_Property.get → call fget(obj). Exact type only, for + // the reason `descroperation.py:169-176` gives its own shortcut: calling + // the accessor in place of `type(w_descr).__get__` is licensed only where + // the type cannot have overridden `__get__`. A subclass falls through to + // the general MRO lookup at the end of this function, which finds either + // its override or `property`'s own typedef entry. + if is_exact_property(descr) { // W_Property.get receives `space.w_None` for class access. Internally // that state is a null pointer so the actual None singleton can still // be a property-bearing instance. @@ -11053,7 +11058,8 @@ unsafe fn set( // raise AttributeError ("can't set attribute") rather than falling // through to the instance dict (`descrobject.c property_descr_set`, // mirrored at `pypy/module/__builtin__/descriptor.py W_Property.set`). - if is_property(descr) { + // Exact type only — see the `__get__` twin. + if is_exact_property(descr) { let fset = w_property_get_fset(descr); if fset.is_null() || is_none(fset) { return Err(property_no_accessor(descr, obj, "setter")?); @@ -11105,18 +11111,13 @@ unsafe fn set( return Ok(true); } - // General __set__: look up on descriptor's type MRO. GetSetProperty - // is no longer INSTANCE_TYPE-shaped (it carries `GETSET_DESCRIPTOR - // _TYPE` so its GetSetProperty payload is GC-traced), so resolve - // the type through `crate::typedef::r#type` rather than the - // `is_instance` branch. - let descr_type = if pyre_object::typedef::is_getset_property(descr) { - crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()) - } else if is_instance(descr) { - w_instance_get_type(descr) - } else { - std::ptr::null_mut() - }; + // General __set__: `space.lookup(w_descr, '__set__')` is an MRO lookup on + // whatever `type(w_descr)` is, so resolve the type the same way for every + // descriptor kind — the `__get__` twin at the end of `get` already does. + // The narrower `is_getset_property` / `is_instance` pair this replaces left + // a native-layout subclass instance (a `property` subclass, say) with a + // null type and so no MRO lookup at all. + let descr_type = crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()); if !descr_type.is_null() && let Some(set_fn) = lookup_in_type_where(descr_type, "__set__") && !set_fn.is_null() @@ -11131,8 +11132,8 @@ unsafe fn set( /// /// descroperation.py `space.delete(w_descr, w_obj)` unsafe fn delete(descr: PyObjectRef, obj: PyObjectRef) -> Result<(), crate::PyError> { - // property: call fdel(obj) - if is_property(descr) { + // property: call fdel(obj). Exact type only — see the `__get__` twin. + if is_exact_property(descr) { let fdel = w_property_get_fdel(descr); if fdel.is_null() || is_none(fdel) { return Err(property_no_accessor(descr, obj, "deleter")?); @@ -11177,16 +11178,9 @@ unsafe fn delete(descr: PyObjectRef, obj: PyObjectRef) -> Result<(), crate::PyEr } return Ok(()); } - // General __delete__: look up on descriptor's type MRO — same - // shape as `set` above (resolve type through `r#type` so non- - // INSTANCE_TYPE descriptors like `GetSetProperty` are reached). - let descr_type = if pyre_object::typedef::is_getset_property(descr) { - crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()) - } else if is_instance(descr) { - w_instance_get_type(descr) - } else { - std::ptr::null_mut() - }; + // General __delete__: look up on descriptor's type MRO — same shape as + // `set` above. + let descr_type = crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()); if !descr_type.is_null() && let Some(del_fn) = lookup_in_type_where(descr_type, "__delete__") && !del_fn.is_null() diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 81d3b6ccfd4..1fe26794207 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -1960,7 +1960,11 @@ unsafe fn property_descr_fast_path( return None; } let w_descr = unsafe { crate::baseobjspace::lookup_in_type(w_type, name) }?; - if !unsafe { pyre_object::descriptor::is_property(w_descr) } { + // Exact type: the fold calls `fget`/`fset` directly, which stands in for + // `type(w_descr).__get__` only where that cannot have been overridden + // (`descroperation.py:169-176`). A `property` subclass keeps the base + // layout and retags only `w_class`, so the layout test admits it. + if !unsafe { pyre_object::descriptor::is_exact_property(w_descr) } { return None; } Some((w_type, version_tag, w_descr)) diff --git a/pyre/pyre-object/src/descriptor.rs b/pyre/pyre-object/src/descriptor.rs index c85d4523736..fda1a44c04e 100644 --- a/pyre/pyre-object/src/descriptor.rs +++ b/pyre/pyre-object/src/descriptor.rs @@ -353,6 +353,29 @@ pub unsafe fn is_property(obj: PyObjectRef) -> bool { py_type_check(obj, &PROPERTY_TYPE) } +/// `type(obj) is property`, as opposed to [`is_property`]'s layout test. +/// +/// `descroperation.py:169-176 get_and_call_function` spells out why the +/// difference decides who may take an accessor shortcut: `typ = type(w_descr)` +/// then `if typ is Function or typ is FunctionWithFixedCode`, with +/// "isinstance(typ, Function) would not be correct here". Everything else +/// reaches its accessor through `space.get`, i.e. `type(w_descr).__get__` off +/// the MRO — so calling `fget` in place of `__get__` is licensed only when the +/// descriptor's type is `property` itself and cannot have overridden it. +/// +/// The two answers really do separate: `property_descr_new` allocates through +/// [`w_property_new`], which sets `w_class` to `property`, and calls +/// `tag_subclass_instance` — the only writer of `w_class` — solely when the +/// requested type is not `property`. `ob_type`, which [`is_property`] reads, +/// stays the shared layout word either way. +/// +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_property(obj: PyObjectRef) -> bool { + unsafe { is_property(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&PROPERTY_TYPE)) } +} + #[cfg(test)] mod property_tests { use super::*; From 6b94842d5cb781abcbc71fdfa206132220e7cdc6 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 10:58:22 +0900 Subject: [PATCH 11/24] property: implement the `w_fget?` / `w_fset?` quasi-immutable declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LOAD_ATTR and STORE_ATTR property folds baked the accessor as a trace constant while holding only the receiver's class, its `w_class`, and the type's `_version_tag?`. That triple makes the DESCRIPTOR constant and stops there: `property.__init__` on an installed property replaces the accessors in place and bumps no type's version, so the compiled trace kept calling the previous getter. `descriptor.py:175 _immutable_fields_ = ["w_fget?", "w_fset?", "w_fdel?"]` is what covers the slot upstream. The `?` is both halves — `rclass.py:715-718 hook_setfield` emits `jit_force_quasi_immutable` ahead of every store, and `pyjitpl.py:1084-1088` records `QUASIIMMUT_FIELD` + `GUARD_NOT_INVALIDATED` on the read, so `quasiimmut.py:95-100` revokes every loop that folded it. Wire the two slots a fold bakes through the existing `QuasiImmutField` port, the way the other seven `?` fields are wired: * `fget_watchers` / `fset_watchers` on `W_Property`, swept by `w_property_reinit` — the only writer, since `p.fget = f` has no `direct_member_set` arm; * `PROPERTY_FGET_INDEX` / `PROPERTY_FSET_INDEX` descrs, reserved rather than `stable_field_index`-derived because a `PyObjectRef` at the first offsets past the header names a layout every `W_*` class shares, and the index is what selects the pointer cast in the two dispatchers; * arms in `install_quasiimmut_field` and `register_quasi_immutable_deps`; * `walker_pin_property_accessor` in both folds, over a rewind point, because the callee inline still has decline paths past that point. A marker, never a load: the descriptor is a baked `ConstPtr`, and reading a field through one is the hazard `guards_the_callee_function` exists to avoid. `record_quasiimmut_field` dereferences the owner only at record and compile time, and a property is allocated non-moving. New fixture `bench/synth/property_accessor_invalidation.py`, carrying `# pyre-check: no-cpython`: CPython 3.14's `LOAD_ATTR_PROPERTY` specialization caches `fget` under the type version alone, so a specialized read there keeps the previous getter too and CPython cannot be the oracle. pypy answers `599999 / 599999 / 199999`, as does pyre's interpreter; the JIT answered `400001 / 400001 / 1`. Assisted-by: Claude --- ...y_accessor_invalidation.cranelift.jitstats | 15 +++ ...erty_accessor_invalidation.dynasm.jitstats | 15 +++ .../synth/property_accessor_invalidation.py | 93 ++++++++++++++++ ...operty_accessor_invalidation.wasm.jitstats | 15 +++ .../src/objspace/std/mapdict.rs | 35 +++--- pyre/pyre-jit-trace/src/descr.rs | 71 ++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 30 ++++- .../src/jitcode_dispatch/mod.rs | 28 +++++ pyre/pyre-jit-trace/src/state.rs | 10 +- pyre/pyre-jit/src/eval.rs | 14 ++- pyre/pyre-object/src/descriptor.rs | 103 +++++++++++++++++- 11 files changed, 405 insertions(+), 24 deletions(-) create mode 100644 pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats create mode 100644 pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats create mode 100644 pyre/bench/synth/property_accessor_invalidation.py create mode 100644 pyre/bench/synth/property_accessor_invalidation.wasm.jitstats diff --git a/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats b/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats b/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/property_accessor_invalidation.py b/pyre/bench/synth/property_accessor_invalidation.py new file mode 100644 index 00000000000..937dccbcde3 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.py @@ -0,0 +1,93 @@ +# pyre-check: no-cpython +# `descriptor.py:175 W_Property._immutable_fields_ = ["w_fget?", "w_fset?", +# "w_fdel?"]`. The `?` is what lets a tracer bake the accessor and equally what +# registers the invalidation an assignment to the slot owes, so re-initialising +# an installed property revokes every loop that folded it. +# +# CPython is not an oracle for this: its `LOAD_ATTR_PROPERTY` specialization +# caches `fget` under the receiver type's version alone, and `property.__init__` +# on an installed descriptor bumps no type's version, so a specialized read +# keeps answering with the previous getter. Cold, CPython sees the new one — +# the divergence is the specialization's, and pyre follows pypy's `?` instead. +# +# Each rebind happens INSIDE its loop: a read after the loop is interpreted and +# would not consult what the trace baked. The accessor bodies are residual-free +# so the folds stand rather than aborting. +N = 400000 +SWITCH = N // 2 + + +def first_getter(self): + return 1 + + +def second_getter(self): + return 2 + + +def first_setter(self, value): + self.slot = 1 + + +def second_setter(self, value): + self.slot = 2 + + +class Getter: + x = property(first_getter) + + +class Setter: + slot = 0 + y = property(None, first_setter) + + +def rebind_getter(): + obj = Getter() + descr = Getter.__dict__['x'] + total = 0 + i = 0 + while i < N: + total += obj.x + if i == SWITCH: + descr.__init__(second_getter) + i += 1 + # SWITCH+1 reads of 1, then N-SWITCH-1 reads of 2. + print('getter', total) + + +def rebind_setter(): + obj = Setter() + descr = Setter.__dict__['y'] + total = 0 + i = 0 + while i < N: + obj.y = i + total += obj.slot + if i == SWITCH: + descr.__init__(None, second_setter) + i += 1 + print('setter', total) + + +def drop_getter(): + # The sharper case: the re-init leaves no getter at all, and `W_Property.get` + # (descriptor.py:224-225) raises rather than calling the old function. + obj = Getter() + descr = Getter.__dict__['x'] + raised = 0 + i = 0 + while i < N: + try: + obj.x + except AttributeError: + raised += 1 + if i == SWITCH: + descr.__init__(None) + i += 1 + print('dropped', raised) + + +rebind_getter() +rebind_setter() +drop_getter() diff --git a/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats b/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 1fe26794207..dacec04e004 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -1970,19 +1970,24 @@ unsafe fn property_descr_fast_path( Some((w_type, version_tag, w_descr)) } -/// LOAD_ATTR `property` fast path: return the type, version tag, and Python -/// `fget` when `obj.name` reads a property getter, so the full-body walker can -/// inline `fget(obj)` in place of the opaque `getattr` residual. Returns `None` -/// (leave the residual) for a write-only property or any shape -/// [`property_descr_fast_path`] declines. A custom `__getattribute__` owns the -/// read (mapdict.py:1497-1499), so it declines to the residual. +/// LOAD_ATTR `property` fast path: return the type, version tag, the property +/// object, and its Python `fget` when `obj.name` reads a property getter, so +/// the full-body walker can inline `fget(obj)` in place of the opaque `getattr` +/// residual. Returns `None` (leave the residual) for a write-only property or +/// any shape [`property_descr_fast_path`] declines. A custom +/// `__getattribute__` owns the read (mapdict.py:1497-1499), so it declines to +/// the residual. +/// +/// The property object is part of the answer because `fget` alone cannot be +/// baked: `descriptor.py:175` declares the slot `w_fget?`, so the fold owes it +/// a `QUASIIMMUT_FIELD` marker naming the owner. /// /// # Safety /// `w_obj` must be a live object. pub unsafe fn property_get_fast_path( w_obj: PyObjectRef, name: &str, -) -> Option<(PyObjectRef, u64, PyObjectRef)> { +) -> Option<(PyObjectRef, u64, PyObjectRef, PyObjectRef)> { let (w_type, version_tag, w_descr) = unsafe { property_descr_fast_path(w_obj, name) }?; if unsafe { crate::baseobjspace::getattribute_if_not_from_object(w_type) }.is_some() { return None; @@ -1991,22 +1996,22 @@ pub unsafe fn property_get_fast_path( if fget.is_null() || unsafe { pyre_object::pyobject::is_none(fget) } { return None; } - Some((w_type, version_tag, fget)) + Some((w_type, version_tag, w_descr, fget)) } /// STORE_ATTR `property` fast path: the setter twin of -/// [`property_get_fast_path`], returning the type, version tag, and Python -/// `fset` when `obj.name = value` writes a property setter. Returns `None` -/// (leave the residual) for a read-only property or any shape -/// [`property_descr_fast_path`] declines. A custom `__setattr__` owns the write -/// (mapdict.py:1612-1614), so it declines to the residual. +/// [`property_get_fast_path`], returning the type, version tag, the property +/// object, and its Python `fset` when `obj.name = value` writes a property +/// setter. Returns `None` (leave the residual) for a read-only property or any +/// shape [`property_descr_fast_path`] declines. A custom `__setattr__` owns +/// the write (mapdict.py:1612-1614), so it declines to the residual. /// /// # Safety /// `w_obj` must be a live object. pub unsafe fn property_set_fast_path( w_obj: PyObjectRef, name: &str, -) -> Option<(PyObjectRef, u64, PyObjectRef)> { +) -> Option<(PyObjectRef, u64, PyObjectRef, PyObjectRef)> { let (w_type, version_tag, w_descr) = unsafe { property_descr_fast_path(w_obj, name) }?; if unsafe { crate::baseobjspace::setattr_if_not_from_object(w_type) }.is_some() { return None; @@ -2015,7 +2020,7 @@ pub unsafe fn property_set_fast_path( if fset.is_null() || unsafe { pyre_object::pyobject::is_none(fset) } { return None; } - Some((w_type, version_tag, fset)) + Some((w_type, version_tag, w_descr, fset)) } /// The unboxed counterpart of [`load_attr_fast_path`]. It applies the same diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index 847e97bbacd..35eca5ee4df 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -69,6 +69,20 @@ const HOLDER_TYP_INDEX: u32 = MAPDICT_DESCR_TAG | 3; // `descr.rs`). const AUDIT_HOLDER_HOOKS_INDEX: u32 = MAPDICT_DESCR_TAG | 4; +// `W_Property.fget` / `.fset` are `PyObjectRef` at the first two offsets past +// the object header — the single most crowded coordinate in the tree, since +// every `W_*` class's first reference field lands there. A +// `stable_field_index(offset, size, type, signed)` would therefore name a +// layout, not an owner, and the index is what selects the pointer cast in +// `install_quasiimmut_field` / `register_quasi_immutable_deps`. Reserved for +// the same reason as the map-node block above, in a tag of its own because the +// owner here IS a `PyObject` and the reasoning that groups those four does not +// apply. Disjoint from FIELD (0x10xx_xxxx), ARRAY, SIZE, CELL, MAPDICT, +// `object.typeptr` (0x6000_0000), the native mapdict block, and the GC tid. +const PROPERTY_DESCR_TAG: u32 = 0x5100_0000; +const PROPERTY_FGET_INDEX: u32 = PROPERTY_DESCR_TAG; +const PROPERTY_FSET_INDEX: u32 = PROPERTY_DESCR_TAG | 1; + // The generated native user layouts append mapdict fields at different base // sizes. HeapCache keys by descriptor index; give each translated STRUCT field // the distinct identity provided by descr.py's per-STRUCT cache. @@ -3290,6 +3304,63 @@ pub fn audit_holder_hooks_descr() -> DescrRef { AUDIT_HOLDER_HOOKS_FIELD_DESCR.clone() } +/// `descriptor.py:175 W_Property._immutable_fields_ = ["w_fget?", "w_fset?", +/// "w_fdel?"]` — the property's getter slot. +/// +/// The LOAD_ATTR property fold inlines `fget(obj)` against a descriptor the +/// receiver's class + `_version_tag?` already pin, which makes the descriptor +/// object constant but says nothing about its accessor slots: `__init__` on an +/// installed property replaces them in place and bumps no type's version. The +/// `?` is what covers the slot, and it costs a `QUASIIMMUT_FIELD` marker plus +/// one `GUARD_NOT_INVALIDATED` per trace rather than a load and a `GUARD_VALUE` +/// per iteration — see [`walker_pin_type_version_tag`](crate::jitcode_dispatch) +/// for why the difference is load-bearing across a residual call. +/// +/// A marker only: the fold never loads through the baked descriptor pointer, +/// which is what keeps it clear of the baked-`ConstPtr` hazard that made the +/// inline-call path skip its `Function.code` reads for a constant callable. +static PROPERTY_FGET_FIELD_DESCR: LazyLock = LazyLock::new(|| { + Arc::new( + majit_ir::descr::SimpleFieldDescr::new_with_name( + PROPERTY_FGET_INDEX, + core::mem::offset_of!(pyre_object::descriptor::W_Property, fget), + std::mem::size_of::(), + Type::Ref, + false, + majit_ir::descr::ArrayFlag::Unsigned, + "W_Property.fget".to_string(), + "fget".to_string(), + ) + .with_quasi_immutable(true), + ) +}); + +pub fn property_fget_descr() -> DescrRef { + PROPERTY_FGET_FIELD_DESCR.clone() +} + +/// The `w_fset?` twin of [`PROPERTY_FGET_FIELD_DESCR`], read by the STORE_ATTR +/// property fold. +static PROPERTY_FSET_FIELD_DESCR: LazyLock = LazyLock::new(|| { + Arc::new( + majit_ir::descr::SimpleFieldDescr::new_with_name( + PROPERTY_FSET_INDEX, + core::mem::offset_of!(pyre_object::descriptor::W_Property, fset), + std::mem::size_of::(), + Type::Ref, + false, + majit_ir::descr::ArrayFlag::Unsigned, + "W_Property.fset".to_string(), + "fset".to_string(), + ) + .with_quasi_immutable(true), + ) +}); + +pub fn property_fset_descr() -> DescrRef { + PROPERTY_FSET_FIELD_DESCR.clone() +} + /// `W_ObjectObject` SizeDescr group (`objectobject.rs:34-46`) — the instance /// layout `[ob_type | w_class | map | storage]`. Built with a parent SizeDescr /// (unlike a bare [`make_field_descr`]) so a `getfield_gc` on `map` / `storage` diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 8ca97b08ba0..7a76426b2b6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -6200,7 +6200,7 @@ pub(crate) fn try_walker_inline_property_get( let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { return Ok(None); }; - let Some((w_type, version_tag, fget)) = (unsafe { + let Some((w_type, version_tag, w_descr, fget)) = (unsafe { pyre_interpreter::objspace::std::mapdict::property_get_fast_path(concrete_obj, &name) }) else { return Ok(None); @@ -6230,7 +6230,12 @@ pub(crate) fn try_walker_inline_property_get( ConcreteValue::Ref(concrete_obj), ]; let fget_const = ctx.trace_ctx.const_ref(fget as i64); - try_walker_inline_resolved_user_call( + // Everything below emits, and the callee inline has decline paths of its + // own past this point, so keep a rewind point the way the type-call fold + // does. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); + walker_pin_property_accessor(ctx, op.pc, w_descr, crate::descr::property_fget_descr())?; + let inlined = try_walker_inline_resolved_user_call( ctx, op, code, @@ -6260,7 +6265,12 @@ pub(crate) fn try_walker_inline_property_get( // read (same allowance the exception `__str__`/`__repr__` override uses). false, None, - ) + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) } /// Inline the receiver type's `__getattr__` hook for an attribute the type and @@ -6503,7 +6513,7 @@ pub(crate) fn try_walker_inline_property_set( let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { return Ok(None); }; - let Some((w_type, version_tag, fset)) = (unsafe { + let Some((w_type, version_tag, w_descr, fset)) = (unsafe { pyre_interpreter::objspace::std::mapdict::property_set_fast_path(concrete_obj, &name) }) else { return Ok(None); @@ -6542,7 +6552,10 @@ pub(crate) fn try_walker_inline_property_set( ConcreteValue::Ref(concrete_value), ]; let fset_const = ctx.trace_ctx.const_ref(fset as i64); - try_walker_inline_resolved_user_call( + // Rewind point for the same reason as the getter twin. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); + walker_pin_property_accessor(ctx, op.pc, w_descr, crate::descr::property_fset_descr())?; + let inlined = try_walker_inline_resolved_user_call( ctx, op, code, @@ -6571,7 +6584,12 @@ pub(crate) fn try_walker_inline_property_set( true, false, None, - ) + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) } /// Whether a concrete object is the canonical machine-word `int` layout that diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 0b3d04387b5..679871790ea 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -8626,6 +8626,34 @@ fn walker_pin_type_version_tag( walker_flush_guard_not_invalidated(ctx, op_pc) } +/// The `descriptor.py:175 _immutable_fields_ = ["w_fget?", "w_fset?", +/// "w_fdel?"]` twin of [`walker_pin_type_version_tag`]: pin the accessor slot +/// a property fold is about to bake. +/// +/// The receiver pins the folds already hold — class, `w_class`, and the type's +/// `_version_tag?` — make the DESCRIPTOR a compile-time answer, and stop +/// there: `property.__init__` on an installed descriptor replaces `fget`/ +/// `fset` in place and bumps no type's version, so without this the trace kept +/// calling the previous getter. Upstream covers exactly that gap with the `?` +/// on the slots themselves. +/// +/// A marker, never a load: the descriptor is a baked `ConstPtr`, and reading a +/// field through one is the hazard `try_walker_inline_resolved_user_call`'s +/// `guards_the_callee_function` gate exists to avoid. `record_quasiimmut_field` +/// dereferences the owner only at record and compile time, and a property is +/// allocated non-moving, so both reads see the object where the constant says +/// it is. +fn walker_pin_property_accessor( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + w_descr: pyre_object::PyObjectRef, + field: majit_ir::DescrRef, +) -> Result<(), DispatchError> { + let descr_const = ctx.trace_ctx.const_ref(w_descr as i64); + crate::state::record_quasiimmut_field(ctx.trace_ctx, descr_const, field); + walker_flush_guard_not_invalidated(ctx, op_pc) +} + /// The `celldict.py:34 _immutable_fields_ = ["version?"]` twin of /// [`walker_pin_type_version_tag`]: pin the module namespace's strategy version /// so the folds that bake a slot's stored cell (or the absence of a name) are diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 4a1b7845781..e5f5e6e6387 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -5058,7 +5058,7 @@ fn install_quasiimmut_field(ctx: &mut TraceCtx, obj: OpRef, descr: &DescrRef) { // one must fail loudly rather than reinterpret a headerless map-node // allocation as a `W_TypeObject`. Dropping the old implicit `W_TypeObject` // fallback is safe: the arms below are every quasi-immutable descr this - // binary can mint — the seven hand-minted singletons, plus the nine + // binary can mint — the nine hand-minted singletons, plus the nine // `Function` fields `function.py:34-42` declares, which // `function_quasi_immut_slot` resolves as a group. No analyzer-derived // descr reaches here: a `#[jit_immutable_fields]` entry would need the @@ -5093,6 +5093,14 @@ fn install_quasiimmut_field(ctx: &mut TraceCtx, obj: OpRef, descr: &DescrRef) { pyre_interpreter::module::sys::vm::audit_holder_install_hooks_watcher( struct_ptr as *const _, ); + } else if index == crate::descr::property_fget_descr().index() { + pyre_object::descriptor::w_property_install_fget_watcher( + struct_ptr as pyre_object::PyObjectRef, + ); + } else if index == crate::descr::property_fset_descr().index() { + pyre_object::descriptor::w_property_install_fset_watcher( + struct_ptr as pyre_object::PyObjectRef, + ); } else if let Some(slot) = crate::descr::function_quasi_immut_slot(index) { pyre_interpreter::function::function_install_quasi_immut( struct_ptr as pyre_object::PyObjectRef, diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index fa269a4fe8b..dc65c277914 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -6377,10 +6377,12 @@ pub(crate) fn register_quasi_immutable_deps(_green_key: u64) { let holder_attr = pyre_jit_trace::descr::holder_attr_descr().index(); let holder_typ = pyre_jit_trace::descr::holder_typ_descr().index(); let audit_holder_hooks = pyre_jit_trace::descr::audit_holder_hooks_descr().index(); + let property_fget = pyre_jit_trace::descr::property_fget_descr().index(); + let property_fset = pyre_jit_trace::descr::property_fset_descr().index(); // Hoisted because each accessor clones a `LazyLock` descr; the index also // decides which type `dep_ptr` is cast to, so the chain below ends in a // fail-loud default rather than reinterpreting a headerless map node as a - // `W_TypeObject`. These seven plus the nine `Function` fields + // `W_TypeObject`. These nine plus the nine `Function` fields // `function_quasi_immut_slot` resolves are every quasi-immutable descr this // binary mints — see the same reasoning on `state.rs // install_quasiimmut_field`. @@ -6421,6 +6423,16 @@ pub(crate) fn register_quasi_immutable_deps(_green_key: u64) { dep_ptr as *const _, &flag, ); + } else if field_index == property_fget { + pyre_object::descriptor::w_property_register_fget_watcher( + dep_ptr as pyre_object::PyObjectRef, + &flag, + ); + } else if field_index == property_fset { + pyre_object::descriptor::w_property_register_fset_watcher( + dep_ptr as pyre_object::PyObjectRef, + &flag, + ); } else if let Some(slot) = pyre_jit_trace::descr::function_quasi_immut_slot(field_index) { pyre_interpreter::function::function_register_quasi_immut_watcher( diff --git a/pyre/pyre-object/src/descriptor.rs b/pyre/pyre-object/src/descriptor.rs index fda1a44c04e..42a618c5535 100644 --- a/pyre/pyre-object/src/descriptor.rs +++ b/pyre/pyre-object/src/descriptor.rs @@ -156,9 +156,13 @@ mod super_tests { /// Python property descriptor object. /// -/// Layout: `[ob_type | fget | fset | fdel | w_doc | w_name | getter_doc]` +/// Layout: `[ob_type | fget | fset | fdel | w_doc | w_name | getter_doc | +/// fget_watchers | fset_watchers]` #[pyre_class("property", type_id = 19, static_name = "PROPERTY")] pub struct W_Property { + /// `descriptor.py:175 _immutable_fields_ = ["w_fget?", "w_fset?", + /// "w_fdel?"]` declares all three quasi-immutable; the hidden watcher + /// fields below implement the `?` for the two a fold bakes. pub fget: PyObjectRef, pub fset: PyObjectRef, pub fdel: PyObjectRef, @@ -175,6 +179,23 @@ pub struct W_Property { /// was copied from `fget.__doc__` (descriptor.py:196-204); `_copy` /// uses it to drop the inherited doc when the getter is replaced. pub getter_doc: bool, + /// The hidden `mutate_w_fget` field for `descriptor.py:175 + /// _immutable_fields_ = ["w_fget?", ...]` — see [`crate::quasiimmut`]. + /// + /// Holds no GC pointers, so the derived `PTR_OFFSETS` has nothing to walk + /// here. The allocation is [`crate::gc_hook::try_gc_alloc_stable_raw`], + /// i.e. non-moving, which is [`crate::quasiimmut::QuasiImmutField`]'s + /// stated precondition: the lock cannot be remapped out from under a + /// holder. A property the collector reclaims without a prior invalidation + /// leaks its instance box, the same bounded leak `W_TypeObject` carries, + /// because a GC object's `Drop` never runs. + /// + /// `w_fdel?` is declared upstream on the same line and gets no watcher + /// here: no fold bakes `fdel`, so nothing would ever register on it. A + /// `__delete__` fold must add the third one rather than bake without it. + pub fget_watchers: crate::quasiimmut::QuasiImmutField, + /// The `w_fset?` twin of [`Self::fget_watchers`]. + pub fset_watchers: crate::quasiimmut::QuasiImmutField, } /// Allocate a new property object. @@ -214,6 +235,8 @@ pub fn w_property_new(fget: PyObjectRef, fset: PyObjectRef, fdel: PyObjectRef) - w_doc: PY_NULL, w_name: PY_NULL, getter_doc: false, + fget_watchers: crate::quasiimmut::QuasiImmutField::new(), + fset_watchers: crate::quasiimmut::QuasiImmutField::new(), }, ); } @@ -228,6 +251,8 @@ pub fn w_property_new(fget: PyObjectRef, fset: PyObjectRef, fdel: PyObjectRef) - w_doc: PY_NULL, w_name: PY_NULL, getter_doc: false, + fget_watchers: crate::quasiimmut::QuasiImmutField::new(), + fset_watchers: crate::quasiimmut::QuasiImmutField::new(), }) } @@ -269,6 +294,19 @@ pub unsafe fn w_property_reinit( fdel: PyObjectRef, ) { let prop = obj as *mut W_Property; + // `rclass.py:715-718 hook_setfield` emits `jit_force_quasi_immutable` + // ahead of every store to a `?` field, so the accessors this replaces stop + // being trace constants before they stop being the live values. Nothing + // else revokes them: re-initialising an installed descriptor changes no + // type's version tag, which is the only other pin a fold over `obj.name` + // holds. The `is_installed` test is `pyjitpl.py:1112`'s + // `mutatebox.nonnull()` — a property no loop watches pays one load. + if (*prop).fget != fget && (*prop).fget_watchers.is_installed() { + crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fget_watchers); + } + if (*prop).fset != fset && (*prop).fset_watchers.is_installed() { + crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fset_watchers); + } (*prop).fget = fget; (*prop).fset = fset; (*prop).fdel = fdel; @@ -278,6 +316,69 @@ pub unsafe fn w_property_reinit( crate::gc_hook::try_gc_write_barrier(obj as *mut u8); } +/// `quasiimmut.py:116-126 get_current_qmut_instance` for +/// `descriptor.py:175`'s `w_fget?` — install the instance at RECORD time so a +/// write reached later in the same trace sees it. The +/// [`w_type_install_quasi_immut`](crate::typeobject::w_type_install_quasi_immut) +/// shape. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_install_fget_watcher(obj: PyObjectRef) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fget_watchers + .ensure_installed(); +} + +/// The `w_fset?` twin of [`w_property_install_fget_watcher`]. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_install_fset_watcher(obj: PyObjectRef) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fset_watchers + .ensure_installed(); +} + +/// `quasiimmut.py:72-75 register_loop_token` for `w_fget?` — record a compiled +/// loop's invalidation flag so `property.__init__` revokes it. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_register_fget_watcher( + obj: PyObjectRef, + flag: &std::sync::Arc, +) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fget_watchers + .register_loop_token(flag); +} + +/// The `w_fset?` twin of [`w_property_register_fget_watcher`]. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_register_fset_watcher( + obj: PyObjectRef, + flag: &std::sync::Arc, +) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fset_watchers + .register_loop_token(flag); +} + /// `descriptor.py:249-250 W_Property.get_doc` — returns the raw slot /// (NULL plays None; the caller wraps). /// # Safety From 6152f1dbcf50b2c13492ffc8d5609460f36dbf59 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 10:58:22 +0900 Subject: [PATCH 12/24] reconstructed_all_ref_call_stack: drop `call_kw` from the whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function publishes an aborting residual's Ref argument list as the caller's Python operand stack, which is only sound where the two orders agree. `call_kw`'s wire layout is `(callable, null_or_self, kwnames, arg0..arg{n-1})` — `majit-ir` `effectinfo.rs` `PyreHelperKind::CallKw` and `codewriter.rs`'s `op_args` build — while the stack `CALL_KW` pops is `[callable, null_or_self, arg0..arg{n-1}, kwnames]`, since `eval.rs call_kw` pops `kwnames` first. The list is a permutation of the image, not the image. A real `CALL_KW` always carries a non-empty kwnames tuple, so `n >= 1` on every reachable path and the two orders never coincide. The flush's only structural check is a depth compare (`state.rs:5524`), which a permutation of the right length passes, so the resumed interpreter would pop `arg{n-1}` as its keyword-name tuple. Declining is the same strictly-narrowing remedy the LOAD_ATTR/STORE_ATTR entries took: the operand image comes from the per-slot resume sources instead. `call_function_ex`'s list does match its stack, so it stays. No repro constructed — the shape needs an inline sub-walk to abort under a `CALL_KW` — so this rests on the two layouts rather than on an oracle. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 7a76426b2b6..db2048ac944 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -1588,17 +1588,29 @@ pub(crate) fn emit_walker_loop_callee_call_assembler( /// single-frame snapshot (`entry_py_pc` / `outer_active_boxes`), which is /// sound for side-effect-free leaves (re-execute the whole call on deopt). /// -/// That layout is a property of the CALL-family helpers alone, so the residual -/// this reads from has to be one of them. Every other entry the inline lever -/// serves passes its own receiver-plus-metadata list, not an operand-stack -/// image: `load_attr_fn(obj, code, name_idx)` is `r_args = [obj, code]` and -/// `store_attr_fn` is `[obj, value, code]`, whose `code` operand is a code -/// object the Python stack never held. Publishing it as a stack slot resumes -/// the interpreter with the code object where the receiver belongs — the -/// `__getattr__`/`property` folds returned `AttributeError: 'code' object has -/// no attribute ` for an attribute their own hook answers. Decline for -/// those instead; their operand image comes from the per-slot resume sources +/// That layout is a property of a subset of the CALL-family helpers, so the +/// residual this reads from has to be one of them. Every other entry the +/// inline lever serves passes its own receiver-plus-metadata list, not an +/// operand-stack image: `load_attr_fn(obj, code, name_idx)` is +/// `r_args = [obj, code]` and `store_attr_fn` is `[obj, value, code]`, whose +/// `code` operand is a code object the Python stack never held. Publishing it +/// as a stack slot resumes the interpreter with the code object where the +/// receiver belongs — the `__getattr__`/`property` folds returned +/// `AttributeError: 'code' object has no attribute ` for an attribute +/// their own hook answers. Decline for those instead; their operand image +/// comes from the per-slot resume sources /// ([`reconstructed_call_stack_from_resume_sources`]). +/// +/// `call_kw` is excluded for the same reason one step subtler: its list is a +/// PERMUTATION of the stack rather than a different set of values. The wire +/// order is `(callable, null_or_self, kwnames, arg0..arg{n-1})` +/// (`majit-ir effectinfo.rs` `PyreHelperKind::CallKw`), while `CALL_KW` pops +/// `kwnames` FIRST (`eval.rs call_kw`), so the stack image is +/// `[callable, null_or_self, arg0..arg{n-1}, kwnames]`. A real `CALL_KW` +/// always carries a non-empty kwnames tuple, so `n >= 1` on every reachable +/// path and the two orders never coincide. The flush's only structural check +/// is a depth compare, which a permutation of the right length passes, and the +/// re-executed `CALL_KW` would then pop `arg{n-1}` as its keyword-name tuple. pub(crate) fn reconstructed_all_ref_call_stack( code: &[u8], op: &DecodedOp, @@ -1607,9 +1619,7 @@ pub(crate) fn reconstructed_all_ref_call_stack( ) -> Option> { if !matches!( call_descr.get_extra_info().pyre_helper, - majit_ir::PyreHelperKind::CallFn - | majit_ir::PyreHelperKind::CallKw - | majit_ir::PyreHelperKind::CallFunctionEx + majit_ir::PyreHelperKind::CallFn | majit_ir::PyreHelperKind::CallFunctionEx ) { return None; } @@ -1650,8 +1660,8 @@ pub(crate) fn reconstructed_all_ref_call_stack( // Only `null_or_self@1` may be null, and the layout above names it by // index, so it is checked by position rather than by admitting a null // anywhere. Everything else here is a Python value the rewound `CALL` - // pops — an argument, or `kwnames` in the `call_kw` layout — and a null in - // one of those slots is an UNRESOLVED register, not a value: the concrete + // pops, and a null in one of those slots is an UNRESOLVED register, not a + // value: the concrete // Ref bank holds `Ref(null)` for a box the walk never materialized, which // is why `concrete_ref_for_color` tests for it and why the prefix loop // above declines on it. Publishing one lets the resumed interpreter pop a From fd5ea02f43df1008d1760c79891fe74333279d67 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 17:51:57 +0900 Subject: [PATCH 13/24] jit: pin `code?` on the inline arm that emits no value guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `function.py:47 _immutable_fields_ = ['code?', ...]`. The inline lever bakes `code` in the strongest form there is — it selects which callee jitcode the trace walks into — but only re-proved it with a per-iteration `getfield_gc_r` + `guard_value`, and only on the arm where the pinned operand IS the resolved function. c2a02f4bb77 restricted that arm to a non-constant callable because the reads would otherwise dereference a baked `ConstPtr`. #1336 declared the field `quasi("code", ...)` and wired `function_quasi_immut_slot` into both `install_quasiimmut_field` and `register_quasi_immutable_deps`, but left the other arm as it was; its own comment says so — "this only gives up the `f.__code__ = g.__code__` re-check on that path". Pin it there. `walker_pin_function_code` records a `QUASIIMMUT_FIELD` marker on `function_code_descr()` plus one `GUARD_NOT_INVALIDATED` per trace, which needs no runtime read at all — that is what makes it available on the arm the guard form is not: a constant callable, and a specializer that dispatched on some other object. The compile-time registrar resolves the owner by the raw address the optimizer recorded, so a callee the collector can relocate would be registered through a stale pointer. The jitcode `MAKE_FUNCTION` lowering allocates its function in the nursery, unlike `function_new_impl`'s `try_gc_alloc_stable_raw`, so the lever refuses the inline when `rgc.can_move` answers true for that arm. `extra_tests/parity_tests/function_code_reassigned_midloop.py`: a 40000 iteration loop reassigning `__code__` at the halfway point printed 40499 instead of 10019501 for a module-level callee. The method shape and the fresh `MAKE_FUNCTION` callee (which keeps the value guard) are in the same fixture. Assisted-by: Claude --- .../function_code_reassigned_midloop.py | 86 +++++++++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 80 ++++++++++------- .../src/jitcode_dispatch/mod.rs | 32 +++++++ 3 files changed, 168 insertions(+), 30 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py diff --git a/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py new file mode 100644 index 00000000000..88b5a2852dc --- /dev/null +++ b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py @@ -0,0 +1,86 @@ +# CPython-suite gap: `test_funcattrs` reassigns `__code__` and then calls the +# function once, never from inside a loop hot enough to have inlined the old +# body. +# parity-tests reason: this is a pyre JIT inline-lever regression. + +# `function.py:47 _immutable_fields_ = ['code?', 'w_func_globals?', +# 'closure?[*]', 'defs_w?[*]']`. The `?` is what lets the inline lever bake +# `code` and equally what registers the invalidation an assignment to the slot +# owes. The lever bakes it in the strongest form there is — `code` selects +# which callee body the trace walks into — so without the `?` a loop keeps +# running a body the function no longer has. +# +# The per-iteration `getfield_gc_r` + `guard_value` the lever emits elsewhere +# cannot stand in here: it reads the field off the pinned operand, and for a +# constant callable that operand is a baked `ConstPtr`. +# +# Each reassignment happens INSIDE its loop. A call after the loop is +# interpreted and would not consult what the trace baked. + +N = 40000 +SWITCH = N // 2 + + +def small(): + return 1 + + +def big(): + return 500 + + +def module_level_callee(): + # The constant-callable shape: `small` is resolved once and baked. + total = 0 + i = 0 + while i < N: + total += small() + if i == SWITCH: + small.__code__ = big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale __code__: %r != %r' % (total, expected) + + +class Holder: + def m(self): + return 1 + + +def m_big(self): + return 500 + + +def method_callee(): + # The method shape: the receiver's type version pins the descriptor, which + # says nothing about the function's own `code` slot. + obj = Holder() + total = 0 + i = 0 + while i < N: + total += obj.m() + if i == SWITCH: + Holder.m.__code__ = m_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale method __code__: %r != %r' % (total, expected) + + +def fresh_callee_still_inlines(): + # A `MAKE_FUNCTION` in the loop body allocates a fresh callee every + # iteration, so the lever keeps re-proving `code` off the live function + # instead. Here only to catch that arm being given up along the way. + total = 0 + i = 0 + while i < N: + def helper(x): + return x + 1 + total += helper(i) + i += 1 + assert total == N * (N + 1) // 2, 'fresh-callee inline changed answer: %r' % (total,) + + +module_level_callee() +method_callee() +fresh_callee_still_inlines() +print("OK") diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index db2048ac944..219341c7017 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -3174,6 +3174,41 @@ fn try_walker_inline_resolved_user_call_inner( if !positional_only && vararg_slot.is_none() { return Ok(None); } + // Not every caller pins the callee function itself. A specializer that + // resolves an app-level method behind a builtin — `str(e)` reaching an + // exception subclass's `__str__` — passes the CALL's own operand, which is + // the `str` builtin, while `callable` is the resolved `Function`. Reading + // `Function.code` off that operand is a type-confused load: it returns + // whatever sits at the same offset in a `PyCFunction`, so the guard + // compares a value that is not `code` and fails every iteration (99480 + // failures and 497 bridges on `synth/exception_subclass_attrs`, a 31x + // slowdown). Guard the fields only when the pinned object really is the + // function whose code this inline resolved. + let pinned_object_is_the_callee = unsafe { + (*callable_guard_value).ob_type as *const () as usize + == &pyre_interpreter::FUNCTION_TYPE as *const _ as usize + && pyre_interpreter::function_get_code(callable_guard_value) as usize + == w_code as pyre_object::PyObjectRef as usize + }; + // A trace-constant callable is excluded for a second reason: the field + // reads would dereference a baked `ConstPtr`, and loading through one + // dangles as soon as a minor collection moves the object + // (`synth/inline_subwalk_property_mutates` — a property getter that + // allocates on every iteration — segfaulted on cranelift under CI's macOS + // runner with the reads in place). Comparing against such a constant is + // fine; that is why the `code?` marker below covers this arm instead. + let guards_the_callee_function = + !callable_guard_op.is_constant() && pinned_object_is_the_callee; + if !guards_the_callee_function && majit_gc::can_move(majit_ir::GcRef(callable as usize)) { + // The arm below stands the baked code up on `function.py:47`'s `code?` + // instead of a per-iteration guard, and the marker names its owner by + // raw address at both record and compile time. The jitcode + // `MAKE_FUNCTION` lowering allocates its function in the nursery, so + // such a callee can be relocated between those two reads; refuse the + // inline rather than bake a body no invalidation covers. `rgc.can_move` + // parity — false when no moving GC is active. + return Ok(None); + } // `Function.funccall_valuestack` fills every parameter the call left // unbound from `defs_w` before entering the frame // (`function.py:188-193,217-231`); `Arguments.parse` reaches the same frame @@ -3967,35 +4002,6 @@ fn try_walker_inline_resolved_user_call_inner( } } - // Not every caller pins the callee function itself. A specializer that - // resolves an app-level method behind a builtin — `str(e)` reaching an - // exception subclass's `__str__` — passes the CALL's own operand, which is - // the `str` builtin, while `callable` is the resolved `Function`. Reading - // `Function.code` off that operand is a type-confused load: it returns - // whatever sits at the same offset in a `PyCFunction`, so the guard - // compares a value that is not `code` and fails every iteration (99480 - // failures and 497 bridges on `synth/exception_subclass_attrs`, a 31x - // slowdown). Guard the fields only when the pinned object really is the - // function whose code this inline resolved. - // - // A trace-constant callable is excluded for a second reason: the field - // reads would dereference a baked `ConstPtr`, and a baked constant object - // pointer is not GC-forwarded yet (gh #108 gc-table — see the note in - // `synth/exception_subclass_attrs.py`). Comparing against such a constant - // is fine, but loading through one dangles as soon as a minor collection - // moves the object: `synth/inline_subwalk_property_mutates` — a property - // getter that allocates on every iteration — segfaults on cranelift under - // CI's macOS runner with the reads in place. The callable being constant - // means something already pinned the object, so this only gives up the - // `f.__code__ = g.__code__` re-check on that path. - let guards_the_callee_function = !callable_guard_op.is_constant() - && unsafe { - (*callable_guard_value).ob_type as *const () as usize - == &pyre_interpreter::FUNCTION_TYPE as *const _ as usize - && pyre_interpreter::function_get_code(callable_guard_value) as usize - == callee_code_key - }; - // Keep the closure cells as red operands. A MAKE_FUNCTION in the caller's // loop creates a fresh function and fresh enclosing cells on every // iteration; the trace-time cell pointers are only the concrete shadow @@ -4021,6 +4027,16 @@ fn try_walker_inline_resolved_user_call_inner( .record_guard(OpCode::GuardValue, &[callable_guard_op, expected], 0); walker_capture_snapshot_for_last_guard(ctx, op.pc)?; } + // Pinning the operand pins none of the callee's fields, and this inline + // bakes `code` in the strongest form there is — it selects which callee + // body the trace walks into. `function.py:47 _immutable_fields_ = + // ['code?', ...]` is what covers that, and the `?` costs one marker + // plus one `GUARD_NOT_INVALIDATED` per trace instead of a load and a + // `GUARD_VALUE` per iteration. It is also the only form available + // here: the guard arm below reads the field off the pinned operand, + // which this arm either cannot do (the operand is not the callee) or + // must not do (a baked `ConstPtr`). + walker_pin_function_code(ctx, op.pc, callable)?; } else { // `function.py:91-96 getcode()` promotes `self.code`, never `self`. // The code object below and globals namespace in `InlineCalleeConsts` @@ -4035,7 +4051,11 @@ fn try_walker_inline_resolved_user_call_inner( // `opimpl_getfield_gc_r` pairs each read with `record_quasiimmut_field` // and assigning the field invalidates the traces that folded it; the // value guards stay on top as the stricter identity check the reads - // below assume. + // below assume. They also stay because this arm exists for a callee + // whose identity changes every iteration, so the field is re-read + // anyway, and because the marker resolves its owner by raw address — + // the guard is the only answer that keeps working for a callee the + // collector can relocate. // // Guarding the function OBJECT instead pinned its identity, which a // callee built by a `MAKE_FUNCTION` in the caller's own loop body can diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 679871790ea..a5d13978203 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -8654,6 +8654,38 @@ fn walker_pin_property_accessor( walker_flush_guard_not_invalidated(ctx, op_pc) } +/// The `function.py:47 _immutable_fields_ = ['code?', 'w_func_globals?', +/// 'closure?[*]', 'defs_w?[*]']` twin of [`walker_pin_property_accessor`]: pin +/// the callee's code slot when the inline lever cannot re-prove it per +/// iteration. +/// +/// The inline bakes `code` by choosing which callee jitcode to walk into, so +/// the value ends up spread across the inlined body rather than in one box a +/// `GUARD_VALUE` could re-check. Where the caller pins the callee function +/// itself, that guard still runs and is the cheaper answer for a callee whose +/// identity changes every iteration; everywhere else — a constant callable, or +/// a specializer that dispatched on some other object — this marker is what +/// makes `f.__code__ = g.__code__` revoke the loop. +/// +/// A marker, never a load: the owner is dereferenced only at record and +/// compile time, which is why this arm is clear of the baked-`ConstPtr` hazard +/// that keeps the guard arm off a constant callable. Both reads resolve the +/// owner by raw address, so the caller refuses a callee the collector can +/// relocate. +fn walker_pin_function_code( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + callable: pyre_object::PyObjectRef, +) -> Result<(), DispatchError> { + let callable_const = ctx.trace_ctx.const_ref(callable as i64); + crate::state::record_quasiimmut_field( + ctx.trace_ctx, + callable_const, + crate::descr::function_code_descr(), + ); + walker_flush_guard_not_invalidated(ctx, op_pc) +} + /// The `celldict.py:34 _immutable_fields_ = ["version?"]` twin of /// [`walker_pin_type_version_tag`]: pin the module namespace's strategy version /// so the folds that bake a slot's stored cell (or the absence of a name) are From 8681735046beaaf98a6cad66983e16dcdd503abb Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 15:26:04 +0900 Subject: [PATCH 14/24] parity: cover the property-accessor and __getattr__-hook callees in the code? fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both folds resolve their callee to a trace constant, so they land on the same `try_walker_inline_resolved_user_call` arm the module-level callee does — the one that carries the `code?` marker rather than a per-iteration value guard. They were not measured before a3aca797c3d, so this records the shapes rather than a repro. parity: all pass (dynasm). Assisted-by: Claude --- .../function_code_reassigned_midloop.py | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py index 88b5a2852dc..71511c29059 100644 --- a/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py +++ b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py @@ -66,6 +66,62 @@ def method_callee(): assert total == expected, 'baked a stale method __code__: %r != %r' % (total, expected) +def getter_small(self): + return 1 + + +def getter_big(self): + return 500 + + +class WithProperty: + x = property(getter_small) + + +def property_accessor_callee(): + # The property fold resolves the accessor to a trace constant, so it lands + # on the same arm the module-level callee does. + obj = WithProperty() + total = 0 + i = 0 + while i < N: + total += obj.x + if i == SWITCH: + getter_small.__code__ = getter_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale accessor __code__: %r != %r' % (total, expected) + + +def hook_small(self, name): + return 1 + + +def hook_big(self, name): + return 500 + + +class WithHook: + pass + + +WithHook.__getattr__ = hook_small + + +def getattr_hook_callee(): + # The `__getattr__` fold resolves its callee the same way. + obj = WithHook() + total = 0 + i = 0 + while i < N: + total += obj.absent + if i == SWITCH: + hook_small.__code__ = hook_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale hook __code__: %r != %r' % (total, expected) + + def fresh_callee_still_inlines(): # A `MAKE_FUNCTION` in the loop body allocates a fresh callee every # iteration, so the lever keeps re-proving `code` off the live function @@ -82,5 +138,7 @@ def helper(x): module_level_callee() method_callee() +property_accessor_callee() +getattr_hook_callee() fresh_callee_still_inlines() print("OK") From 93b06da4a338bf5f2474f8a0d72a0504b99dba85 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 18 Aug 2026 21:23:34 +0900 Subject: [PATCH 15/24] blackhole: mark each multi-frame level's frame finished as it returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `convert_and_run_from_pyjitpl` released a level without performing the `frame_finished_execution` store `pyopcode.py:239-241 RETURN_VALUE` and `pyopcode.py:184 handle_operation_error` perform before leaving a frame. The walker performs it at the `*_return` jitcode ops (`finish_current_frame_execution`); the blackhole did not, so a callee frame that outlived the call read back as still executing. Add `on_leave_level` to `PyjitplBlackholeFrameConfig` and `drive_multi_frame_blackhole`, called from `run_forever_with_portal` at `blackhole.py:1759` with the level's `virtualizable_ptr`. The bottommost level does not reach it: it leaves through `handle_jitexception`'s propagating arm, which returns first, so its frame stays with the interpreter and its own exception-table search. Pyre wires it to `state::finish_blackhole_level_frame`, which skips a null pointer — the inlined level whose frame was never materialized. `on_leave_level` is only correct alongside `per_frame`, which is what makes `virtualizable_ptr` name the level's own frame; without it every level shares the portal's virtualizable and a nested level would name the frame above it. Documented on the field. The `walk_abort_adopted` exclusion for `LoopBearingCalleeInlineUnsupported` stays. Narrowing it to `blackhole_required: false` produces wrong code on `bench/synth/inline_subwalk_user_iterator` (`TypeError: ... 'int' and 'object'`) and `bench/synth/list_append_write_barrier_gc` (`stack underflow during interpreter peek`), and regresses `bench/synth/selfrec_tail_exception_unwind` (guard_failures 937 -> 5393); `PYRE_WALKABORT_OFF=1` is the control. The comment now names those two witnesses in place of the finished-flag reason this commit removes. Gates: check.py --backend dynasm ALL PASSED 441/441; parity_tests --dynasm-only all pass; cpython_tests 207 PASS / 1 FAIL, test.test_pickle, which fails identically with this change disabled (its fix, the `check_exc_match` w_class pin, is not an ancestor of this branch). Assisted-by: Claude --- majit/majit-metainterp/src/blackhole.rs | 29 ++++++++++++++++++++++- majit/majit-metainterp/src/jitdriver.rs | 4 ++++ pyre/pyre-jit-trace/src/state.rs | 31 +++++++++++++++++++++++++ pyre/pyre-jit-trace/src/trace.rs | 31 +++++++++++++++++-------- 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 2282cf83372..33f7afffc40 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -2645,7 +2645,7 @@ pub fn run_forever( bh: BlackholeInterpreter, current_exc: i64, ) -> JitException { - run_forever_with_portal(builder, bh, current_exc, None, None, None) + run_forever_with_portal(builder, bh, current_exc, None, None, None, None) } /// blackhole.py:1752 _run_forever with optional portal runner callback. @@ -2668,6 +2668,7 @@ pub fn run_forever_with_portal( mut current_exc: i64, portal_runner: Option<&dyn Fn(&JitException) -> Result<(BhReturnType, i64), JitException>>, on_enter_level: Option<&dyn Fn(i64)>, + on_leave_level: Option<&dyn Fn(i64)>, mut terminal_out: Option<&mut Option>, ) -> JitException { loop { @@ -2717,6 +2718,18 @@ pub fn run_forever_with_portal( // blackhole.py:1759 let next = bh.nextblackholeinterp.take(); + // `pyopcode.py:239-241 RETURN_VALUE` (`frame_finished_execution = True`) + // and `pyopcode.py:184 handle_operation_error` (the same store on the + // no-handler propagation): the level reached here has returned to its + // caller by one of those two routes, so its frame's execution is over. + // Threaded from the interpreter side for the same reason as + // `on_enter_level` — the transition is a property of the embedder's + // frame object, which majit-metainterp cannot name. The bottommost + // level never arrives: it leaves through `handle_jitexception`'s + // propagating arm, which returns above. + if let Some(on_leave_level) = on_leave_level { + on_leave_level(bh.virtualizable_ptr); + } builder.release_interp(bh); // blackhole.py:1760 // RPython: blackholeinterp = blackholeinterp.nextblackholeinterp @@ -2747,6 +2760,17 @@ pub struct PyjitplBlackholeFrameConfig<'a> { /// the resumed frame chain. Threaded from the interpreter side because /// majit-metainterp cannot reference `ExecutionContext`. pub on_enter_level: Option<&'a dyn Fn(i64)>, + /// The `frame_finished_execution` store `pyopcode.py:239-241 RETURN_VALUE` + /// and `pyopcode.py:184 handle_operation_error` perform before leaving a + /// frame. Threaded from the interpreter side for the same reason as + /// [`Self::on_enter_level`]; called once per level that returns to its + /// caller, with that level's `virtualizable_ptr`. + /// + /// Set it only alongside [`Self::per_frame`], which is what makes that + /// pointer name the level's OWN frame. Without it every level shares the + /// portal's virtualizable, and a nested level would hand back the frame + /// ABOVE it — marking a caller that is still running as finished. + pub on_leave_level: Option<&'a dyn Fn(i64)>, } pub fn convert_and_run_from_pyjitpl( @@ -2761,6 +2785,7 @@ pub fn convert_and_run_from_pyjitpl( let mut next_bh: Option> = None; let roots_depth = majit_gc::shadow_stack::resume_ref_roots_depth(); let on_enter_level = config.as_ref().and_then(|config| config.on_enter_level); + let on_leave_level = config.as_ref().and_then(|config| config.on_leave_level); for (frame_index, frame) in framestack.frames.iter().enumerate() { let mut cur_bh = builder.acquire_interp(); @@ -2807,6 +2832,7 @@ pub fn convert_and_run_from_pyjitpl( current_exc, None, on_enter_level, + on_leave_level, terminal_out, ); majit_gc::shadow_stack::pop_resume_ref_roots_to(roots_depth); @@ -4222,6 +4248,7 @@ mod tests { Some(&portal_runner), None, None, + None, ); assert!( matches!( diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index 913297939e4..43a6b1293cc 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -310,6 +310,7 @@ pub fn drive_multi_frame_blackhole( raising_exception: bool, per_frame: Option<&[(i64, usize)]>, on_enter_level: Option<&dyn Fn(i64)>, + on_leave_level: Option<&dyn Fn(i64)>, ) -> MultiFrameBlackholeResult { let mut ref_locations = Vec::new(); let mut packed_ref_roots = Vec::new(); @@ -361,6 +362,7 @@ pub fn drive_multi_frame_blackhole( virtualizable_stack_base, per_frame, on_enter_level, + on_leave_level, }), Some(&mut terminal), ); @@ -2308,6 +2310,7 @@ impl JitDriver { raising_exception, None, None, + None, ); let MultiFrameBlackholeResult { outcome, terminal } = outcome; if crate::majit_log_enabled() { @@ -8053,6 +8056,7 @@ impl JitDriver { }), None, None, + None, ); // compile.py:716 assert 0, "unreachable" if crate::majit_log_enabled() { diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index e5f5e6e6387..ea68a569948 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -1019,6 +1019,37 @@ pub fn pyjitcode_for_jitcode_index(jitcode_index: i32) -> Option( // carrier, which resumes the OUTER frame at its CALL rather than // inside the discarded callee attempt. The nested-residual variant // marked `blackhole_required: true` owns a complete per-frame image - // and so passes `leaves_complete_image`, but the handoff finishes the - // callee inside the blackhole, which has no counterpart to - // `PyFrame.finish_value`'s `frame_finished_execution` store — the - // walker emits that store itself (`finish_current_frame_execution`) - // and the interpreter performs it on RETURN_VALUE, while the - // blackhole does neither. A frame that outlives the call then reads - // back as still executing, which `parity_tests/` - // `jit_inline_traceback_frame_clear.py` catches on - // `sys._getframe().clear()` once the loop compiles. Restore the - // carrier for it until the blackhole can publish that transition; + // and so passes `leaves_complete_image`, but the image it hands the + // blackhole is not a valid forward resume for every shape that reaches + // it. Two `bench/synth` fixtures are the standing witnesses, both + // wrong-code rather than a decline: `inline_subwalk_user_iterator` + // (the inlined callee's return value comes back as an untyped ref, so + // the caller's `acc += v` raises `TypeError: ... 'int' and 'object'`) + // and `list_append_write_barrier_gc` (`stack underflow during + // interpreter peek` — the resumed frame's operand stack is short). + // `PYRE_WALKABORT_OFF=1` is the control: both pass with the leg + // disabled. The `frame_finished_execution` store the handoff used to + // skip is NO LONGER one of the reasons — the drive now performs it at + // every level it leaves (`state::finish_blackhole_level_frame`, wired + // as `on_leave_level`), which is what + // `parity_tests/jit_inline_traceback_frame_clear.py` needs on + // `sys._getframe().clear()`. Restore the carrier for this variant + // until the two image defects above are closed; // `ForceQuasiImmutable` resumes AT the forcing opcode via // `flush_qmut_abort_state` (arm below), which re-runs the write the // walk stopped in front of instead of finishing the frame past it. From be73d90a8bb7aa599b996bdc5e815707b8dc7dcd Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 08:06:19 +0900 Subject: [PATCH 16/24] jit: build the loop region from the exception table instead of a pc interval `loop_region_end` returned one `header..=end` span and grew `end` to any backward jump whose target landed inside the span. 3.14 lays an out-of-line handler after the code that follows its `try`, so a `try`/`except` inside a loop puts the handler past everything between the loop and it, and the span grew across that gap. `loop_region_ranges` returns the loop body plus each rejoining handler's own range, taking the handler's start from the exception table. In `synth/range_ctor_in_loop` the grown span reached from the while header (unit 25) to the handler's `JUMP_BACKWARD_NO_INTERRUPT` (unit 406) and so covered the trailing comprehension's `FOR_ITER` at unit 323. That `FOR_ITER` is a `LIST_APPEND` body holding a `len(item)` call, which `for_iter_body_is_jit_safe_at` refuses, so both the whole-frame gate and the `escaping_range_append` region check declined and `main` ran interpreted end to end. The comprehension is not in the loop; the exception table's only entry covering loop pcs is `[117..130) -> 384`. `maybe_compile_and_run` asked `loop_region_contains_escaping_range_append` per back edge while `eval_with_jit_inner` asked `frame_has_traceable_escaping_range_loop` for the frame, so a frame the frame gate admitted still had every back edge but one refused. Traces closed on the inner `for` loops that could not then be entered, leaving their guards without a bridge target: 17896 guard failures and `bridge_no_targets_close=89` at N=20000, and the JIT ran the fixture slower than the interpreter. Both gates now read the frame-level answer, which admits no frame the frame gate did not already admit. `88930aa7029` dropped `loop_region_contains_escaping_range_append` entirely and admitted any frame with a safe loop region; that shipped wrong code (`for_iter_call_bearing_comprehension.py` lost a list element to a loop inside `random`) and was reverted. This keeps the predicate as the frame-level precondition, so no new frame is admitted. `range_ctor_in_loop` measures 22.2x dynasm / 30.2x cranelift / 24.7x wasm on this gate's metric, from 122x; its jitstats record 3 compiled loops and 3 bridges where they recorded 1 loop and 0 bridges, and its ceiling moves 190 -> 96. `inline_freevar_after_mayforce`'s cranelift `guard_failures` baseline reads 1009 on darwin, ubuntu and windows alike, so 1008 is re-recorded rather than banded. `PYRE_FBW_REPLAY_DIRTY_BODY` gets the gate-triage row `every_live_gate_has_a_triage_entry` asked for. Assisted-by: Claude --- ..._freevar_after_mayforce.cranelift.jitstats | 2 +- .../range_ctor_in_loop.cranelift.jitstats | 7 +- .../synth/range_ctor_in_loop.dynasm.jitstats | 7 +- pyre/bench/synth/range_ctor_in_loop.py | 18 ++- .../synth/range_ctor_in_loop.wasm.jitstats | 7 +- pyre/gate-triage.md | 5 +- pyre/pyre-jit/src/eval.rs | 137 ++++++++++++------ 7 files changed, 123 insertions(+), 60 deletions(-) diff --git a/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats b/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats index a53022ed6c8..1432ba1cbbb 100644 --- a/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats +++ b/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1008 +guard_failures=1009 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats b/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats b/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/bench/synth/range_ctor_in_loop.py b/pyre/bench/synth/range_ctor_in_loop.py index 00f253820a3..d580276e150 100644 --- a/pyre/bench/synth/range_ctor_in_loop.py +++ b/pyre/bench/synth/range_ctor_in_loop.py @@ -1,4 +1,4 @@ -# pyre-check: max-pypy-ratio=190 +# pyre-check: max-pypy-ratio=96 # Pins virtual range construction for one-, two-, and three-bound calls while # retaining correct residual behavior for exceptional, subclass, index, and # escaping-object shapes. @@ -9,11 +9,17 @@ # pypy spends 0.10s, clearing the floor even on the platform with the # coarsest timer. # -# The ceiling rose from 50 because that bound was fitted to the floored -# denominator, not because anything got slower: the honest ratio here is -# 83x as a median of interleaved pairwise runs. It is dominated by the four -# deliberately residual shapes below rather than by the virtualized loops -- -# each iteration also raises and catches a ValueError. +# `main` used to run interpreted end to end. The `try: range(0, 3, 0)` below +# puts an out-of-line handler after the trailing comprehension, and the loop +# region that gates the back edge grew across the gap between them and picked +# up that comprehension's call-bearing `FOR_ITER` -- an opcode this loop never +# reaches. With the region built from the exception table instead, the while +# loop and the three `for` loops compile, and this gate's own metric falls +# from 122x to 23.2x dynasm / 30.2x cranelift / 24.7x wasm. A separate +# min-of-five interleaved harness reads the same move as 158x to 35.2x / +# 38.3x, so the ceiling is set from the slower of the two readings: 2.5x of +# 38.3x, the slack every bench here carries against a runner 2.5x slower than +# an idle local box. N = 400000 diff --git a/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats b/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index 33e23b5119b..b40f83e5ecc 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -1014,7 +1014,7 @@ the folds it selects, not before them. `PYRE_FBW_SPEC_CENSUS` in §6c is its read-only half: the per-fold consulted/fired tallies. -### §6c — Default-OFF diagnostics, censuses and probes (66): keep, cost nothing +### §6c — Default-OFF diagnostics, censuses and probes (67): keep, cost nothing Each is inert unless set, so none is a removal target by this file's already-ON criterion. They are listed so they cannot be missed again. @@ -1027,7 +1027,8 @@ already-ON criterion. They are listed so they cannot be missed again. `PYRE_DYNASM_EXEC_DIAG`, `PYRE_FBW_CENSUS`, `PYRE_FBW_DEPTH_CENSUS`, `PYRE_FBW_INLINE_DIAG`, `PYRE_FBW_LOOPBODY_SCAN_FULL`, `PYRE_FBW_LOOPBODY_SCAN_LOOP_ONLY`, -`PYRE_FBW_MF_DIAG`, `PYRE_FBW_SPEC_CENSUS`, `PYRE_FBW_STRICT_DIAG`, +`PYRE_FBW_MF_DIAG`, `PYRE_FBW_REPLAY_DIRTY_BODY`, `PYRE_FBW_SPEC_CENSUS`, +`PYRE_FBW_STRICT_DIAG`, `PYRE_FIELD_IDENTITY_CENSUS`, `PYRE_FORITER_INFLIGHT_CENSUS`, `PYRE_FOR_ITER_GATE_DIAG`, `PYRE_GC_DIAG`, `PYRE_GC_FREELIST_DIAG`, `PYRE_JD1_DEBUG`, `PYRE_JD1_DUMP`, diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index dc65c277914..541eaf74f84 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -7402,38 +7402,89 @@ fn for_iter_bodies_all_jit_safe(code: &pyre_interpreter::CodeObject) -> bool { true } -/// Return the end of the natural loop region whose header is `loop_header_pc`. -/// Out-of-line exception handlers can rejoin the loop through a backward jump -/// to the middle of the body, so grow the region until every such rejoining -/// handler is included. -fn loop_region_end(code: &pyre_interpreter::CodeObject, loop_header_pc: usize) -> Option { +/// Return the pc ranges that make up the natural loop region whose header is +/// `loop_header_pc`: the loop body, plus every out-of-line exception handler +/// that rejoins the body through a backward jump. An empty result means +/// `loop_header_pc` has no backedge and so names no region. +/// +/// The region is a set of ranges rather than one span because a handler is laid +/// out after the code that follows its `try`, not next to the body it protects. +/// Whatever sits between the two — a later comprehension, a disjoint loop — +/// belongs to neither and cannot run in this backedge's trace, so covering the +/// gap would gate the backedge on `FOR_ITER`s it never reaches. The exception +/// table names where the out-of-line code begins, which is what lets a +/// rejoining jump widen the region back to its own handler instead of across +/// the gap. +fn loop_region_ranges( + code: &pyre_interpreter::CodeObject, + loop_header_pc: usize, +) -> Vec> { use pyre_interpreter::Instruction as I; - let mut region_end = None; - loop { - let previous_end = region_end; - let mut arg_state = pyre_interpreter::OpArgState::default(); - for (pc, unit) in code.instructions.iter().copied().enumerate() { - let (instr, op_arg) = arg_state.get(unit); - let target = match instr { - I::JumpBackward { delta } => { - Some(skip_caches(code, pc + 1).saturating_sub(delta.get(op_arg).as_usize())) - } - I::JumpBackwardNoInterrupt { delta } => { - Some((pc + 1).saturating_sub(delta.get(op_arg).as_usize())) - } - _ => None, - }; - let extends_region = match (target, region_end) { - (Some(target), _) if target == loop_header_pc => true, - (Some(target), Some(end)) => pc > end && (loop_header_pc..=end).contains(&target), - _ => false, - }; - if extends_region { - region_end = Some(region_end.map_or(pc, |end: usize| end.max(pc))); + + let mut backward_jumps: Vec<(usize, usize)> = Vec::new(); + let mut arg_state = pyre_interpreter::OpArgState::default(); + for (pc, unit) in code.instructions.iter().copied().enumerate() { + let (instr, op_arg) = arg_state.get(unit); + let target = match instr { + I::JumpBackward { delta } => { + Some(skip_caches(code, pc + 1).saturating_sub(delta.get(op_arg).as_usize())) + } + I::JumpBackwardNoInterrupt { delta } => { + Some((pc + 1).saturating_sub(delta.get(op_arg).as_usize())) } + _ => None, + }; + if let Some(target) = target { + backward_jumps.push((pc, target)); } - if region_end == previous_end { - return region_end; + } + + let Some(body_end) = backward_jumps + .iter() + .filter(|(_, target)| *target == loop_header_pc) + .map(|(pc, _)| *pc) + .max() + else { + return Vec::new(); + }; + + // The exception table is keyed by byte offset; pyre's `pc` is the + // instruction-unit index (two bytes per unit). Only the handlers laid out + // past the body can start an out-of-line block; one inside the body is + // already covered. + let mut handler_starts: Vec = + pyre_interpreter::pycode::decode_exceptiontable(&code.exceptiontable) + .map(|entry| entry.target as usize / 2) + .filter(|start| *start > body_end) + .collect(); + handler_starts.sort_unstable(); + + let mut ranges = vec![loop_header_pc..=body_end]; + loop { + let rejoins: Vec = backward_jumps + .iter() + .filter(|(pc, target)| { + !ranges.iter().any(|range| range.contains(pc)) + && ranges.iter().any(|range| range.contains(target)) + }) + .map(|(pc, _)| *pc) + .collect(); + if rejoins.is_empty() { + return ranges; + } + for pc in rejoins { + // Take the earliest handler that still starts at or before the + // jump: a handler runs on through the ones nested inside it, so + // the block this jump closes begins at the outermost of them. + // Without a handler to name a start the jump is not an out-of-line + // rejoin, and the span back to the body is kept whole rather than + // guessed at. + let start = handler_starts + .iter() + .copied() + .find(|start| *start <= pc) + .unwrap_or(body_end + 1); + ranges.push(start..=pc); } } } @@ -7447,12 +7498,16 @@ fn loop_region_for_iter_bodies_all_jit_safe( loop_header_pc: usize, ) -> bool { use pyre_interpreter::Instruction as I; - let Some(region_end) = loop_region_end(code, loop_header_pc) else { + let ranges = loop_region_ranges(code, loop_header_pc); + if ranges.is_empty() { return true; - }; + } let mut scan_state = pyre_interpreter::OpArgState::default(); - for pc in loop_header_pc..=region_end { - let (instr, _) = scan_state.get(code.instructions[pc]); + for (pc, unit) in code.instructions.iter().copied().enumerate() { + let (instr, _) = scan_state.get(unit); + if !ranges.iter().any(|range| range.contains(&pc)) { + continue; + } if matches!(instr, I::ForIter { .. }) && !for_iter_body_is_jit_safe_at(code, pc) { return false; } @@ -7478,20 +7533,18 @@ fn loop_region_contains_escaping_range_append( AwaitAppendCall, } - let Some(region_end) = loop_region_end(code, loop_header_pc) else { + let ranges = loop_region_ranges(code, loop_header_pc); + if ranges.is_empty() { return false; - }; + } let mut state = State::Searching; let mut decode = pyre_interpreter::OpArgState::default(); for (pc, unit) in code.instructions.iter().copied().enumerate() { let (instr, op_arg) = decode.get(unit); - if pc < loop_header_pc { + if !ranges.iter().any(|range| range.contains(&pc)) { continue; } - if pc > region_end { - break; - } match instr { I::LoadAttr { namei } if code.names[namei.get(op_arg).name_idx() as usize].as_str() == "append" => @@ -9109,7 +9162,7 @@ fn maybe_compile_and_run( let region_safe = cached_loop_region_for_iter_bodies_all_jit_safe(code, loop_header_pc); if !region_safe || (!cached_for_iter_bodies_all_jit_safe(code) - && !cached_loop_region_contains_escaping_range_append(code, loop_header_pc)) + && !frame_has_traceable_escaping_range_loop(code)) { return None; } @@ -13710,8 +13763,8 @@ mod tests { } let direct_end = direct_end.expect("fixture must contain the outer backedge"); - let region_end = loop_region_end(&code, outer_header).expect("loop must have a region"); - assert!(region_end > direct_end); + let ranges = loop_region_ranges(&code, outer_header); + assert!(ranges.iter().any(|range| *range.start() > direct_end)); assert!(!loop_region_for_iter_bodies_all_jit_safe( &code, outer_header From 921a655b8252163232a8a37e369174115ff78395 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 09:13:35 +0900 Subject: [PATCH 17/24] cranelift: skip the LABEL demoted-ref reload when no later LABEL reads it The LABEL header re-materialized every demoted ref from its root slot. The only reader of that SSA variable is a later LABEL that demotes the same raw: the loop body never `use_var`s a demoted ref, a guard exit reaches the value through `demoted_failarg_slots`, and a JUMP filters demoted positions out of its args. A `MemFlags::trusted()` load is not `readonly`, so the egraph keeps the ones with no reader and they stay in the header on every iteration. Emit the reload only for raws some later LABEL demotes, and take the pinned register lazily so a header with no such raw emits nothing. check.py cranelift 442/442; cargo test -p majit-backend-cranelift 36/36. Assisted-by: Claude --- majit/majit-backend-cranelift/src/compiler.rs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index eac2ebfca8b..866b0167ac0 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -10509,9 +10509,34 @@ impl CraneliftBackend { // earlier LABEL. Re-materialize it from the forwarded root // slot at this header so the fall-through transfer can pass // it onward without restoring a loop phi. + // + // That later LABEL is the only reader of the SSA variable. + // The loop body never `use_var`s a demoted ref — the reason + // `spill_ref_roots` and `reload_ref_roots` both skip one — + // a guard exit reaches the value through + // `demoted_failarg_slots`, and a JUMP filters demoted + // positions out of its args. With no later LABEL demoting + // the same raw the load has no use, and a load is never + // dead code to Cranelift: `MemFlags::trusted()` is not + // `readonly`, so the egraph keeps it, and it would sit in + // the header on every iteration of the loop. if let Some(positions) = demoted_ref_positions_by_label.get(&op_idx) { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); + let mut cur_jf = None; for &(_, raw, ofs) in positions { + let passed_on = + demoted_ref_positions_by_label + .iter() + .any(|(&later_idx, later)| { + later_idx > op_idx + && later + .iter() + .any(|&(_, later_raw, _)| later_raw == raw) + }); + if !passed_on { + continue; + } + let cur_jf = *cur_jf + .get_or_insert_with(|| builder.ins().get_pinned_reg(ptr_type)); let value = builder.ins().load( cl_types::I64, MemFlagsData::trusted(), From d74cb2939c06c5af1f387bf023357343dd472fdb Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 17:52:55 +0900 Subject: [PATCH 18/24] Drop the equal-value skip from `w_property_reinit`; propagate _abc cache errors `rclass.py hook_setfield` emits `jit_force_quasi_immutable` before EVERY store to a `?` field and does not consult the value being written, so re-initialising an installed accessor with the value it already holds is an invalidation too. `w_property_reinit` skipped it, which left a fold baked over a slot whose accessor had just been reassigned to its own current value. `_abc`'s `cache_attr` read every attribute-access failure as a cache miss. Only `AttributeError` means "no cache"; a descriptor or metaclass hook that raises anything else is observable and now propagates. `function_set_func_code` needs no such change: #1336 already writes `function_notify_quasi_immut(obj, QuasiImmutSlot::Code)` unconditionally. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_abc/mod.rs | 27 ++++++++++++-------- pyre/pyre-object/src/descriptor.rs | 16 +++++++----- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index cc46d6e585a..5a0cd821d39 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -76,7 +76,7 @@ fn weak_cache_contains( return Ok(false); }; let probe_slot = roots.publish(&[probe]); - let cache = cache_attr(roots.get(cls_slot), name); + let cache = cache_attr(roots.get(cls_slot), name)?; if cache.is_null() || !unsafe { is_set(cache) } { return Ok(false); } @@ -100,7 +100,7 @@ fn weak_cache_add(cls: PyObjectRef, name: &str, item: PyObjectRef) -> Result<(), return Ok(()); }; let entry_slot = roots.publish(&[entry]); - let cache = cache_attr(roots.get(cls_slot), name); + let cache = cache_attr(roots.get(cls_slot), name)?; if cache.is_null() || !unsafe { is_set(cache) } { return Ok(()); } @@ -113,10 +113,15 @@ fn weak_cache_add(cls: PyObjectRef, name: &str, item: PyObjectRef) -> Result<(), /// The named cache attribute of `cls`, or null when it has none. Read fresh /// at every use: the walks between two reads run arbitrary Python, which can /// rebind the attribute and can move the set. -fn cache_attr(cls: PyObjectRef, name: &str) -> PyObjectRef { +/// +/// `app_abc.py:110` reads the slot as a plain attribute, so only its absence +/// is a miss. A metaclass hook that raises something else raises out of the +/// check rather than being read as a class with no cache. +fn cache_attr(cls: PyObjectRef, name: &str) -> Result { match crate::baseobjspace::getattr_str(cls, name) { - Ok(cache) => cache, - Err(_) => std::ptr::null_mut(), + Ok(cache) => Ok(cache), + Err(err) if err.kind == crate::PyErrorKind::AttributeError => Ok(std::ptr::null_mut()), + Err(err) => Err(err), } } @@ -125,12 +130,12 @@ fn cache_attr(cls: PyObjectRef, name: &str) -> PyObjectRef { /// `int`, reports generation 0, which is below every counter value a /// registration produces — so its negative cache is discarded rather than /// trusted. -fn negative_cache_version(cls: PyObjectRef) -> u64 { - let version = cache_attr(cls, "_abc_negative_cache_version"); +fn negative_cache_version(cls: PyObjectRef) -> Result { + let version = cache_attr(cls, "_abc_negative_cache_version")?; if version.is_null() || !unsafe { is_int(version) } { - return 0; + return Ok(0); } - unsafe { w_int_get_value(version) }.max(0) as u64 + Ok(unsafe { w_int_get_value(version) }.max(0) as u64) } // `_py_abc.ABCMeta.__new__` (`_py_abc.py:48`) gives every ABC its OWN @@ -329,7 +334,7 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result Result { fn reset_caches(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { for name in ["_abc_cache", "_abc_negative_cache"] { - let cache = cache_attr(cls, name); + let cache = cache_attr(cls, name)?; if !cache.is_null() && unsafe { is_set(cache) } { unsafe { w_set_clear(cache) }; } diff --git a/pyre/pyre-object/src/descriptor.rs b/pyre/pyre-object/src/descriptor.rs index 42a618c5535..2ad2379cdc5 100644 --- a/pyre/pyre-object/src/descriptor.rs +++ b/pyre/pyre-object/src/descriptor.rs @@ -296,15 +296,17 @@ pub unsafe fn w_property_reinit( let prop = obj as *mut W_Property; // `rclass.py:715-718 hook_setfield` emits `jit_force_quasi_immutable` // ahead of every store to a `?` field, so the accessors this replaces stop - // being trace constants before they stop being the live values. Nothing - // else revokes them: re-initialising an installed descriptor changes no - // type's version tag, which is the only other pin a fold over `obj.name` - // holds. The `is_installed` test is `pyjitpl.py:1112`'s - // `mutatebox.nonnull()` — a property no loop watches pays one load. - if (*prop).fget != fget && (*prop).fget_watchers.is_installed() { + // being trace constants before they stop being the live values. The hook + // precedes the store and does not consult it, so re-initialising a slot + // with the value it already holds invalidates as well. Nothing else + // revokes them: re-initialising an installed descriptor changes no type's + // version tag, which is the only other pin a fold over `obj.name` holds. + // The `is_installed` test is `pyjitpl.py:1112`'s `mutatebox.nonnull()` — a + // property no loop watches pays one load. + if (*prop).fget_watchers.is_installed() { crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fget_watchers); } - if (*prop).fset != fset && (*prop).fset_watchers.is_installed() { + if (*prop).fset_watchers.is_installed() { crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fset_watchers); } (*prop).fget = fget; From 1643b388b84b6946f24fd2f6c0839f843fa61171 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 11:22:34 +0900 Subject: [PATCH 19/24] _abc: hold the registry and both caches in SimpleWeakSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_abc_init` installed bare `set`s and the module wrapped entries in weakrefs itself, so `_get_dump` had nothing named `data` to hand out and returned an empty tuple, a collected class left its spent weakref behind as a member, and the registry was a strong `list` that kept every registered class alive. `app_abc.py:15-44 SimpleWeakSet` is ported as app-level source and installed the way `_contextvars` installs its own — the `_remove` callback closes over a weakref to the set, so it has to be built where a closure can be. The module now reaches all three collections through `add` / `in` / `clear` / iteration rather than through the set primitives. `_get_dump` returns the three `data` sets and the version; `_reset_registry` clears in place instead of rebinding. `_abc_instancecheck` asks `instance.__class__` and `type(instance)` separately, per `app_abc.py:108-121`: the positive cache is probed against the claimed class before the real type is read, and the two are checked in turn when they differ. `__subclasscheck__` goes through attribute lookup so an overriding `ABCMeta` subclass answers. `register` still admits a callable non-type — pyre's stdlib stubs register those (`Mapping.register(_contextvars.Context)`) — so the weak-referenceability test that upstream gets from its own `isinstance(subclass, type)` guard sits in Rust and `app_abc.py` stays verbatim. check.py dynasm 442/442, cranelift 442/442; parity all pass; cpython_tests 207 PASS / 0 FAIL, and test_abc / test_collections / test_functools / test_enum / test_dataclasses / test_contextlib / test_descr pass on demand; cargo test --all --features dynasm 153 binaries. Assisted-by: Claude --- .../src/module/_abc/app_abc.py | 42 ++ pyre/pyre-interpreter/src/module/_abc/mod.rs | 366 ++++++++++++------ 2 files changed, 279 insertions(+), 129 deletions(-) create mode 100644 pyre/pyre-interpreter/src/module/_abc/app_abc.py diff --git a/pyre/pyre-interpreter/src/module/_abc/app_abc.py b/pyre/pyre-interpreter/src/module/_abc/app_abc.py new file mode 100644 index 00000000000..5777fb78989 --- /dev/null +++ b/pyre/pyre-interpreter/src/module/_abc/app_abc.py @@ -0,0 +1,42 @@ +# `app_abc.py:15-44 SimpleWeakSet`. The registry and both caches are +# instances of this, so `_get_dump` can hand out the `data` sets and a +# collected entry drops itself through the callback the set installs. +# +# Held at app level rather than rebuilt over the raw set primitives because +# the callback closes over a weakref to the set: the discard has to run with +# the set still reachable but no longer keeping itself alive through it. +from _weakref import ref + + +class SimpleWeakSet: + def __init__(self, data=None): + self.data = set() + + def _remove(item, selfref=ref(self)): + self = selfref() + if self is not None: + self.data.discard(item) + + self._remove = _remove + + def __iter__(self): + # Weakref callback may remove entry from set. + # So we make a copy first. + copy = list(self.data) + for itemref in copy: + item = itemref() + if item is not None: + yield item + + def __contains__(self, item): + try: + wr = ref(item) + except TypeError: + return False + return wr in self.data + + def add(self, item): + self.data.add(ref(item, self._remove)) + + def clear(self): + self.data.clear() diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index 5a0cd821d39..345068dec3b 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -18,53 +18,48 @@ use std::sync::atomic::{AtomicU64, Ordering}; // `_abc_negative_cache_version` is compared against this on every check. static INVALIDATION_COUNTER: AtomicU64 = AtomicU64::new(0); -/// `ref(cls)` as `SimpleWeakSet` spells it (`app_abc.py:20`). This is the -/// interpreter-level `weakref.ref` object, not [`weakref::w_weakref_new`]'s -/// bare GC struct: the caches are ordinary sets, so an entry has to be a real -/// object with a type — one whose `__hash__` and `__eq__` go by referent, which -/// is what lets a probe find an entry recorded earlier. -/// -/// `get_or_make_weakref` returns the one weakref a class already has, so only -/// the first probe of a given class allocates. +/// The app-level `SimpleWeakSet` (`app_abc.py:15-44`), stashed at module init +/// the way `weakref_type` stashes its own. The registry and both caches are +/// instances of it, so the collection this module installs is the one +/// `_get_dump` describes and a collected member drops itself. +static SIMPLE_WEAK_SET_TYPE: std::sync::OnceLock = std::sync::OnceLock::new(); + +fn simple_weak_set_type() -> PyObjectRef { + *SIMPLE_WEAK_SET_TYPE + .get() + .expect("_abc.SimpleWeakSet must be installed at module init") as PyObjectRef +} + +/// `SimpleWeakSet()` — the empty collection `_abc_init` installs and the +/// invalidation in `subclass_of` rebinds to. +fn new_simple_weak_set() -> Result { + crate::call::call_function_impl_result(simple_weak_set_type(), &[]) +} + +/// Whether `cls` can be weak-referenced at all, which is what decides whether +/// a `SimpleWeakSet` can hold it. /// -/// `None` for a class that cannot be weak-referenced at all; the caller reads -/// that as "not cacheable" rather than raising, since the answer to the -/// subclass question does not depend on whether it can be remembered. -fn class_weakref(cls: PyObjectRef) -> Option { +/// `app_abc.py:39-40 add` has no such test — upstream reaches it only with a +/// real class, because `_abc_register` rejects everything else. Pyre admits a +/// callable non-type there (see `register`), so the test lives on this side of +/// the boundary rather than in the app-level source, which stays verbatim. +/// `__contains__` needs none: `app_abc.py:33-38` already reads a referent-less +/// item as absent. +fn can_weakref(cls: PyObjectRef) -> bool { use crate::module::_weakref::interp__weakref as wr; let roots = pyre_object::gc_roots::push_roots(); let cls_slot = roots.publish(&[cls]); - // Both calls below allocate, and an allocation can claim root slots of its - // own — so every slot index comes back from the `publish` that made it - // rather than from arithmetic on an earlier one, and every argument is read - // out of its slot rather than from a local copy. - let type_slot = roots.publish(&[wr::weakref_type()]); - let lifeline = wr::getlifeline(roots.get(cls_slot)).ok()?; - let lifeline_slot = roots.publish(&[lifeline]); - Some(wr::get_or_make_weakref( - roots.get(lifeline_slot), - roots.get(type_slot), - roots.get(cls_slot), - )) + wr::getlifeline(roots.get(cls_slot)).is_ok() } -/// `app_abc.py:33-38 SimpleWeakSet.__contains__` — `ref(item) in self.data`, a -/// set whose members are weakrefs, probed with a weakref to the same class. +/// `app_abc.py:33-38 SimpleWeakSet.__contains__` through the membership +/// protocol, which is where the weakref probe and the `TypeError` fallback +/// live. /// -/// A missing or non-set attribute reads as "not cached" rather than raising: -/// `_abc_init` installs both caches, but an ABC built before this module (a +/// A missing collection reads as "not cached" rather than raising: +/// `_abc_init` installs all three, but an ABC built before this module (a /// pickled class, a hand-rolled `ABCMeta` subclass that skips `_abc_init`) -/// has neither, and such a class must still answer subclass checks. -/// -/// Takes the class and the attribute name rather than the set itself, so that -/// the set is read only after the probe exists: making the probe allocates, and -/// a reference read across an allocation names where the object used to be. -/// -/// `wr in self.data` goes through the membership protocol rather than -/// [`w_set_contains`]: a weakref hashes by running interpreter-level code -/// (`_weakref.ref.__hash__` hashes the referent and memoises the result), and -/// the raw set primitives document that a caller whose element hashes that way -/// owes them a digest taken while the operands are still rooted. +/// has none, and such a class must still answer subclass checks. fn weak_cache_contains( cls: PyObjectRef, name: &str, @@ -72,47 +67,60 @@ fn weak_cache_contains( ) -> Result { let roots = pyre_object::gc_roots::push_roots(); let cls_slot = roots.publish(&[cls]); - let Some(probe) = class_weakref(item) else { - return Ok(false); - }; - let probe_slot = roots.publish(&[probe]); + let item_slot = roots.publish(&[item]); let cache = cache_attr(roots.get(cls_slot), name)?; - if cache.is_null() || !unsafe { is_set(cache) } { + if cache.is_null() { return Ok(false); } - crate::baseobjspace::contains(cache, roots.get(probe_slot)) + let cache_slot = roots.publish(&[cache]); + crate::baseobjspace::contains(roots.get(cache_slot), roots.get(item_slot)) } -/// `app_abc.py:39-40 SimpleWeakSet.add` — `self.data.add(ref(item))`. Silently -/// declines a class with no cache slot, for the same reason -/// [`weak_cache_contains`] reads one as a miss, and calls the set's own `add` -/// for the same reason it uses the membership protocol. +/// `app_abc.py:39-40 SimpleWeakSet.add` — `self.data.add(ref(item, self._remove))`. +/// Called rather than open-coded so the entry carries the callback that +/// discards it once the referent dies; a bare `ref` would leave a spent one +/// behind for every class the check ever saw. /// -/// Upstream's `add` passes `ref()` a callback that discards the entry once the -/// referent dies; this does not, so a checked class that is later collected -/// leaves its spent weakref behind as a member. Such an entry answers no -/// probe — a weakref compares by referent and this one has none — and it keeps -/// no class alive, so what it costs is the weakref itself. +/// Silently declines a class with no collection, for the same reason +/// [`weak_cache_contains`] reads one as a miss, and one that cannot be +/// weak-referenced, for the reason [`can_weakref`] records. fn weak_cache_add(cls: PyObjectRef, name: &str, item: PyObjectRef) -> Result<(), crate::PyError> { let roots = pyre_object::gc_roots::push_roots(); let cls_slot = roots.publish(&[cls]); - let Some(entry) = class_weakref(item) else { + let item_slot = roots.publish(&[item]); + if !can_weakref(roots.get(item_slot)) { return Ok(()); - }; - let entry_slot = roots.publish(&[entry]); + } let cache = cache_attr(roots.get(cls_slot), name)?; - if cache.is_null() || !unsafe { is_set(cache) } { + if cache.is_null() { return Ok(()); } - let add = crate::baseobjspace::getattr_str(cache, "add")?; + let cache_slot = roots.publish(&[cache]); + let add = crate::baseobjspace::getattr_str(roots.get(cache_slot), "add")?; let add_slot = roots.publish(&[add]); - crate::call::call_function_impl_result(roots.get(add_slot), &[roots.get(entry_slot)])?; + crate::call::call_function_impl_result(roots.get(add_slot), &[roots.get(item_slot)])?; + Ok(()) +} + +/// `SimpleWeakSet.clear` (`app_abc.py:43-44`) on the named collection, in +/// place, so anything already holding it sees the clear. +fn weak_cache_clear(cls: PyObjectRef, name: &str) -> Result<(), crate::PyError> { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let cache = cache_attr(roots.get(cls_slot), name)?; + if cache.is_null() { + return Ok(()); + } + let cache_slot = roots.publish(&[cache]); + let clear = crate::baseobjspace::getattr_str(roots.get(cache_slot), "clear")?; + let clear_slot = roots.publish(&[clear]); + crate::call::call_function_impl_result(roots.get(clear_slot), &[])?; Ok(()) } -/// The named cache attribute of `cls`, or null when it has none. Read fresh -/// at every use: the walks between two reads run arbitrary Python, which can -/// rebind the attribute and can move the set. +/// The named collection attribute of `cls`, or null when it has none. Read +/// fresh at every use: the walks between two reads run arbitrary Python, which +/// can rebind the attribute and can move the object. /// /// `app_abc.py:110` reads the slot as a plain attribute, so only its absence /// is a miss. A metaclass hook that raises something else raises out of the @@ -146,17 +154,15 @@ fn negative_cache_version(cls: PyObjectRef) -> Result { // single registry). fn abc_init(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { - let fresh = w_list_new(vec![]); - crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; - // `app_abc.py:75-77` — the caches are per-class for the same reason the - // registry is: resolved up the MRO, one base's caches would answer for - // every descendant ABC, and a hit on `Rational` would satisfy - // `Integral`. Each value is built before the call that stores it, so - // that no allocation happens between reading `cls` and using it. - let cache = w_set_new(); - crate::baseobjspace::setattr_str(cls, "_abc_cache", cache)?; - let negative_cache = w_set_new(); - crate::baseobjspace::setattr_str(cls, "_abc_negative_cache", negative_cache)?; + // `app_abc.py:74-77` — registry and both caches are per-class for the + // same reason: resolved up the MRO, one base's would answer for every + // descendant ABC, and a hit on `Rational` would satisfy `Integral`. + // Each value is built before the call that stores it, so that no + // allocation happens between reading `cls` and using it. + for name in ["_abc_registry", "_abc_cache", "_abc_negative_cache"] { + let fresh = new_simple_weak_set()?; + crate::baseobjspace::setattr_str(cls, name, fresh)?; + } let version = w_int_new(INVALIDATION_COUNTER.load(Ordering::Relaxed) as i64); crate::baseobjspace::setattr_str(cls, "_abc_negative_cache_version", version)?; let mut abstract_names = Vec::new(); @@ -239,17 +245,14 @@ fn register(args: &[PyObjectRef]) -> Result { } else if !crate::baseobjspace::callable_w(subclass) { return Err(crate::PyError::type_error("Can only register classes")); } - let registry = match crate::baseobjspace::getattr_str(cls, "_abc_registry") { - Ok(r) if !unsafe { is_none(r) } => r, - _ => { - let fresh = w_list_new(vec![]); - crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; - fresh - } - }; - unsafe { - w_list_append(registry, subclass); + // `app_abc.py:99 cls._abc_registry.add(subclass)`. An ABC that never ran + // `_abc_init` has no collection to add to; it gets one here rather than + // dropping the registration. + if cache_attr(cls, "_abc_registry")?.is_null() { + let fresh = new_simple_weak_set()?; + crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; } + weak_cache_add(cls, "_abc_registry", subclass)?; // `app_abc.py:100-101` — invalidate every negative cache. A class this // registration now makes a subclass may already be recorded as a non-match // somewhere, and only the counter can reach those entries: they live on @@ -335,7 +338,7 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result Result rcls, + Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) => return Err(err), + }; + // A registered entry that is not a class cannot be a base + // class, so it can never make `subclass` a subclass — skip + // it rather than letting `issubclass` raise. `range` is + // registered to `Sequence` but is a builtin function in + // pyre, so without this guard a single bad entry aborts the + // whole recursive check. + if !unsafe { is_type(rcls) } { + continue; + } + let item_roots = pyre_object::gc_roots::push_roots(); + let rcls_slot = item_roots.base(); + item_roots.pin_root(rcls); + if crate::baseobjspace::issubclass( + roots.get(subclass_slot), + item_roots.get(rcls_slot), + )? { + break 'decide true; } } } @@ -457,24 +465,85 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result Result { if args.len() < 2 { return Ok(w_bool_from(false)); } - let cls = args[0]; - let instance = args[1]; - if unsafe { crate::baseobjspace::isinstance_w(instance, cls) } { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[args[0]]); + let instance_slot = roots.publish(&[args[1]]); + + // `app_abc.py:111 subclass = instance.__class__`. + let subclass = crate::baseobjspace::getattr_str(roots.get(instance_slot), "__class__")?; + let subclass_slot = roots.publish(&[subclass]); + if weak_cache_contains(roots.get(cls_slot), "_abc_cache", roots.get(subclass_slot))? { return Ok(w_bool_from(true)); } - // `type(instance)` — the instance's real class. User-defined instances - // carry the generic layout marker in `ob_type` and the real class in - // `w_class`, so reading `ob_type` directly would resolve to `object`; - // `r#type` returns the class for both builtin and user instances. - let subclass = crate::typedef::r#type(instance).map_or(std::ptr::null_mut(), |p| p.as_ptr()); - if subclass.is_null() { + + // `app_abc.py:113 subtype = type(instance)` — the instance's real class. + // User-defined instances carry the generic layout marker in `ob_type` and + // the real class in `w_class`, so reading `ob_type` directly would resolve + // to `object`; `r#type` returns the class for both builtin and user + // instances. + let subtype = crate::typedef::r#type(roots.get(instance_slot)) + .map_or(std::ptr::null_mut(), |p| p.as_ptr()); + if subtype.is_null() { return Ok(w_bool_from(false)); } - Ok(w_bool_from(subclass_of(cls, subclass)?)) + let subtype_slot = roots.publish(&[subtype]); + + if std::ptr::eq(roots.get(subtype_slot), roots.get(subclass_slot)) { + // `app_abc.py:115-117` — one class, so the negative cache can answer. + // The version test is `==`, not `<`: a cache recorded against a + // *later* counter than the one read here cannot describe this + // registry either. + if negative_cache_version(roots.get(cls_slot))? + == INVALIDATION_COUNTER.load(Ordering::Relaxed) + && weak_cache_contains( + roots.get(cls_slot), + "_abc_negative_cache", + roots.get(subclass_slot), + )? + { + return Ok(w_bool_from(false)); + } + return Ok(w_bool_from(subclasscheck_of( + roots.get(cls_slot), + roots.get(subclass_slot), + )?)); + } + // `app_abc.py:121 any(cls.__subclasscheck__(c) for c in (subclass, subtype))`. + for slot in [subclass_slot, subtype_slot] { + if subclasscheck_of(roots.get(cls_slot), roots.get(slot))? { + return Ok(w_bool_from(true)); + } + } + Ok(w_bool_from(false)) +} + +/// `cls.__subclasscheck__(subclass)` through attribute lookup, the way +/// `app_abc.py:118` and `:121` spell it, so an `ABCMeta` subclass that +/// overrides the hook is the one that answers. +fn subclasscheck_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let subclass_slot = roots.publish(&[subclass]); + let check = crate::baseobjspace::getattr_str(roots.get(cls_slot), "__subclasscheck__")?; + let check_slot = roots.publish(&[check]); + let result = + crate::call::call_function_impl_result(roots.get(check_slot), &[roots.get(subclass_slot)])?; + let result_slot = roots.publish(&[result]); + crate::baseobjspace::is_true(roots.get(result_slot)) } fn subclasscheck(args: &[PyObjectRef]) -> Result { @@ -491,7 +560,7 @@ fn subclasscheck(args: &[PyObjectRef]) -> Result { /// an outstanding `get_cache_token` survives a registry reset. fn reset_registry(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { - crate::baseobjspace::setattr_str(cls, "_abc_registry", w_list_new(vec![]))?; + weak_cache_clear(cls, "_abc_registry")?; } Ok(w_none()) } @@ -506,15 +575,42 @@ fn reset_registry(args: &[PyObjectRef]) -> Result { fn reset_caches(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { for name in ["_abc_cache", "_abc_negative_cache"] { - let cache = cache_attr(cls, name)?; - if !cache.is_null() && unsafe { is_set(cache) } { - unsafe { w_set_clear(cache) }; - } + weak_cache_clear(cls, name)?; } } Ok(w_none()) } +/// `_abc._get_dump(cls)` (`app_abc.py:165-173`): shallow copies of the +/// registry, both caches, and the negative-cache version. The three sets are +/// the collections' own `data`, which is why they are `SimpleWeakSet`s and not +/// bare sets — `ABC._dump_registry` prints what this returns. +fn get_dump(args: &[PyObjectRef]) -> Result { + let Some(&cls) = args.first() else { + return Ok(w_tuple_new(vec![])); + }; + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let mut items = Vec::with_capacity(4); + for name in ["_abc_registry", "_abc_cache", "_abc_negative_cache"] { + let cache = cache_attr(roots.get(cls_slot), name)?; + // A class that never ran `_abc_init` has nothing to describe; an empty + // set keeps the tuple's shape rather than raising at a debug helper. + let data = if cache.is_null() { + w_set_new() + } else { + let cache_slot = roots.publish(&[cache]); + crate::baseobjspace::getattr_str(roots.get(cache_slot), "data")? + }; + roots.publish(&[data]); + items.push(data); + } + items.push(w_int_new( + negative_cache_version(roots.get(cls_slot))? as i64 + )); + Ok(w_tuple_new(items)) +} + crate::py_module! { "_abc", functions: { @@ -523,8 +619,20 @@ crate::py_module! { "_abc_register" / 2 = register, "_abc_instancecheck" / 2 = instancecheck, "_abc_subclasscheck" / 2 = subclasscheck, - "_get_dump" / 1 = |_| Ok(w_tuple_new(vec![])), + "_get_dump" / 1 = get_dump, "_reset_registry" / 1 = reset_registry, "_reset_caches" / 1 = reset_caches, }, + extra_init: |ns| { + crate::importing::appleveldef_install_seeded( + ns, + include_str!("app_abc.py"), + "app_abc.py", + &["SimpleWeakSet"], + &[], + ); + let simple_weak_set = crate::module_ns_get(ns, "SimpleWeakSet") + .expect("_abc.SimpleWeakSet must be installed by appleveldefs"); + let _ = SIMPLE_WEAK_SET_TYPE.set(simple_weak_set as usize); + }, } From d3b521e66beee0edd774be45c1f3e2f8dbdd44e8 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 11:40:09 +0900 Subject: [PATCH 20/24] jit: admit a type call whose metaclass leaves __call__ alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline instantiation asked whether the metatype WAS `type`, while its comment asked whether the metatype overrides `__call__`. An `ABCMeta` subclass supplies `__instancecheck__`, `__subclasscheck__` and `register` and leaves `__call__` alone, so `typeobject.py:_type_call` is what runs for it — but the identity test refused every class built with one. The test now resolves `__call__` on the metatype and compares it to the one `type` supplies. The answer is a dict lookup, so it is pinned the way the `__new__` / `__init__` answers already are: the metaclass's own version tag, because a metaclass that gains a `__call__` does not move the class's tag. A metaclass whose dict changes are untracked declines. `abcmeta_type_call_inline.py` covers both arms — a class the change admits, and a `__call__` installed on its metaclass mid-loop, which must take over on the next iteration. This does not move `synth/inline_freevar_after_mayforce`: `Fraction` defines its own `__new__`, so it declines one check later, at `__new__ overridden`. No gated counter moves on either backend. check.py dynasm 442/442, cranelift 442/442; parity all pass; cpython_tests 207 PASS / 0 FAIL; cargo test --all --features dynasm 153 binaries. Assisted-by: Claude --- .../parity_tests/abcmeta_type_call_inline.py | 70 +++++++++++++++++++ .../src/jitcode_dispatch/inline_call.rs | 36 +++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py diff --git a/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py b/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py new file mode 100644 index 00000000000..ab4a7490138 --- /dev/null +++ b/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py @@ -0,0 +1,70 @@ +# CPython-suite gap: `test_abc` never instantiates an ABCMeta-built class in a +# loop hot enough to compile, and no suite test installs `__call__` on a +# metaclass after such a loop has run. +# `typeobject.py:_type_call` runs unless the metatype supplies a `__call__` of +# its own. An `ABCMeta` subclass supplies `__instancecheck__`, +# `__subclasscheck__` and `register` and leaves `__call__` alone, so a class +# built with one instantiates through the same path a plain class does. The +# inline emit used to ask whether the metatype WAS `type`, which refused every +# such class -- and `Fraction`, `Decimal` and every `collections.abc` subclass +# with it. +# +# parity-tests reason: the admission is only sound while the answer holds, and +# what makes it hold is a pin on the METACLASS's version tag -- the class's own +# tag does not move when its metaclass gains an attribute. A `__call__` +# installed on the metaclass mid-loop must take over on the next iteration, so +# this belongs where a stale answer is visible as a wrong number rather than as +# a missed optimisation. +# +# Each rebind happens INSIDE its loop: a call after the loop is interpreted and +# would not consult what the trace baked. +import abc + +N = 40000 +SWITCH = N // 2 + + +class Meta(abc.ABCMeta): + pass + + +class Point(metaclass=Meta): + def __init__(self, x): + self.x = x + + +class Fixed: + x = 7 + + +def inlines(): + # The plain shape: default `__new__`, an `__init__` the walk can enter, and + # a metaclass that overrides neither. + total = 0 + i = 0 + while i < N: + total += Point(i).x + i += 1 + assert total == N * (N - 1) // 2, 'wrong sum: %r' % (total,) + + +def metaclass_gains_call(): + total = 0 + i = 0 + while i < N: + total += Point(i).x + if i == SWITCH: + Meta.__call__ = lambda cls, x: Fixed() + i += 1 + # `i == SWITCH` is assigned before the rebind, so iterations 0..SWITCH read + # their own index and the rest read `Fixed.x`. + expected = SWITCH * (SWITCH + 1) // 2 + (N - SWITCH - 1) * 7 + assert total == expected, 'baked a stale metaclass __call__: %r != %r' % ( + total, + expected, + ) + + +inlines() +metaclass_gains_call() +print('OK') diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 219341c7017..e3d5f0d7a95 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -5647,9 +5647,33 @@ pub(crate) fn try_walker_inline_type_call( } // What follows is `type.__call__`. A metaclass that overrides `__call__` // runs instead of it and may return anything at all, so it stays residual. - if !std::ptr::eq(unsafe { (*w_type).w_class }, w_metatype) { - return type_call_decline("metaclass overrides __call__"); - } + // + // The question is which `__call__` the metatype resolves to, not whether it + // is `type` itself: `ABCMeta` supplies `__instancecheck__`, + // `__subclasscheck__` and `register` and leaves `__call__` alone, so every + // class that registers with a `numbers` / `collections.abc` ABC — which is + // every `Fraction`, `Decimal` and `deque` construction — resolves to the + // same `type.__call__` a plain class does. Comparing the metatype's + // identity refused all of them. + let w_metaclass = unsafe { (*w_type).w_class }; + let metaclass_to_pin = if std::ptr::eq(w_metaclass, w_metatype) { + None + } else { + let meta_call = + unsafe { pyre_interpreter::baseobjspace::lookup_in_type(w_metaclass, "__call__") }; + let type_call = + unsafe { pyre_interpreter::baseobjspace::lookup_in_type(w_metatype, "__call__") }; + if meta_call != type_call { + return type_call_decline("metaclass overrides __call__"); + } + // The answer above is a dict lookup, so it needs the same pin the + // `__new__` / `__init__` answers get. A metaclass whose dict changes + // are untracked cannot supply one. + if unsafe { pyre_object::typeobject::w_type_get_version_tag(w_metaclass) } == 0 { + return type_call_decline("metaclass has no version tag"); + } + Some(w_metaclass) + }; // A version tag of 0 is a type whose dict changes are not tracked, so the // `__new__` / `__init__` / `__del__` lookups below cannot be pinned. let version_tag = unsafe { pyre_object::typeobject::w_type_get_version_tag(w_type) }; @@ -5727,6 +5751,12 @@ pub(crate) fn try_walker_inline_type_call( .heap_cache_mut() .replace_box(r_args[0], type_const); walker_pin_type_version_tag(ctx, op.pc, type_const)?; + // A metaclass that does not override `__call__` today can be given one, and + // that changes its own version tag rather than the class's. + if let Some(w_metaclass) = metaclass_to_pin { + let metaclass_const = ctx.trace_ctx.const_ref(w_metaclass as i64); + walker_pin_type_version_tag(ctx, op.pc, metaclass_const)?; + } // The walker is the executor here, so the instance the rest of this walk // reads has to be a real one — the same split `trace_box_int` makes between From cf19b58223789fab1fcb6ff66fe3472ad7ba2c61 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 13:33:44 +0900 Subject: [PATCH 21/24] inline diag: name the decline site and the deferred-admit term that refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `try_walker_inline_resolved_user_call_inner` has 37 statement-level declines and every call shape reaches them, so a caller that logged only "callee inline declined" left the reader to guess which test refused. Each now reports its own `inline_call.rs:` under the existing `PYRE_FBW_INLINE_DIAG`, and the FOR_ITER deferred-admit conjunction reports which of its five terms was false rather than only its result. On `synth/inline_freevar_after_mayforce` this names the chain in one run: `forward` declines at the FOR_ITER gate with `safety=Dirty`, and the body is dirty because `Fraction(2, 89)` is an opaque residual — `[type-call-decline] __new__ overridden`, since `Fraction` defines its own `__new__` and the emit builds only the `object.__new__` layout. The nested `adjust` declines on `boundary=false`, which is the walk position rather than a predicate. So the whole arithmetic chain stays residual behind one root, and the JIT buys 1.38x over the interpreter there against pypy's 24x. check.py dynasm 442/442, cranelift 442/442; parity all pass; cpython_tests 207 PASS / 0 FAIL; cargo test --all --features dynasm 153 binaries. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 97 ++++++++++++------- 1 file changed, 60 insertions(+), 37 deletions(-) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index e3d5f0d7a95..8357e02d947 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -3135,6 +3135,19 @@ pub(crate) fn try_walker_inline_resolved_user_call( /// user call nested inside a specialized builtin: it deliberately leaves the /// outer residual's destination untouched, so guards still snapshot the /// caller at the builtin-call boundary. +/// Report which decline in [`try_walker_inline_resolved_user_call_inner`] a +/// call hit. The function has three dozen of them and reaches them from every +/// call shape, so a caller that only learns "declined" has to guess; the +/// `[binop-inline-decline]` and `[type-call-decline]` lines above name the +/// call, and this names the test inside it that refused. +#[inline] +fn resolved_inline_decline(op_pc: usize, line: u32) -> Result, DispatchError> { + if fbw_inline_diag_enabled() { + eprintln!("[resolved-inline-decline] pc={op_pc} inline_call.rs:{line}"); + } + Ok(None) +} + #[allow(clippy::too_many_arguments)] fn try_walker_inline_resolved_user_call_inner( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3172,7 +3185,7 @@ fn try_walker_inline_resolved_user_call_inner( let positional_only = fbw_callee_scope_is_positional_only(w_code); let vararg_slot = fbw_callee_vararg_slot(w_code); if !positional_only && vararg_slot.is_none() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // Not every caller pins the callee function itself. A specializer that // resolves an app-level method behind a builtin — `str(e)` reaching an @@ -3207,7 +3220,7 @@ fn try_walker_inline_resolved_user_call_inner( // such a callee can be relocated between those two reads; refuse the // inline rather than bake a body no invalidation covers. `rgc.can_move` // parity — false when no moving GC is active. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // `Function.funccall_valuestack` fills every parameter the call left // unbound from `defs_w` before entering the frame @@ -3231,7 +3244,7 @@ fn try_walker_inline_resolved_user_call_inner( let Some(defaults) = (unsafe { positional_defaults_for_inline(callable, &missing, nparams) }) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; callee_args.resize(nparams, OpRef::NONE); callee_arg_concretes.resize(nparams, ConcreteValue::Null); @@ -3253,22 +3266,22 @@ fn try_walker_inline_resolved_user_call_inner( // below; folding the placeholder into the tuple would put the Method // object where the receiver belongs. Decline that one shape. if bound_method.is_some() && nparams == 0 { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if callee_arg_concretes.len() != callee_args.len() || callee_args.len() <= nparams { // The empty tuple is a runtime singleton (`() is tuple([])`), so a // freshly allocated walker tuple would not be the object the // interpreter installs for a zero-surplus call. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let surplus_ops: Vec = callee_args[nparams..].to_vec(); let mut surplus_concretes = Vec::with_capacity(surplus_ops.len()); for concrete in &callee_arg_concretes[nparams..] { let ConcreteValue::Ref(obj) = *concrete else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if obj.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } surplus_concretes.push(obj); } @@ -3276,7 +3289,7 @@ fn try_walker_inline_resolved_user_call_inner( // same constructor `emit_object_tuple_inline` reproduces. let concrete = pyre_object::w_tuple_new_array_backed(surplus_concretes); if concrete.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } callee_args.truncate(nparams); callee_arg_concretes.truncate(nparams); @@ -3312,38 +3325,38 @@ fn try_walker_inline_resolved_user_call_inner( // needs fresh cell allocation and stays residual until that constructor // half is ported too. if callee_args.len() != seeded_locals { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let raw_callee_code = unsafe { pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) as *const pyre_interpreter::CodeObject }; if raw_callee_code.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let callee_code = unsafe { &*raw_callee_code }; let mut concrete_freevar_cells = Vec::new(); let concrete_closure = if has_closure { if !callee_code.cellvars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let closure = unsafe { pyre_interpreter::function_get_closure(callable) }; if closure.is_null() || !unsafe { pyre_object::is_tuple(closure) } || unsafe { pyre_object::w_tuple_len(closure) } != callee_code.freevars.len() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } for i in 0..callee_code.freevars.len() { let Some(cell) = (unsafe { pyre_object::w_tuple_getitem(closure, i as i64) }) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; concrete_freevar_cells.push(cell); } closure } else { if !callee_code.freevars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } pyre_object::PY_NULL }; @@ -3361,7 +3374,7 @@ fn try_walker_inline_resolved_user_call_inner( .warm_state_mut() .can_inline_callable(callee_green_key) { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if fbw_inline_recursion_count(ctx, callee_code_key) >= FBW_MAX_INLINE_RECURSION { if let Some((driver, _)) = crate::driver::try_driver_pair() { @@ -3370,13 +3383,13 @@ fn try_walker_inline_resolved_user_call_inner( .warm_state_mut() .disable_noninlinable_function(callee_green_key); } - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let Some(body) = crate::state::sub_jitcode_body_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if nparams > body.num_regs_r { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // Inlining a callee whose body carries an `abort_permanent` marker walks // the sub-walk straight into it. That surfaces as @@ -3403,10 +3416,10 @@ fn try_walker_inline_resolved_user_call_inner( // means no installed body or descr pool, which the pool fetch immediately // below declines on regardless. let Some(body_facts) = sub_jitcode_body_facts_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if body_facts.has_abort_permanent { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // The callee body resolves its `d`/`j` descr operands through its OWN // per-fn pool, not the caller's. Without this the sub-walk reads the @@ -3415,7 +3428,7 @@ fn try_walker_inline_resolved_user_call_inner( let Some((callee_descr_refs, callee_perfn_descrs, callee_lookup)) = crate::state::sub_jitcode_descr_pool_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; // EXACT int/float only. These feed `fbw_callee_body_replay_safety`, whose // question is "will the walker specialize this body's BINARY_OP to a native @@ -3518,7 +3531,7 @@ fn try_walker_inline_resolved_user_call_inner( let subwalk_admit = ctx.fbw_mode.carrier_resume && !ctx.fbw_mode.snapshot_sym.is_null(); let safe_root_bridge = root_bridge || subwalk_admit; if !safe_root_bridge { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } bridge_rec_root_selfrec = unsafe { let raw = pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) @@ -3539,7 +3552,7 @@ fn try_walker_inline_resolved_user_call_inner( // `SELFREC_CA_FOLD_ACTIVE` exemption from the hazard arm (:2696), so its // recursive residual is not what named the callee here. if !bridge_rec_root_selfrec && fbw_hazardous_inline_denied(callee_code_key) { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A legacy, unseeded inline sub-walk inside a FOR_ITER body resumes a guard // at the caller's CALL boundary, so deopt re-executes the whole callee. @@ -3638,6 +3651,16 @@ fn try_walker_inline_resolved_user_call_inner( && !pyre_interpreter::code_has_for_iter(callee_code) && !body_facts.has_exception_table && !fbw_foriter_deferred_call_denied(callee_code_key); + if !foriter_deferred_admit && fbw_inline_diag_enabled() { + eprintln!( + "[inline-foriter-deferred] pc={} boundary={entry_is_call_boundary} \ + header={loop_header_admitted} for_iter={} exc_table={} denied={}", + op.pc, + pyre_interpreter::code_has_for_iter(callee_code), + body_facts.has_exception_table, + fbw_foriter_deferred_call_denied(callee_code_key), + ); + } foriter_deferred_admit } CalleeReplaySafety::Dirty => { @@ -3666,7 +3689,7 @@ fn try_walker_inline_resolved_user_call_inner( // source handles it. Stored bound methods instead take the explicit // multi-frame red-frame path above. if !legacy_admit { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } } // A widened method-form body that also raises was declined here until the @@ -3737,7 +3760,7 @@ fn try_walker_inline_resolved_user_call_inner( // and re-runs the instantiation, making the result discard unnecessary to // represent. if constructor_result.is_some() && !strict_inlinable { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A zero-param callee has no positional argument to seed, so the register // convention above holds vacuously and the strict path serves it like any @@ -3748,7 +3771,7 @@ fn try_walker_inline_resolved_user_call_inner( // body still takes the residual rather than the decline-to-interpretation // below. if nparams == 0 && !strict_inlinable { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A self-recursive callee unrolls until its own frame count reaches @@ -3818,7 +3841,7 @@ fn try_walker_inline_resolved_user_call_inner( "InlineCallee::BranchyHandlerDirty" }, ); - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A callee that raises inline needs the cross-frame bridge the carrier // drain builds once a guard inside the compiled chain fails. The drain @@ -3887,7 +3910,7 @@ fn try_walker_inline_resolved_user_call_inner( None }; if foriter_dirty_bound && !try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if !strict_inlinable && !try_multiframe && !force_caller_boundary_resume { // A non-self-recursive loop/branch callee that neither the strict nor @@ -3900,13 +3923,13 @@ fn try_walker_inline_resolved_user_call_inner( // cache; making an uninlineable method body blacklist the whole outer // loop turns a correct specialization into a compile regression. if method_form { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // Full-portal cutover: instead of poisoning the trace, fall through to // the CALL_ASSEMBLER fold (`try_walker_call_assembler_self_recursive`, // reached next in the residual-call dispatch) so a recursive callee at // the inline cap enters via its own (possibly tmp-callback) loop token. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let mut callable_guard_op = callable_guard_op; @@ -4291,7 +4314,7 @@ fn try_walker_inline_resolved_user_call_inner( None => i, }; if reg >= callee_regs_r.len() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } callee_regs_r[reg] = callee_args[i]; callee_concrete_r[reg] = callee_arg_concretes[i]; @@ -4390,7 +4413,7 @@ fn try_walker_inline_resolved_user_call_inner( // `strict_seed` already excludes such a callee, so only the // multiframe path reaches this. if !callee_code.cellvars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // POP_JUMP_IF_NONE / POP_JUMP_IF_NOT_NONE lower to an `is`/`is_not` // identity residual call whose operands must be Ref (the codewriter @@ -4459,7 +4482,7 @@ fn try_walker_inline_resolved_user_call_inner( }); if has_is_none_branch { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::IsNoneBranch"; break 'seed; @@ -4473,7 +4496,7 @@ fn try_walker_inline_resolved_user_call_inner( crate::state::ensure_jitcode_index(callee_code_key as *const ()) else { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoCalleeJitcode"; break 'seed; @@ -4485,7 +4508,7 @@ fn try_walker_inline_resolved_user_call_inner( || ec_reg as usize >= callee_regs_r.len() { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoPortalRedRegs"; break 'seed; @@ -4501,7 +4524,7 @@ fn try_walker_inline_resolved_user_call_inner( let sym_ptr = ctx.fbw_mode.snapshot_sym; if sym_ptr.is_null() { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoSnapshotSym"; break 'seed; @@ -5392,7 +5415,7 @@ fn try_walker_inline_resolved_user_call_inner( // observes and changes nothing (`exc_override_sample_safe`), // and it keeps a legal program from killing the enclosing // loop's trace, which `callee_inline_unsupported` would. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // `descr_call` discards `__init__`'s result after checking it is // None and returns the instance instead (`check_init_returned_none`). From 052b763e0cdb145bc0ac17104b0aad25bf6f32f7 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 16:49:19 +0900 Subject: [PATCH 22/24] _abc: read _get_dump's three `.data` values back from their root slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_dump` published each `data` and then pushed the raw local into `items`, so the slot was never read again. The loop runs three times and the reads in between — `cache_attr`, `getattr_str`, and `negative_cache_version` — go through the descriptor protocol, so they allocate and can run a minor collection. `data` is whatever `cache.data` answers, which can be a list or a dict, the two kinds a collection moves, so `w_tuple_new` could embed a pre-move address in a live tuple. Keep the slot indices, take the version before the reloads, and build `items` from `roots.get`. `app_abc.py _get_dump` is a single expression, so the translator holds all three values live across the same reads. The `abc_init` header comment described the registry as a per-class list and cited `_py_abc`; the body's own comment already states the per-class reasoning against `app_abc.py`. Assisted-by: Claude --- pyre/pyre-interpreter/src/module/_abc/mod.rs | 27 +++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index 345068dec3b..a79978cc25b 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -146,12 +146,8 @@ fn negative_cache_version(cls: PyObjectRef) -> Result { Ok(unsafe { w_int_get_value(version) }.max(0) as u64) } -// `_py_abc.ABCMeta.__new__` (`_py_abc.py:48`) gives every ABC its OWN -// `_abc_registry`. Create it here as a per-class list so the registry is not -// inherited: without an own entry `register`/`subclass_of` would resolve -// `_abc_registry` up the MRO and share one base class's list across every -// descendant ABC (e.g. Complex/Real/Rational/Integral all collapsing to a -// single registry). +/// `app_abc.py _abc_init` — install the three collections and the +/// negative-cache generation, then compute `__abstractmethods__`. fn abc_init(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { // `app_abc.py:74-77` — registry and both caches are per-class for the @@ -591,7 +587,13 @@ fn get_dump(args: &[PyObjectRef]) -> Result { }; let roots = pyre_object::gc_roots::push_roots(); let cls_slot = roots.publish(&[cls]); - let mut items = Vec::with_capacity(4); + // `app_abc.py _get_dump` builds the four-tuple as a single expression, so + // every `.data` it reads stays a live variable that the later reads reload. + // Keep the slots rather than the raw pointers: `data` is whatever + // `cache.data` answers, so it can be a list or a dict — the two kinds a + // minor collection moves — and each further `cache_attr` / `getattr_str` + // runs the descriptor protocol. + let mut data_slots = Vec::with_capacity(3); for name in ["_abc_registry", "_abc_cache", "_abc_negative_cache"] { let cache = cache_attr(roots.get(cls_slot), name)?; // A class that never ran `_abc_init` has nothing to describe; an empty @@ -602,12 +604,13 @@ fn get_dump(args: &[PyObjectRef]) -> Result { let cache_slot = roots.publish(&[cache]); crate::baseobjspace::getattr_str(roots.get(cache_slot), "data")? }; - roots.publish(&[data]); - items.push(data); + data_slots.push(roots.publish(&[data])); } - items.push(w_int_new( - negative_cache_version(roots.get(cls_slot))? as i64 - )); + // The last read that can run Python; take it before the reloads below so + // they answer with final addresses. + let version = w_int_new(negative_cache_version(roots.get(cls_slot))? as i64); + let mut items: Vec = data_slots.iter().map(|&slot| roots.get(slot)).collect(); + items.push(version); Ok(w_tuple_new(items)) } From 07ca44747a3faeffd4376e6cc2588c44d0756d34 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 16:49:49 +0900 Subject: [PATCH 23/24] getattr hook inline: cut the trace back when the callee sub-walk declines `try_walker_inline_getattr_hook` emits before it calls `try_walker_inline_resolved_user_call`: `walker_guard_mapdict_instance_shape` records a `GuardClass`, a `GuardValue`, a type-version quasi-immutable pin and calls `class_now_known` plus `replace_box`, and `walker_guard_function_field` records a `GetfieldGcR` + `GuardValue` and another `replace_box`. That call has decline paths of its own past that point, and the caller in `residual_call.rs` then falls through to the generic attribute residual, so the guards and the heap-cache entries stayed in the trace with nothing reading them. Take the trace position before the first emit and cut back to it on the decline, the way `try_walker_inline_property_get` and `try_walker_inline_property_set` already do. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 8357e02d947..343a739d479 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -6492,6 +6492,10 @@ pub(crate) fn try_walker_inline_getattr_hook( return Ok(None); } + // Everything below emits, and the callee inline has decline paths of its + // own past this point, so keep a rewind point the way the property twins + // do. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); // Both pins the oracle asked for, plus the layout guard its map read needs. walker_guard_mapdict_instance_shape(ctx, op.pc, obj, concrete_obj, w_type, version_tag, map)?; // The pins above make the DESCRIPTOR a constant; they say nothing about the @@ -6531,7 +6535,7 @@ pub(crate) fn try_walker_inline_getattr_hook( callee_args.push(name_const); callee_arg_concretes.push(ConcreteValue::Ref(name_obj)); let getattr_const = ctx.trace_ctx.const_ref(w_func as i64); - try_walker_inline_resolved_user_call( + let inlined = try_walker_inline_resolved_user_call( ctx, op, code, @@ -6559,7 +6563,12 @@ pub(crate) fn try_walker_inline_getattr_hook( true, false, None, - ) + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) } /// Inline a `property` setter store (`obj.value = x`) after the plain-attribute From 7e0240fc3c75c5f2c521148e05385b5a9c0ee8b8 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 19 Aug 2026 17:53:29 +0900 Subject: [PATCH 24/24] Fold three spellings of PYRE_FBW_INLINE_DIAG into the cached gate; fix a count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `type_call_diag_enabled` and two open-coded `std::env::var("PYRE_FBW_INLINE_DIAG")` reads in `inline_call.rs` each re-read the variable per call and spell it `var` where `fbw_inline_diag_enabled` spells it `var_os`, so a non-UTF-8 value made them disagree. All three now go through that gate, which caches in a `OnceLock`. `gate-triage.md` §6c's heading said 67 over a list of 68 distinct `PYRE_*` names; the heading was already one behind before `PYRE_FBW_REPLAY_DIRTY_BODY` was added. That entry also gets its prerequisite written down: `replay_safety_dump_body` returns unless `PYRE_FBW_INLINE_DIAG` is set too, so setting it alone prints nothing. The `register_quasi_immutable_deps` count this commit also carried is dropped: #1336 rewrote that comment, and after the rebase it correctly reads nine hand-minted singletons plus the nine `Function` fields the group arm resolves. Assisted-by: Claude --- pyre/gate-triage.md | 8 +++++++- .../src/jitcode_dispatch/inline_call.rs | 17 ++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index b40f83e5ecc..44573661b8b 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -1014,7 +1014,7 @@ the folds it selects, not before them. `PYRE_FBW_SPEC_CENSUS` in §6c is its read-only half: the per-fold consulted/fired tallies. -### §6c — Default-OFF diagnostics, censuses and probes (67): keep, cost nothing +### §6c — Default-OFF diagnostics, censuses and probes (68): keep, cost nothing Each is inert unless set, so none is a removal target by this file's already-ON criterion. They are listed so they cannot be missed again. @@ -1054,6 +1054,12 @@ value knobs bound the capture window, sampling rate, and report size. This is a diagnostic tool rather than a temporary runtime experiment, so it retires only if the example itself is removed. +`PYRE_FBW_REPLAY_DIRTY_BODY` is a sub-knob of `PYRE_FBW_INLINE_DIAG` rather +than a gate of its own: `replay_safety_dump_body` returns unless both are set, +so setting it alone prints nothing. It lists each callee body as it is scanned, +which is what lets the `pc` on a following `[replay-dirty]` line be matched to +an op. It goes with the inline diagnostic it extends. + `PYRE_VSTACK_NO_EXACT` and `PYRE_VSTACK_KEEP_REORDER` are A/B switches over the walk-level operand-stack mirror, each restoring the behaviour its default replaced: resolving the mirror's Python-PC coordinate from the floor tier rather diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 343a739d479..2c870449348 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -3707,7 +3707,7 @@ fn try_walker_inline_resolved_user_call_inner( // 15.3 once admitted, and `self.i >= self.n` 1207 -> 15.9. Swapping that // `raise` for a `return` already measured 17.7, which is what named the // token rather than the branch or the attribute compare. - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if fbw_inline_diag_enabled() { let mut pc = 0usize; let mut shown = 0; while pc < body.code.len() && shown < 8 { @@ -5321,7 +5321,7 @@ fn try_walker_inline_resolved_user_call_inner( let (outcome, _end_pc) = match callee_outcome { Ok(v) => v, Err(e) => { - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if fbw_inline_diag_enabled() { eprintln!("[inline-abort] callee sub-walk err: {e:?}"); } // gh#467: a supported abort fired inside this top-level inline @@ -5579,14 +5579,9 @@ fn try_walker_inline_resolved_user_call_inner( } } -/// Whether the instantiation emit's decline reasons are being collected. -fn type_call_diag_enabled() -> bool { - std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() -} - /// Report why the instantiation emit declined, under `PYRE_FBW_INLINE_DIAG`. fn type_call_decline(reason: &str) -> Result, DispatchError> { - if type_call_diag_enabled() { + if fbw_inline_diag_enabled() { eprintln!("[type-call-decline] {reason}"); } Ok(None) @@ -5624,7 +5619,7 @@ pub(crate) fn try_walker_inline_type_call( // about, so name the reason only for a call that does resolve to a // class, and only while the reasons are being collected — the extra // resolution below is diagnostic cost, not tracing cost. - if type_call_diag_enabled() + if fbw_inline_diag_enabled() && r_args.len() >= 2 && walker_concrete_ref_object(ctx, r_args[1]).is_none() && walker_concrete_ref_object(ctx, r_args[0]) @@ -5802,7 +5797,7 @@ pub(crate) fn try_walker_inline_type_call( instance, &pyre_object::pyobject::INSTANCE_TYPE as *const _ as i64, ); - if type_call_diag_enabled() { + if fbw_inline_diag_enabled() { eprintln!( "[type-call-inline] pc={} class={} init={}", op.pc, @@ -5861,7 +5856,7 @@ pub(crate) fn try_walker_inline_type_call( // stood. Say so when it does not: without this line the diagnostic // reads as a successful fold on a trace that ends up carrying the whole // instantiation as a residual. - if type_call_diag_enabled() { + if fbw_inline_diag_enabled() { eprintln!( "[type-call-rewind] pc={} class={} why=__init__ sub-walk declined", op.pc,