From 0ba79876384464a8a77f010f8be6360a942a7039 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 26 Aug 2026 08:33:33 +0900 Subject: [PATCH 01/10] jit: install the GC root walkers at init_gc_subsystem instead of first eval `init_gc_root_walkers` ran only from `eval_with_jit_inner`, so the walkers were absent for every collection that happened before the first Python frame. `init_typeobjects` builds each builtin type object and its namespace dict in that window; the type object is a `malloc_typed` block outside the GC heap, so `walk_builtin_type_dicts_gc` is the only path the collector has to those young dicts. Call it at the tail of `init_gc_subsystem`, after `initialize_rbigint_parts_cache`. The first-eval call stays for threads that reach an eval loop without running that bootstrap. Measured with a `gc_stress` build on `print("trivial ok")`: before, SIGBUS in `w_dict_items` under `retag_classmethod_descriptors` (typedef.rs); after, startup completes 784 minor and 193 major collections and reaches `sys` module registration. Assisted-by: Claude --- pyre/pyre-jit/src/eval.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index e875121db07..b6859e496aa 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -4815,8 +4815,9 @@ fn walk_immortal_store_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { } /// Phase B: root walkers that reference interpreter state (immortal dicts, -/// mapdict side table, etc.). Called on first eval entry, after the -/// interpreter is initialized. +/// mapdict side table, etc.). Registration stores fn pointers only, so it +/// runs at the tail of `init_gc_subsystem`, before anything the walkers +/// answer for has been allocated. fn install_gc_root_walkers() { pyre_interpreter::eval::register_interpreter_global_root_walker(); majit_gc::shadow_stack::register_extra_root_walker(walk_parked_exception_roots); @@ -5091,6 +5092,16 @@ pub fn init_gc_subsystem() { // because it allocates and so needs this thread to hold the GIL. majit_rlib::rbigint::initialize_rbigint_parts_cache(); PYRE_OBJECT_HOOKS_INSTALLED.call_once(install_pyre_object_hooks); + // The root walkers belong to the same bootstrap as the collector they feed: + // interpreter startup builds every builtin type object and its namespace + // dict before the first Python frame exists, and `walk_builtin_type_dicts_gc` + // is the only path to those young dicts. Installed at first eval instead, + // a collection in that window reclaims a namespace dict whose type object + // still points at it — measured under `MAJIT_GC_STRESS=1`, which faults in + // `w_dict_items` while `retag_classmethod_descriptors` sweeps the registry. + // Placed after `initialize_rbigint_parts_cache` so the first walk finds + // that cache built rather than manufacturing it from inside the walker. + init_gc_root_walkers(); } /// Guards the one-time install of the process-global pyre-object GC hooks. @@ -5101,8 +5112,9 @@ thread_local! { } /// Phase B of GC init: register root walkers that touch interpreter -/// state (immortal dicts, mapdict side table, etc.). Must run after -/// the interpreter is initialized — called on first eval entry. +/// state (immortal dicts, mapdict side table, etc.). Called from +/// `init_gc_subsystem` once the collector is installed, and again on the +/// first eval entry for the paths that reach an eval loop without it. /// Idempotent. pub fn init_gc_root_walkers() { if GC_ROOT_WALKERS_INSTALLED.with(|c| c.get()) { @@ -8482,9 +8494,10 @@ fn eval_with_jit_inner( // through the frame — compiled, JIT eval loop, or declined to the plain // evaluator — passes exactly once. let _recursion_depth = pyre_interpreter::call::enter_recursive_frame(frame); - // Phase B of GC init: register root walkers that reference - // interpreter state. Safe here — the interpreter is initialized. - // Phase A (GC build + backend install) ran at boot in init_jit_hooks. + // Phase B of GC init: register root walkers that reference interpreter + // state. `init_gc_subsystem` already installs them on the path that + // builds the collector; this covers a thread that reaches an eval loop + // without having run that bootstrap itself. init_gc_root_walkers(); // PYRE_JIT=0 disables JIT entirely, falling back to plain interpreter. static PYRE_JIT_DISABLED: std::sync::OnceLock = std::sync::OnceLock::new(); From 3d214b25288639df6a56752f47a1aa6157658d24 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 26 Aug 2026 09:13:05 +0900 Subject: [PATCH 02/10] interp: root the file wrapper across its attribute stores in open_raw_file `open_raw_file` built the wrapper with `w_instance_new` and then carried it in a Rust local through nine or ten `setattr_str` calls whose value argument allocates. An allocation is a collection point and nothing else names the instance yet, so the collector was free to reclaim it between the construction and the first store; `fileio_store_stat_atopen` had the same shape across its three `w_int_new` calls. Pin the instance on the shadow stack and read the address back before each store, through `new_rooted_file_wrapper` / `file_wrapper_store`, at all six construction sites. Measured with a `gc_stress` build on `print("trivial ok")`: before, the write barrier in `_set_mapdict_increase_storage1` aborted on a freed header after 784 minor / 193 major collections; after, startup runs 3719 minor / 780 major collections and reaches class creation. Assisted-by: Claude --- pyre/pyre-interpreter/src/builtins.rs | 195 ++++++++++++++++---------- 1 file changed, 123 insertions(+), 72 deletions(-) diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 1cb270a73c3..37bfdb70d94 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -16517,12 +16517,18 @@ type FileioStatAtOpen = (); #[cfg(unix)] fn fileio_store_stat_atopen(self_obj: PyObjectRef, stat: &FileioStatAtOpen) { + // `w_int_new` allocates once per slot, so the receiver is read back from + // its root between the stores rather than carried in the parameter. + let _roots = pyre_object::gc_roots::push_roots(); + let self_slot = pyre_object::gc_roots::pin_roots(&[self_obj]); for (name, value) in [ ("__file_stat_mode__", stat.mode as i64), ("__file_stat_size__", stat.size as i64), ("__file_stat_blksize__", stat.blksize as i64), ] { - crate::baseobjspace::setdictvalue_native(self_obj, name, w_int_new(value)); + let value = w_int_new(value); + let self_obj = pyre_object::gc_roots::shadow_stack_get(self_slot); + crate::baseobjspace::setdictvalue_native(self_obj, name, value); } } @@ -18498,6 +18504,33 @@ pub fn builtin_open(args: &[PyObjectRef]) -> Result result } +/// A new file-wrapper instance, pinned on the shadow stack, returned as its +/// slot index. +/// +/// The instance cannot ride a Rust local across the attribute stores that +/// follow it: every store builds its value by allocating, an allocation is a +/// collection point, and nothing but this frame names the instance yet, so +/// the collector is free to move or reclaim it between two stores. The +/// caller keeps a `push_roots` scope open and reads the current address back +/// through [`file_wrapper_store`] or `shadow_stack_get`. +fn new_rooted_file_wrapper() -> usize { + pyre_object::gc_roots::pin_roots(&[pyre_object::w_instance_new(file_wrapper_type())]) +} + +/// Store one attribute on the file wrapper rooted at `slot`. +/// +/// `value` is already built when this runs — the caller's argument expression +/// is what allocates — so the wrapper is read back here, after that +/// allocation, rather than passed in. `value` itself is pinned for the store, +/// which allocates the attribute name. +fn file_wrapper_store(slot: usize, name: &str, value: PyObjectRef) { + let _roots = pyre_object::gc_roots::push_roots(); + let value_slot = pyre_object::gc_roots::pin_roots(&[value]); + let wrapper = pyre_object::gc_roots::shadow_stack_get(slot); + let value = pyre_object::gc_roots::shadow_stack_get(value_slot); + let _ = crate::baseobjspace::setattr_str(wrapper, name, value); +} + /// Low-level storage opener used by `W_FileIO.descr_init`. /// /// This is pyre's `_open_fd`/raw-storage boundary. It deliberately does not @@ -18585,18 +18618,22 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { None => true, }; let binary = mode.contains('b'); - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_fd__", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closefd", w_bool_from(closefd)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - fileio_store_stat_atopen(wrapper, &stat_atopen); - return Ok(wrapper); + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store(wrapper_slot, "__file_fd__", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closefd", w_bool_from(closefd)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + fileio_store_stat_atopen( + pyre_object::gc_roots::shadow_stack_get(wrapper_slot), + &stat_atopen, + ); + return Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)); } if closefd_obj.map(crate::baseobjspace::is_true).transpose()? == Some(false) { @@ -18653,17 +18690,21 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { // An opener is free to ignore the flags it was handed, so the binary // mode `open_flags_for_mode` asked for is only guaranteed here. fileio_set_binary_mode(fd); - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_fd__", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", path_obj); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - fileio_store_stat_atopen(wrapper, &stat_atopen); - return Ok(wrapper); + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store(wrapper_slot, "__file_fd__", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", path_obj); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + fileio_store_stat_atopen( + pyre_object::gc_roots::shadow_stack_get(wrapper_slot), + &stat_atopen, + ); + return Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)); } // The sandbox routes the whole open→read/write→close chain through the @@ -18685,18 +18726,22 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { return Err(error); } }; - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_fd__", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", path_obj); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closefd", w_bool_from(true)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - fileio_store_stat_atopen(wrapper, &stat_atopen); - Ok(wrapper) + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store(wrapper_slot, "__file_fd__", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", path_obj); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closefd", w_bool_from(true)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + fileio_store_stat_atopen( + pyre_object::gc_roots::shadow_stack_get(wrapper_slot), + &stat_atopen, + ); + Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)) } #[cfg(all(not(feature = "sandbox"), unix))] { @@ -18746,18 +18791,22 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { return Err(error); } }; - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_fd__", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", path_obj); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closefd", w_bool_from(true)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - fileio_store_stat_atopen(wrapper, &stat_atopen); - Ok(wrapper) + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store(wrapper_slot, "__file_fd__", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", path_obj); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closefd", w_bool_from(true)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + fileio_store_stat_atopen( + pyre_object::gc_roots::shadow_stack_get(wrapper_slot), + &stat_atopen, + ); + Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)) } #[cfg(all(not(feature = "sandbox"), windows, feature = "host_env"))] { @@ -18781,19 +18830,20 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { })? .into_raw(); - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_fd__", w_int_new(fd as i64)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store(wrapper_slot, "__file_fd__", w_int_new(fd as i64)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); // Carry binary-ness so descriptor reads/readlines wrap their chunks as // `bytes` for `rb`; tokenize.detect_encoding relies on that result. - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", path_obj); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closefd", w_bool_from(true)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - Ok(wrapper) + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", path_obj); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closefd", w_bool_from(true)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)) } // Preserve the existing non-Windows fallback (notably wasm) unchanged; // the Windows host build above owns a real CRT descriptor instead. @@ -18874,26 +18924,27 @@ fn open_raw_file(args: &[PyObjectRef]) -> Result { } } - let wrapper = pyre_object::w_instance_new(file_wrapper_type()); - let _ = crate::baseobjspace::setattr_str( - wrapper, + let _wrapper_roots = pyre_object::gc_roots::push_roots(); + let wrapper_slot = new_rooted_file_wrapper(); + file_wrapper_store( + wrapper_slot, "__file_data__", pyre_object::bytesobject::w_bytes_from_bytes(&data), ); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_pos__", w_int_new(0)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_name__", w_str_new(&path)); - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_mode__", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "__file_pos__", w_int_new(0)); + file_wrapper_store(wrapper_slot, "__file_name__", w_str_new(&path)); + file_wrapper_store(wrapper_slot, "__file_mode__", w_str_new(&mode)); // Carry binary-ness so read/readline wrap their chunks as `bytes` in // binary mode (`'rb'`), matching the fd-backed branch above. Without // this a path-backed `open(p, 'rb').readline()` would hand back `str`, // breaking `tokenize.detect_encoding` (`first.startswith(BOM_UTF8)`). - let _ = crate::baseobjspace::setattr_str(wrapper, "__file_binary__", w_bool_from(binary)); - let _ = crate::baseobjspace::setattr_str(wrapper, "encoding", w_str_new(&encoding)); - let _ = crate::baseobjspace::setattr_str(wrapper, "errors", w_str_new(&errors)); - let _ = crate::baseobjspace::setattr_str(wrapper, "name", w_str_new(&path)); - let _ = crate::baseobjspace::setattr_str(wrapper, "mode", w_str_new(&mode)); - let _ = crate::baseobjspace::setattr_str(wrapper, "closed", w_bool_from(false)); - Ok(wrapper) + file_wrapper_store(wrapper_slot, "__file_binary__", w_bool_from(binary)); + file_wrapper_store(wrapper_slot, "encoding", w_str_new(&encoding)); + file_wrapper_store(wrapper_slot, "errors", w_str_new(&errors)); + file_wrapper_store(wrapper_slot, "name", w_str_new(&path)); + file_wrapper_store(wrapper_slot, "mode", w_str_new(&mode)); + file_wrapper_store(wrapper_slot, "closed", w_bool_from(false)); + Ok(pyre_object::gc_roots::shadow_stack_get(wrapper_slot)) } } From 320e1c9cbaf7bd6d420b5eebed6c7a0b4487a51b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 26 Aug 2026 09:44:27 +0900 Subject: [PATCH 03/10] interp: root the new class across create_all_slots in type.__new__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `type_descr_new_with_metaclass` carried the `w_type_new` result in a Rust local through `type_new_take_qualname` and `create_all_slots`, both of which allocate, before `tag_subclass_instance` took the write barrier on it. Nothing else refers to a class that young — the classcell is optional and `weak_subclasses` is weak — so a major cycle in that window sweeps it, which is the hazard the `_entry_roots` scope further down already reasons about. Pin it for liveness right after construction; a type does not move, so the local remains a good address. Measured with a `gc_stress` build: before, `store_subclass_tag`'s barrier aborted on an invalid type id after 3719 minor / 780 major collections; after, `MAJIT_GC_STRESS=1 pyre-dynasm print("trivial ok")` completes with rc=0 (0.04s unstressed against 3.75s stressed, so the flag is live). Assisted-by: Claude --- pyre/pyre-interpreter/src/builtins.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 37bfdb70d94..07ad2e1680f 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -6364,6 +6364,14 @@ fn type_descr_new_with_metaclass( let dict_obj = unsafe { pyre_object::w_dict_copy(class_ns) }; let dict_obj = pyre_object::gc_roots::pin_root(dict_obj); let w_type = pyre_object::w_type_new(name, w_effective_bases, dict_obj as *mut u8); + // Nothing refers to a class this young — the classcell is optional and + // `weak_subclasses` is weak — while the passes below allocate, so a + // major cycle sweeps it out from under `create_all_slots` and the + // barrier in `tag_subclass_instance` then writes through a freed + // header. This is the `_entry_roots` scope's reasoning extended back + // to the construction; a type does not move, so the local stays a good + // address and the root is for liveness only. + let _ = pyre_object::gc_roots::pin_root(w_type); // The type allocation may have moved the namespace, so the qualname // pass receives the forwarded address rather than the word above. type_new_take_qualname(w_type, pyre_object::gc_roots::shadow_stack_get(dict_root))?; From 81e1a7265707d0b87770f3ac65df640fa355f918 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 26 Aug 2026 19:07:42 +0900 Subject: [PATCH 04/10] jit: cache the abort-ceiling refusal so a latched loop stops re-deriving it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WarmEnterState::maybe_compile_decision` refuses a cell whose `abort_count` reached `MAX_TRACE_ABORT_COUNT`, and a loop that can never trace re-derives that refusal on every back edge. Measured with `sys.setprofile` over a warm loop, `abort_ceiling_refused` tracked the iteration count one-for-one: 194776 at 200k iterations, 794776 at 800k, and 0 for a loop with no call in its body. `maybe_compile_and_run` now caches the refusal per green key against a new `WarmEnterState::cell_generation`, bumped by `install_new_cell`, `attach_procedure_to_interp`, `attach_procedure_to_interp_for_key` and `attach_tmp_callback_to_interp` — the mutations that can make a refused key runnable again. `is_ceiling_latched` restates the decision's condition for a caller that wants only that answer. Graded as the same tree built twice: 2.3% faster on the profiled arm, faster in 5 of 5 rounds, `abort_ceiling_refused` 194776 -> 1. `PYRE_JIT=0` is not a control for this — it is read in `eval_with_jit_inner` and routes the frame to `execute_frame_plain`, a different eval loop. Assisted-by: Claude --- majit/majit-metainterp/src/warmstate.rs | 127 +++++++++++++++++++++++- pyre/pyre-jit/src/eval.rs | 64 +++++++++++- 2 files changed, 187 insertions(+), 4 deletions(-) diff --git a/majit/majit-metainterp/src/warmstate.rs b/majit/majit-metainterp/src/warmstate.rs index 3a46c0e67b0..f43ed9b9852 100644 --- a/majit/majit-metainterp/src/warmstate.rs +++ b/majit/majit-metainterp/src/warmstate.rs @@ -684,6 +684,17 @@ pub struct WarmEnterState { /// warmspot.py:110: memory_manager — generation-based loop aging. /// pyjitpl.py: try_to_free_some_loops calls next_generation(). pub memory_manager: crate::memmgr::MemoryManager, + /// Bumped by every mutation that can change what + /// [`Self::maybe_compile_decision`] answers for a key it already refused + /// at the abort ceiling: installing a cell, which resets `abort_count` to + /// zero, and attaching a procedure token, which can make the key runnable + /// through a sibling cell in the same chain. + /// + /// It exists so a caller can cache that refusal rather than re-derive it + /// per back edge. The refusal is pure — it bumps a diagnostic slot and + /// returns `NotHot` above `decay_all_counters`, deliberately — so a cache + /// keyed on this counter changes nothing but the work spent reaching it. + cell_generation: u64, } /// Result of checking whether a green key is hot. @@ -793,6 +804,7 @@ impl WarmEnterState { m.max_unroll_loops = DEFAULT_MAX_UNROLL_LOOPS; m }, + cell_generation: 0, } } @@ -899,6 +911,34 @@ impl WarmEnterState { self.counter.tick(bucket, self.increment_threshold) } + /// The counter [`Self::cell_generation`] documents. + pub fn cell_generation(&self) -> u64 { + self.cell_generation + } + + fn bump_cell_generation(&mut self) { + self.cell_generation = self.cell_generation.wrapping_add(1); + } + + /// Whether [`Self::maybe_compile_decision`] would refuse `cell_key` at the + /// abort ceiling. + /// + /// The condition is restated here rather than shared with that decision + /// because the decision reads the cell once and answers four other + /// questions from the same borrow, while a caller asking only this one + /// wants it alone. The two must agree, which + /// `is_ceiling_latched_agrees_with_the_decision_it_mirrors` asserts. + pub fn is_ceiling_latched(&self, cell_key: u64) -> bool { + let Some(cell) = self.cell_by_key(cell_key) else { + return false; + }; + if cell.is_compiled() || cell.is_tracing() { + return false; + } + let dead_token = cell.has_seen_a_procedure_token() && cell.get_procedure_token().is_none(); + !dead_token && cell.abort_ceiling_latched() + } + /// The `dead_token` gate below is narrower than /// `warmstate.py maybe_compile_and_run`'s tokenless arm, and /// deliberately so. Upstream drops EVERY tokenless cell there — "it was an @@ -1344,7 +1384,9 @@ impl WarmEnterState { let token = token.into(); let cell = self.ensure_cell_by_key(cell_key); cell.flags &= !jc_flags::TRACING; - cell.set_procedure_token(token, false) + let previous = cell.set_procedure_token(token, false); + self.bump_cell_generation(); + previous } /// Typed form of [`Self::attach_procedure_to_interp`]. @@ -1367,7 +1409,9 @@ impl WarmEnterState { .lookup_chain_with_key_mut(key) .expect("ensure_cell_for_key just installed a cell matching this key"); cell.flags &= !jc_flags::TRACING; - cell.set_procedure_token(token, false) + let previous = cell.set_procedure_token(token, false); + self.bump_cell_generation(); + previous } /// warmstate.py `cell.set_procedure_token(procedure_token, tmp=True)`. @@ -1388,6 +1432,7 @@ impl WarmEnterState { let token = token.into(); let cell = self.ensure_cell_by_key(cell_key); let _old = cell.set_procedure_token(token, true); + self.bump_cell_generation(); } /// warmstate.py `finally: cell.flags &= ~JC_TRACING` parity — @@ -2897,6 +2942,7 @@ impl WarmEnterState { /// upstream needs no equivalent because it hands the cell object itself /// on (warmstate.py:483/:511) and never re-derives it from a number. pub fn install_new_cell(&mut self, hash: u64, newcell: Option) { + self.bump_cell_generation(); let mut keep = newcell.map(Box::new); if let Some(cell) = &mut keep && cell.cell_key.is_none() @@ -6736,4 +6782,81 @@ mod tests { cleanup_chain and left it in the bucket", ); } + + #[test] + fn is_ceiling_latched_agrees_with_the_decision_it_mirrors() { + let mut ws = WarmEnterState::new(2); + let cell_key = 42u64; + ws.ensure_cell_by_key(cell_key); + assert!( + !ws.is_ceiling_latched(cell_key), + "a fresh cell is not latched", + ); + + for _ in 0..MAX_TRACE_ABORT_COUNT { + ws.abort_tracing(cell_key, false); + } + assert!(ws.is_ceiling_latched(cell_key)); + + // The refusal is what this predicate stands in for, and it leaves the + // generation alone — otherwise a cache keyed on it would be + // invalidated by the very answer it is caching. + let generation = ws.cell_generation(); + assert!(matches!( + ws.maybe_compile_decision(cell_key), + HotResult::NotHot + )); + assert_eq!(ws.cell_generation(), generation); + assert!( + ws.is_ceiling_latched(cell_key), + "the refusal did not consume the latch", + ); + + // `install_new_cell` keeps a cell that is not removable, so this one + // stays latched; what matters for a cache is that the generation moves + // anyway, because the same call drops removable cells and lets a fresh + // one trace in their place. + ws.install_new_cell(cell_key, None); + assert_ne!(ws.cell_generation(), generation); + } + + #[test] + fn a_dead_token_is_not_a_ceiling_latch_because_the_decision_cleans_it_up() { + let mut ws = WarmEnterState::new(2); + let key = GreenKey::new(vec![7, 11]); + let token_num = ws.alloc_token_number(); + ws.attach_procedure_to_interp_for_key(&key, JitCellToken::new(token_num)); + for _ in 0..MAX_TRACE_ABORT_COUNT { + ws.abort_tracing_for_key(&key, false); + } + let cell = ws + .get_cell_for_key(&key) + .expect("fixture: the cell is still there"); + assert!( + cell.abort_count >= MAX_TRACE_ABORT_COUNT, + "fixture: the ceiling must be latched", + ); + let cell_key = cell.cell_key.expect("fixture: the cell carries its key"); + assert!( + !ws.is_ceiling_latched(cell_key), + "a dead token takes the cleanup path, so the decision does NOT \ + refuse at the ceiling and a cache must not answer for it", + ); + } + + #[test] + fn attaching_a_procedure_token_moves_the_generation_a_cache_keys_on() { + let mut ws = WarmEnterState::new(2); + let cell_key = 7u64; + ws.ensure_cell_by_key(cell_key); + let generation = ws.cell_generation(); + let token_num = ws.alloc_token_number(); + ws.attach_procedure_to_interp(cell_key, JitCellToken::new(token_num)); + assert_ne!( + ws.cell_generation(), + generation, + "a token can make a latched key runnable through its chain, so a \ + cache must be told", + ); + } } diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index b6859e496aa..de2860cf6cb 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -9549,6 +9549,49 @@ fn deliver_inflight_foriter_item(frame: &mut PyFrame) -> bool { true } +thread_local! { + /// Green keys whose cell `WarmEnterState::maybe_compile_decision` refuses + /// at the abort ceiling, against the `cell_generation` the refusal was + /// observed at. + /// + /// A loop that a profiler forces to decline — `is_being_profiled` is a + /// portal green, and a loop whose body calls anything declines with + /// `DispatchError::ProfiledResidualCall` — can never trace, so its cell + /// latches and every later back edge re-derives the same refusal. Measured + /// with `sys.setprofile` over a warm loop: `abort_ceiling_refused` tracks + /// the iteration count one-for-one (194776 at 200k iterations, 794776 at + /// 800k), while a loop with no call in its body reads exactly 0. The + /// re-derivation costs a green-key mint, three per-code gate lookups and a + /// bucket-chain walk per iteration. Graded as the same tree built twice — + /// the only valid control, since `PYRE_JIT=0` is read by + /// `eval_with_jit_inner` and routes the frame to `execute_frame_plain`, a + /// different eval loop, rather than isolating this door — the profiled arm + /// runs 2.3% faster with the cache, faster in 5 of 5 rounds, and + /// `abort_ceiling_refused` falls from 194776 to 1. + /// + /// Caching it is behaviour-preserving: the refusal bumps a diagnostic slot + /// and returns `NotHot` above `decay_all_counters`, which + /// `maybe_compile_decision` documents as deliberate, so a latched cell + /// already contributes no decay. The generation is what keeps the cache + /// honest — `WarmEnterState` moves it whenever a cell is installed or a + /// procedure token attached, the two mutations that can make a refused key + /// runnable again. + static CEILING_LATCHED: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashMap::new()); +} + +/// Whether `green_key` was already refused at the abort ceiling, and nothing +/// has happened since that could change the answer. +fn ceiling_latch_is_current(green_key: u64, generation: u64) -> bool { + CEILING_LATCHED.with(|latched| latched.borrow().get(&green_key) == Some(&generation)) +} + +fn record_ceiling_latch(green_key: u64, generation: u64) { + CEILING_LATCHED.with(|latched| { + latched.borrow_mut().insert(green_key, generation); + }); +} + /// RPython warmstate.py maybe_compile_and_run. /// /// Entry point to the JIT. Called at can_enter_jit (back-edge). @@ -9575,6 +9618,14 @@ fn maybe_compile_and_run( if *NO_JIT.get_or_init(|| std::env::var_os("PYRE_NO_JIT").is_some()) { return None; } + // The gates below and the decision at the end answer `None` for a green + // key whose cell has latched at the abort ceiling, and go on answering it + // for every back edge of a loop that can no longer trace. Take the cached + // answer instead; `CEILING_LATCHED` documents why that is the same answer. + let cell_generation = driver.meta_interp_mut().warm_state_mut().cell_generation(); + if ceiling_latch_is_current(green_key, cell_generation) { + return None; + } // Not every back-edge reaching this helper passed `eval_with_jit_inner`'s // classification: `portal_runner_dispatch` enters `eval_loop_jit` for a // frame forced through the portal, and that route exists precisely for a @@ -9697,8 +9748,17 @@ fn maybe_compile_and_run( majit_metainterp::warmstate::HotResult::RunCompiled => { execute_assembler(frame, green_key, loop_header_pc, driver, info, env) } - majit_metainterp::warmstate::HotResult::NotHot - | majit_metainterp::warmstate::HotResult::AlreadyTracing => None, + majit_metainterp::warmstate::HotResult::NotHot => { + if driver + .meta_interp_mut() + .warm_state_mut() + .is_ceiling_latched(green_key) + { + record_ceiling_latch(green_key, cell_generation); + } + None + } + majit_metainterp::warmstate::HotResult::AlreadyTracing => None, } } From 375a8b0425f260c7c845d53ad9674cc849ae09bc Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 26 Aug 2026 19:07:56 +0900 Subject: [PATCH 05/10] jit-trace: stage ABORT_TOO_LONG so the abort handler does not redo the walker's bookkeeping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `note_root_trace_too_long` performs the whole of `MetaInterp::blackhole_if_trace_too_long` — `find_biggest_function`, `disable_noninlinable_function`, `portal_trace_positions = None`, `trace_next_iteration`, and the `prepare_trace_segmenting` / bridge arms — and then the walker returns `DispatchError::TraceTooLong`, which carries no `Counters.ABORT_*`. The JitDriver abort handler's reason ladder therefore fell through to `blackhole_if_trace_too_long` and ran it a second time. The log is retired by then, so `find_biggest_function` answers `None` however the trace overflowed and the root takes `prepare_trace_segmenting`, which stamps it with `JC_FORCE_FINISH` + `JC_DONT_TRACE_HERE` — neither of which is ever cleared — even when an inlined callee was named and disabled on its own. `note_root_trace_too_long` now stages `ABORT_TOO_LONG`, which the ladder consults ahead of that fallback. The `JitCodeMachine` path (`pyjitpl/dispatch.rs`) stages nothing and keeps the fallback, which is its own bookkeeping path. On `trace_too_long_inline_multiframe`: 31 too-long aborts, 14 of them naming a huge inlined callee, and `blackhole_if_trace_too_long: aborting` goes from 31 log lines to 0. Adoptions total 31 before and after (21+10, 17+14), equal to `loops_aborted`, so no abort fell back to legacy replay; 4 moved from a single-frame to a multi-frame image. `loops_compiled` is unchanged at 2. The re-recorded baselines also pick up three badness fields the current base prints, all zero. `a_second_too_long_run_segments_a_root_the_first_one_spared` pins what the second entry cost. Assisted-by: Claude --- majit/majit-metainterp/src/pyjitpl.rs | 65 +++++++++++++++++++ ..._long_inline_multiframe.cranelift.jitstats | 7 +- ...too_long_inline_multiframe.dynasm.jitstats | 7 +- ...e_too_long_inline_multiframe.wasm.jitstats | 7 +- .../src/jitcode_dispatch/mod.rs | 20 +++--- pyre/pyre-jit-trace/src/state.rs | 14 ++++ 6 files changed, 105 insertions(+), 15 deletions(-) diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index e101b6fe913..2062992a44b 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -22395,6 +22395,71 @@ mod metainterp_static_data_tests { assert_eq!(meta.find_biggest_function(), None); } + /// pyjitpl.py:2817-2831 runs the too-long bookkeeping once per abort: the + /// reason travels on the `SwitchToBlackhole` instance and the `_interpret` + /// catch never re-enters the check that raised it. This pins what a second + /// entry costs, because pyre reaches the same handler through a + /// `DispatchError` that carries no reason and so had a path back into it. + /// + /// The first run names the oversized callee and disables just that callee, + /// deliberately leaving the root un-marked so it can retrace without it. + /// It also retires the log it read, so a second run can name nothing and + /// takes `prepare_trace_segmenting` instead — which stamps the root with + /// `JC_FORCE_FINISH` + `JC_DONT_TRACE_HERE`, neither of which is ever + /// cleared. The callee's size is what overflowed the trace; the root pays + /// for it permanently. + #[test] + fn a_second_too_long_run_segments_a_root_the_first_one_spared() { + // `start_tracing` opens the loop header this walk is rooted at, and + // its green key is the one the segmenting arm would mark. + const ROOT: u64 = 0; + const CALLEE: u64 = 0xa11; + + let (mut meta, jc) = meta_with_recursive_portal(); + start_tracing(&mut meta); + // One inlined callee, sized by the ops recorded between its entries. + meta.perform_call(jc, &[], Some(CALLEE)).unwrap_err(); + record_ops(&mut meta, 5); + meta.popframe(true); + meta.tracing + .as_mut() + .expect("tracing is Some") + .set_trace_limit(0); + + assert_eq!( + meta.blackhole_if_trace_too_long(), + Some(AbortReason::TooLong) + ); + assert!( + !meta.warm_state_mut().can_inline_callable(CALLEE), + "the named callee is the one that gets disabled" + ); + assert!( + !meta.warm_state_mut().should_force_finish_tracing(ROOT), + "the root is only asked to retrace, so it must not be force-finished" + ); + assert!( + meta.warm_state_mut().can_inline_callable(ROOT), + "the root is only asked to retrace, so it must stay inlinable" + ); + + // Exactly what a second entry sees: the same over-budget trace, and a + // log this abort already retired. + assert_eq!(meta.find_biggest_function(), None); + assert_eq!( + meta.blackhole_if_trace_too_long(), + Some(AbortReason::TooLong) + ); + assert!( + meta.warm_state_mut().should_force_finish_tracing(ROOT), + "a second run has no callee to name and segments the root instead" + ); + assert!( + !meta.warm_state_mut().can_inline_callable(ROOT), + "and stamps it dont-trace-here, which nothing clears" + ); + } + #[test] fn portal_trace_positions_are_rearmed_for_each_trace() { // pyjitpl.py. Upstream builds a MetaInterp per tracing attempt; diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats index 88de6dd70fb..5895a645a10 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats @@ -2,8 +2,11 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=10 -fbw_blackhole_adopted_single_frame=21 +fbw_blackhole_adopted_multi_frame=14 +fbw_blackhole_adopted_single_frame=17 +fbw_escape_plain_fallback=0 +fbw_escape_plain_fallback_unclean=0 +fbw_midbody_latch_new_unjournaled=0 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats index 88de6dd70fb..5895a645a10 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats @@ -2,8 +2,11 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=10 -fbw_blackhole_adopted_single_frame=21 +fbw_blackhole_adopted_multi_frame=14 +fbw_blackhole_adopted_single_frame=17 +fbw_escape_plain_fallback=0 +fbw_escape_plain_fallback_unclean=0 +fbw_midbody_latch_new_unjournaled=0 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats index 6f5caa75783..8bd30019c65 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats @@ -2,8 +2,11 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=10 -fbw_blackhole_adopted_single_frame=20 +fbw_blackhole_adopted_multi_frame=14 +fbw_blackhole_adopted_single_frame=16 +fbw_escape_plain_fallback=0 +fbw_escape_plain_fallback_unclean=0 +fbw_midbody_latch_new_unjournaled=0 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index a106320cc1f..af7f85479b9 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -3405,20 +3405,22 @@ pub fn walk( pc = next_pc; // pyjitpl.py `_interpret`: `blackhole_if_trace_too_long()` runs // after every `run_one_step()`. This loop is that loop's counterpart — - // `step` is `run_one_step` — and the check came with the per-opcode - // tracing loop it replaced (`pyre-jit/src/eval.rs` still runs it on - // that path), so nothing bounded a walk that kept recording without - // reaching a close. `TraceCtx::is_too_long` is the `history.length() > + // `step` is `run_one_step` — and it is the only place the check runs: + // the concrete bytecode dispatch loop it replaced deliberately dropped + // it (`pyre-jit/src/eval.rs`, whose comment records why), so nothing + // else bounds a walk that keeps recording without reaching a close. + // `TraceCtx::is_too_long` is the `history.length() > // warmrunnerstate.trace_limit` test and `num_ops` is a `Vec::len`, so // this is one comparison per step. // // The walker layer holds `&mut TraceCtx` and cannot reach // `MetaInterp::blackhole_if_trace_too_long` for the full bookkeeping; - // `note_root_trace_too_long` carries the warm-state half, split into - // the loop and bridge arms `prepare_trace_segmenting` (pyjitpl.py) - // keeps apart. The `find_biggest_function` → - // `disable_noninlinable_function` half (pyjitpl.py) still only - // runs on the per-opcode path. + // `note_root_trace_too_long` performs it instead — the + // `find_biggest_function` → `disable_noninlinable_function` half as well + // as the loop and bridge arms `prepare_trace_segmenting` (pyjitpl.py) + // keeps apart — and stages `ABORT_TOO_LONG` so the abort handler takes + // the staged reason rather than running that bookkeeping a second time + // against the log this one has already retired. // // `blackhole_if_trace_too_long` raises AFTER `run_one_step`, so the // forward image must carry `pc`, the already-advanced `next_pc`, rather diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 686d0c7d940..f77bdded252 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -3900,6 +3900,20 @@ pub(crate) fn note_root_trace_too_long( // ever do here. let huge_fn = crate::driver::try_driver_pair().and_then(|(driver, _)| { let meta = driver.meta_interp_mut(); + // pyjitpl.py:2831 `raise SwitchToBlackhole(ABORT_TOO_LONG)`: the raise + // carries the reason from here to the `_interpret` catch, and the catch + // never re-runs the check that produced it. The walker's counterpart of + // that raise is the `DispatchError::TraceTooLong` the caller returns, + // which has no room for a `Counters.ABORT_*`, so the reason travels in + // the staging slot the abort handler consults FIRST. Staging it is + // therefore not only accounting: it is what stops the handler falling + // through to `MetaInterp::blackhole_if_trace_too_long`, whose bookkeeping + // this function has just performed. A second run reads the log this one + // retires below, so `find_biggest_function` answers `None` however the + // trace overflowed and the root takes `prepare_trace_segmenting`'s + // permanent `JC_FORCE_FINISH` + `JC_DONT_TRACE_HERE` even when an inlined + // callee was named here and disabled on its own. + meta.stage_abort_reason(majit_metainterp::counters::ABORT_TOO_LONG); // pyjitpl.py `jd_sd, greenkey_of_huge_function = self.find_biggest_function()`. let huge_fn = meta.find_biggest_function(); // pyjitpl.py `self.portal_trace_positions = None` — the log's `_pos` From 5ef5e12fb9af7b72aabd50d774c50ef2dd8525d8 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 27 Aug 2026 04:33:48 +0900 Subject: [PATCH 06/10] jit: give the recursive portal entries execute_frame's hook bracket `eval_with_jit_inner` was the only portal entry that ran `ec.call_trace` / `ec.return_trace` / `ec.leaveframe_trace` around the dispatch. `portal_runner_result`, which `ll_portal_runner_shim`, `bh_portal_runner_c`, `pyre_portal_runner` and `bh_call_self_recursive_portal` reach, ran only `enter_recursive_frame` + `install_current_frame` + the dispatch. Split the bracket out of `eval_with_jit_inner` into `portal_activation_bracketed`, and the shared prologue into `enter_portal`, so the entries that begin an activation (`portal_activation_result`) and the entries that resume one (`portal_runner_result`) select the body they need. The `ContinueRunningNormally` and CALL_ASSEMBLER arms keep the unbracketed entry; `portal_runner`, `bh_portal_runner_c` and `bh_call_self_recursive_portal`, each of which is handed a frame constructed on the calling line, take the bracketed one. Measured with `sys.setprofile` over a self-recursive callee driven from a compiled loop, at tail 30000 and DEPTH 3: `call`/`rec` 30000 reported of 120000 owed before, 120000 after, matching cpython at all four tails. Assisted-by: Claude --- .../src/memory/gctransform/framework.rs | 3 + ...hook_sees_a_recursive_portal_activation.py | 117 +++++++++++++++ pyre/pyre-jit/src/call_jit.rs | 10 +- pyre/pyre-jit/src/eval.rs | 136 +++++++++++------- 4 files changed, 211 insertions(+), 55 deletions(-) create mode 100644 pyre/bench/synth/profile_hook_sees_a_recursive_portal_activation.py diff --git a/majit/majit-translate/src/memory/gctransform/framework.rs b/majit/majit-translate/src/memory/gctransform/framework.rs index 8cbea59b820..54cc52a2241 100644 --- a/majit/majit-translate/src/memory/gctransform/framework.rs +++ b/majit/majit-translate/src/memory/gctransform/framework.rs @@ -175,6 +175,9 @@ pub const PYTHON_DISPATCH_SEEDS: &[&str] = &[ "eval::portal_runner", "eval::portal_runner_dispatch", "eval::portal_runner_result", + "eval::portal_activation_result", + "eval::portal_activation_bracketed", + "eval::enter_portal", "eval::eval_loop_jit", // The space-level helpers most builtins reach Python through. "baseobjspace::call_function", diff --git a/pyre/bench/synth/profile_hook_sees_a_recursive_portal_activation.py b/pyre/bench/synth/profile_hook_sees_a_recursive_portal_activation.py new file mode 100644 index 00000000000..6a28b3251ed --- /dev/null +++ b/pyre/bench/synth/profile_hook_sees_a_recursive_portal_activation.py @@ -0,0 +1,117 @@ +# pyre-check: selfcheck +# pyre-check: selfcheck-compiles=hot,root:rec +# The `root:` arm is the premise, not a relaxation: the activations this +# fixture counts are owed precisely because `rec` reaches the JIT and its +# recursive calls leave through the portal runner. A `rec` that stopped +# compiling would make every count below pass without testing anything. +# A profile hook is installed over a loop whose callee calls ITSELF, and the +# recursive activations are the ones a compiled caller performs through the +# portal runner rather than through the interpreter's call door. +# +# `pyframe.py execute_frame` brackets every activation with `ec.call_trace` and +# `leave`'s `_trace('leaveframe')`. pyre's portal sits at `execute_frame` level +# rather than at `dispatch` where upstream's merge point is, so the portal +# carries that bracket itself — and only the ROOT portal entry carried it. The +# recursive entries (`bhimpl_recursive_call_*` -> `bh_portal_runner_c`, the +# CALL_ASSEMBLER force leg, `compile_tmp_callback`'s callback loop, and the +# `jit_force_*_recursive_call_*` helpers) all reach the same portal body with a +# callee frame they built a line earlier, and reported nothing. +# +# THE SHAPE IS THE TEST: `rec(DEPTH)` is exactly DEPTH + 1 activations, so the +# owed count is a multiple of the tail that no partial reporting can reach by +# accident. Measured before the fix, `call/rec` read exactly `tail` from tail +# 10 000 upward — one activation per iteration, the outermost, which is the one +# that still goes through the interpreter — while cpython 3.14.6 reports +# `(DEPTH + 1) * tail`. +# +# THE TAIL IS ALSO A TEST: the loss was total and permanent from the moment +# compiled code took the caller over, so each count is checked at TWO tails and +# must track the difference between them. A count that saturates fails however +# large it is. +# +# `settrace` is checked alongside `setprofile` because they fail differently: +# `w_tracefunc` is guarded at the merge point, so arming a tracer exits compiled +# code, while `is_being_profiled` is a portal green, so arming a profiler mints +# a different cell that KEEPS the JIT and declines at the call. Only the second +# reaches the portal entries this fixture is about, so a fixture that armed only +# a tracer would pass without testing anything. +import sys + +WARM = 20000 # past the loop threshold (1039) many times over +TAILS = (2500, 10000) +DEPTH = 3 + + +def rec(n): + if n <= 0: + return 0 + return rec(n - 1) + 1 + + +def hot(n): + total = 0 + for _ in range(n): + total = (total + rec(DEPTH)) % 1000003 + return total + + +def measure(tail, install): + counts = {} + + def hook(frame, event, arg): + key = (event, frame.f_code.co_name) + counts[key] = counts.get(key, 0) + 1 + return hook + + # Compile the loop first, with nothing installed, so the arming below has + # compiled code to interrupt rather than merely a cold frame to decline. + hot(WARM) + install(hook) + try: + hot(tail) + finally: + install(None) + return counts + + +def exact(counts, key, expected, arm, failures): + got = counts.get(key, 0) + if got != expected: + failures.append('%s: %s = %d, expected %d' % (arm, key, got, expected)) + + +def tracks_the_tail(short, long_, key, per_iteration, arm, failures): + grew = long_.get(key, 0) - short.get(key, 0) + owed = (TAILS[1] - TAILS[0]) * per_iteration + if grew < owed: + failures.append( + '%s: %s grew by %d from tail %d to tail %d, owed %d — the count ' + 'does not track the tail, so the recursive activations are ' + 'unreported from some point onward' + % (arm, key, grew, TAILS[0], TAILS[1], owed) + ) + + +def main(): + failures = [] + for arm, install in (('profile', sys.setprofile), ('trace', sys.settrace)): + measured = [measure(tail, install) for tail in TAILS] + short, long_ = measured + for tail, counts in zip(TAILS, measured): + # One activation of the loop frame; DEPTH + 1 of `rec` per + # iteration, of which DEPTH are recursive. + exact(counts, ('call', 'hot'), 1, arm, failures) + exact(counts, ('return', 'hot'), 1, arm, failures) + exact(counts, ('call', 'rec'), (DEPTH + 1) * tail, arm, failures) + exact(counts, ('return', 'rec'), (DEPTH + 1) * tail, arm, failures) + for key in (('call', 'rec'), ('return', 'rec')): + tracks_the_tail(short, long_, key, DEPTH + 1, arm, failures) + if failures: + for line in failures: + print('FAIL', line) + return 1 + print('PASS a recursive portal activation reports to an installed hook') + return 0 + + +sys.exit(main()) diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 41485c4ef0d..7b4337387bf 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -1287,7 +1287,11 @@ pub extern "C" fn bh_portal_runner_c( ); } frame.set_last_instr_from_next_instr(next_instr as usize); - match crate::eval::portal_runner_result(frame) { + // `bhimpl_recursive_call_r` performs a call the traced code made, so the + // frame it hands over is entering its body for the first time and owes + // `execute_frame`'s hook bracket. The CRN arms below resume a frame whose + // activation already began and take the unbracketed entry instead. + match crate::eval::portal_activation_result(frame) { Ok(result) => result as i64, Err(mut err) => { majit_metainterp::blackhole::BH_LAST_EXC_VALUE @@ -4952,7 +4956,9 @@ fn bh_call_self_recursive_portal( let frame_ptr = create_callee_frame_in_ctx(ec, callable, args); let result = { let frame = unsafe { &mut *(frame_ptr as *mut PyFrame) }; - crate::eval::portal_runner_result(frame) + // The frame was built on the line above, so this entry is the callee's + // activation and owes `execute_frame`'s hook bracket. + crate::eval::portal_activation_result(frame) }; jit_drop_callee_frame(frame_ptr); Some(match result { diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index de2860cf6cb..027c073a253 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -8616,43 +8616,41 @@ fn eval_with_jit_inner( // // portal_ptr = eval_loop_jit at depth 0 (has jit_merge_point + // can_enter_jit back-edge), plain interpreter at depth > 0. - // `pyframe.py execute_frame` brackets the eval loop with - // `ec.call_trace(self)` and `ec.return_trace(self, w_exitvalue)`, and - // upstream's merge point sits inside that bracket: the jitdriver is - // applied to `PyFrame.dispatch` (`interp_jit.py`), which `execute_frame` - // calls between the two hooks. pyre's portal is this function instead, - // one level up, so the arms below enter the frame at its first bytecode, - // past the bracket, and a frame the JIT took over reported neither its - // `call` nor its `return` event. Every declining path above returns - // through `execute_frame_plain`, which reaches the bracket in - // `eval::eval_frame_plain_with_resume`, so these two arms are the only - // ones that owe one. - // - // Both hooks carry their own test — `call_trace` fires on - // `gettrace() is not None or profilefunc is not None`, `return_trace` on - // `gettrace() is not None` — so an untraced activation pays one null read - // per hook. Both run application-level Python and may move the frame, so - // each takes its argument from the root rather than from a saved word. - // - // The bracket is two nested `try`s under one `finally`, and the nesting is - // what decides which hook still runs after another one raised: - // `call_trace` sits in the outer `try`, `return_trace` in the inner - // `finally`, and `leave` in the outer one. So a `call_trace` that raises - // skips the eval body AND `return_trace` yet still owes the leave hook, - // `return_trace` runs on the raising path too — with `w_exitvalue` still - // the `None` the bracket opened with — and each hook that raises replaces - // whatever was pending, in that order. `?` on any of the three would - // flatten the nesting and drop the hooks after it. - fn portal_body(frame_root: &mut FrameRoot) -> PyResult { - match try_function_entry_jit(frame_root.frame()) { - Some(result) => result, - None => handle_jitexception(frame_root.frame()), - } - } + portal_activation_bracketed(&mut frame_root) +} + +/// `pyframe.py execute_frame`'s hook bracket around [`portal_runner_dispatch`], +/// for the entries that BEGIN an activation. +/// +/// Upstream's merge point sits inside that bracket: the jitdriver is applied to +/// `PyFrame.dispatch` (`interp_jit.py`), which `execute_frame` calls between the +/// two hooks. pyre's portal is one level up, at `execute_frame`'s own level, so +/// the dispatch below enters the frame at its first bytecode, past the bracket, +/// and a frame the JIT took over reported neither its `call` nor its `return` +/// event. Every declining path returns through `execute_frame_plain`, which +/// reaches the bracket in `eval::eval_frame_plain_with_resume`, so only the +/// portal arms owe one. +/// +/// Both hooks carry their own test — `call_trace` fires on +/// `gettrace() is not None or profilefunc is not None`, `return_trace` on +/// `gettrace() is not None` — so an untraced activation pays one null read per +/// hook. Both run application-level Python and may move the frame, so each +/// takes its argument from the root rather than from a saved word. +/// +/// The bracket is two nested `try`s under one `finally`, and the nesting is what +/// decides which hook still runs after another one raised: `call_trace` sits in +/// the outer `try`, `return_trace` in the inner `finally`, and `leave` in the +/// outer one. So a `call_trace` that raises skips the eval body AND +/// `return_trace` yet still owes the leave hook, `return_trace` runs on the +/// raising path too — with `w_exitvalue` still the `None` the bracket opened +/// with — and each hook that raises replaces whatever was pending, in that +/// order. `?` on any of the three would flatten the nesting and drop the hooks +/// after it. +fn portal_activation_bracketed(frame_root: &mut FrameRoot) -> PyResult { let ec = frame_root.frame().execution_context as *mut PyExecutionContext; if ec.is_null() { // No execution context is no hook to owe, and no `leave` either. - return portal_body(&mut frame_root); + return portal_runner_dispatch(frame_root); } // `w_exitvalue` is `execute_frame`'s own local, not a re-read of the result // word: it opens as the `None` the bracket opened with, takes the body's @@ -8665,7 +8663,7 @@ fn eval_with_jit_inner( let outer_result = match unsafe { (*ec).call_trace(frame_root.frame() as *mut PyFrame) } { Err(err) => Err(err), Ok(()) => { - let result = portal_body(&mut frame_root); + let result = portal_runner_dispatch(frame_root); if let Ok(value) = &result { w_exitvalue = *value; } @@ -8681,8 +8679,8 @@ fn eval_with_jit_inner( // `setprofile`'s `return` is not `return_trace`'s — that one tests // `gettrace() is not None`. It comes from `executioncontext.py leave`, // whose profile arm runs `_trace(frame, 'leaveframe', w_exitvalue)`, and - // this arm never reaches `leave`: the declining paths above return through - // `execute_frame_plain`, which does, and these two do not. + // this arm never reaches `leave`: the declining paths return through + // `execute_frame_plain`, which does, and the portal arms do not. let live = unsafe { (*ec).leaveframe_trace(frame_root.frame() as *mut PyFrame, w_exitvalue)? }; outer_result.map(|_| live) } @@ -8937,25 +8935,50 @@ fn debug_first_arg_int(frame: &PyFrame) -> Option { /// /// warmspot.py:997-1005: ExitFrameWithExceptionRef → re-raise. pub(crate) fn portal_runner_result(frame: &mut PyFrame) -> PyResult { - // warmspot.py ll_portal_runner: - // maybe_compile_and_run(state.increment_function_threshold, *args) - // return portal_ptr(*args) - // - // portal_ptr is the JIT-aware interpreter (jit_merge_point + - // can_enter_jit). pyre's equivalent is handle_jitexception → - // eval_loop_jit, NOT eval_frame_plain. Routing through - // eval_frame_plain here would skip maybe_enter_jit at every - // opcode of the recursive portal frame, which breaks parity for - // bhimpl_recursive_call_* paths. - // `ll_portal_runner` is an activation entry in its own right: recursive - // portal calls can reach it without the ordinary `eval_with_jit_inner` - // wrapper. Account before constructing `FrameRoot`, because a moving GC - // may change the frame's address while the activation remains the same. + enter_portal(frame, portal_runner_dispatch) +} + +/// [`portal_runner_result`] for an entry that BEGINS the activation rather than +/// continuing one, so it owes `pyframe.py execute_frame`'s hook bracket. +/// +/// The two are separated because pyre's portal is re-entered for reasons +/// upstream's is not. A `ContinueRunningNormally` handoff and a +/// CALL_ASSEMBLER completion resume a frame whose activation already began, and +/// upstream reaches those through `portal_ptr` — `PyFrame.dispatch`, INSIDE +/// `execute_frame`'s bracket — so they must not report a second `call` event. +/// Every other portal entry is handed a callee frame that was just constructed +/// (`create_callee_frame_in_ctx`, `create_self_recursive_callee_frame_impl_1_boxed`, +/// `emit_new_pyframe_inline_with_params`) and is the only bracket that frame +/// will ever get. +/// +/// Measured with `sys.setprofile` over a self-recursive callee driven from a +/// compiled loop: 4 activations per iteration are owed and 1 was reported — +/// exactly the outermost one, which reaches the interpreter's own +/// `execute_frame` — for every tail from 10 000 iterations up. +pub(crate) fn portal_activation_result(frame: &mut PyFrame) -> PyResult { + enter_portal(frame, portal_activation_bracketed) +} + +/// warmspot.py ll_portal_runner: +/// maybe_compile_and_run(state.increment_function_threshold, *args) +/// return portal_ptr(*args) +/// +/// portal_ptr is the JIT-aware interpreter (jit_merge_point + can_enter_jit). +/// pyre's equivalent is handle_jitexception → eval_loop_jit, NOT +/// eval_frame_plain. Routing through eval_frame_plain here would skip +/// maybe_enter_jit at every opcode of the recursive portal frame, which breaks +/// parity for bhimpl_recursive_call_* paths. +/// +/// `ll_portal_runner` is an activation entry in its own right: recursive portal +/// calls can reach it without the ordinary `eval_with_jit_inner` wrapper. +/// Account before constructing `FrameRoot`, because a moving GC may change the +/// frame's address while the activation remains the same. +fn enter_portal(frame: &mut PyFrame, body: fn(&mut FrameRoot) -> PyResult) -> PyResult { let _recursion_depth = pyre_interpreter::call::enter_recursive_frame(frame); let mut frame_root = FrameRoot::new(frame); frame_root.frame().fix_array_ptrs(); let _frame_guard = pyre_interpreter::eval::install_current_frame(frame_root.frame()); - portal_runner_dispatch(&mut frame_root) + body(&mut frame_root) } /// The dispatch half of `portal_runner_result`, taking the caller's @@ -8978,8 +9001,15 @@ fn portal_runner_dispatch(frame_root: &mut FrameRoot) -> PyResult { } } +/// The raw-ref spelling of [`portal_activation_result`], for the compiled-code +/// force helpers whose ABI returns a `PyObjectRef` and stashes the exception. +/// +/// Every caller builds the callee frame immediately before calling +/// (`run_frame_through_portal` receives the one the trace emitted; the +/// `jit_force_*_recursive_call_*` helpers construct theirs a line above), so +/// this entry always begins an activation. pub fn portal_runner(frame: &mut PyFrame) -> pyre_object::PyObjectRef { - match portal_runner_result(frame) { + match portal_activation_result(frame) { Ok(r) => r, Err(mut err) => { crate::call_jit::store_jit_exception(err.to_exc_object() as i64); From 4b67d9f005948a59c7c1d1956001b6e8ca90da7f Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 27 Aug 2026 04:44:53 +0900 Subject: [PATCH 07/10] jit: restore the frame chain when the portal's activation bracket closes `portal_activation_bracketed` ran `ec.leaveframe_trace` but not the rest of `executioncontext.py ExecutionContext.leave`, so a frame whose body ran as compiled code returned without restoring `ec.topframeref` from `f_backref` and without marking its caller escaped. `escaped()` is read by the walker at `jitcode_dispatch/mod.rs`, `residual_call.rs` and `inline_call.rs` to decide whether a caller has to be materialised. `leave_resumed_blackhole_frame` already closes exactly this scope for a blackhole resume, so its body is extracted into `leave_compiled_frame_chain` and shared: identity-guard `topframeref` against this frame, reach the caller through `vref_referent`, and force nothing. Forcing here raises `InvalidVirtualRef: frame-chain vref forced after its frame died` on `exception_escape_inlined_midframe_tb_node` and `exception_try_call_inlined_callee_raise`. The escape arm's firing condition was counted at 2001 / 21001 / 24001 across three probes; no Python-visible wrong answer was reproduced. Assisted-by: Claude --- pyre/pyre-jit/src/call_jit.rs | 47 +++++++++++++++++++++++++++++------ pyre/pyre-jit/src/eval.rs | 23 ++++++++++++++++- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 7b4337387bf..412bdd13940 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -2423,6 +2423,37 @@ fn leave_resumed_blackhole_frame( if frame_ptr.is_null() { return; } + leave_compiled_frame_chain(frame_ptr, got_exception); +} + +/// `executioncontext.py ExecutionContext.leave`'s frame-chain half for a frame +/// whose body ran as compiled code, shared by the blackhole resume above and by +/// the JIT portal's own activation bracket +/// (`eval::portal_activation_bracketed`). +/// +/// Both close a scope whose `enter` ran somewhere the interpreter's `leave` +/// never reaches, and both run once the compiled frame is gone — which is what +/// makes this a narrower operation than `leave`'s own escape branch: +/// +/// * The identity guard. `leave` reads `frame_vref` off `topframeref` and +/// forces it; that word only names this frame while its scope is the open +/// one. A walk that materialised an inlined callee can leave a different vref +/// there, and the caller has no other way to find out. +/// * No force. The guard has already established that `frame_vref` resolves to +/// this frame, so `leave`'s `frame_vref()` has nothing left to materialize, +/// and forcing a vref that was finished with the NULL form is exactly what +/// `force_pyframe_vref` refuses — measured as +/// `InvalidVirtualRef: frame-chain vref forced after its frame died` on +/// `exception_escape_inlined_midframe_tb_node` and +/// `exception_try_call_inlined_callee_raise`. +/// * The caller is reached through `vref_referent` rather than `get_f_back`, +/// which forces, for the same reason. +/// +/// The `topframeref` restore is idempotent for the portal, whose +/// `CurrentFrameGuard` writes back the same word from a shadow-stack slot when +/// it drops: `install_current_frame` seeds `f_backref` from the `topframeref` +/// it displaces, so the two agree by construction. +fn leave_compiled_frame_chain(frame_ptr: *mut PyFrame, got_exception: bool) { let ec = unsafe { (*frame_ptr).execution_context as *mut pyre_interpreter::PyExecutionContext }; if ec.is_null() { return; @@ -2439,13 +2470,6 @@ fn leave_resumed_blackhole_frame( // `nextblackholeinterp` therefore has to close that still-open scope. The // resume-data frame chain itself is not the application frame chain, and // releasing a BlackholeInterpreter does not restore `topframeref`. - // - // Nothing here forces a vref. The guard above already established that - // `frame_vref` resolves to this frame, so `leave`'s own `frame_vref()` has - // nothing left to materialize, and the caller is reached through - // `vref_referent` rather than `get_f_back`: this runs once the compiled - // frame is gone, and forcing a vref that was finished with the NULL form - // is exactly what `force_pyframe_vref` refuses. unsafe { (*ec).topframeref = (*frame_ptr).f_backref; if (*frame_ptr).escaped() || got_exception { @@ -2457,6 +2481,15 @@ fn leave_resumed_blackhole_frame( } } +/// [`leave_compiled_frame_chain`] for the JIT portal, whose frame is a `&mut` +/// borrow rather than a blackhole's virtualizable word. +pub(crate) fn leave_portal_frame_chain(frame: *mut PyFrame, got_exception: bool) { + if frame.is_null() { + return; + } + leave_compiled_frame_chain(frame, got_exception); +} + /// resume.py blackhole_from_resumedata parity: /// Decode rd_numb via ResumeDataDirectReader, build blackhole chain, /// run _run_forever. diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 027c073a253..e7dd039fcb2 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -8681,7 +8681,28 @@ fn portal_activation_bracketed(frame_root: &mut FrameRoot) -> PyResult { // whose profile arm runs `_trace(frame, 'leaveframe', w_exitvalue)`, and // this arm never reaches `leave`: the declining paths return through // `execute_frame_plain`, which does, and the portal arms do not. - let live = unsafe { (*ec).leaveframe_trace(frame_root.frame() as *mut PyFrame, w_exitvalue)? }; + let leave_result = + unsafe { (*ec).leaveframe_trace(frame_root.frame() as *mut PyFrame, w_exitvalue) }; + // `leave`'s frame-chain half, which the portal owed as much as the hook. It + // is the `finally` of `leave`'s own `try`, so a profile hook that raised + // does not skip it, and it runs while `topframeref` still names this frame. + // + // The narrow form, not `leave`'s own: this closes a scope whose body ran as + // compiled code, so it is the same situation as a blackhole resume and is + // shared with it. `leave_compiled_frame_chain` states why the identity + // guard and the absence of a force are load-bearing there. + // + // `got_exception` is `execute_frame`'s flag: false only when the body AND + // `return_trace` both completed, which is exactly `outer_result.is_ok()`. + // Without this a compiled frame that left escaped, or with an exception, + // returns to a caller that was never marked escaped — and `escaped()` is + // what the walker reads to decide whether that caller has to be + // materialised. + crate::call_jit::leave_portal_frame_chain( + frame_root.frame() as *mut PyFrame, + outer_result.is_err(), + ); + let live = leave_result?; outer_result.map(|_| live) } From 2418f5d057626c5152db042147c5bd0cf631c636 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 27 Aug 2026 09:44:17 +0900 Subject: [PATCH 08/10] jit-trace: key the inline sub-walk log on the PyCode object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `note_inline_subwalk_start` minted its green key from `raw_callee_code` — `w_code_get_ptr(w_code)`, the inner `CodeObject` — while every other mint in the tree uses the `PyCode` object: the function-entry key, the back-edge merge point, and `can_inline_callable` / `disable_noninlinable_function` twenty lines above the same call. `note_inline_subwalk_start` is the only production writer of `portal_trace_positions`, so every key `find_biggest_function` could return was CodeObject-keyed. `note_root_trace_too_long`'s huge-function arm filed a cell under a hash no reader computes, `can_inline_callable` found nothing on the next attempt and re-inlined the same callee, and because that arm answered `Some` the root took neither the disable nor `prepare_trace_segmenting`'s permanent stamp. Measured on `bench/synth/trace_too_long_inline_multiframe.py`: loops_compiled 2 -> 50, loops_aborted 31 -> 23, abrt_too_long 31 -> 23, abort_ceiling_refused 552 -> 321, abort_ceiling_banned 2 -> 0. Assisted-by: Claude --- .../src/jitcode_dispatch/inline_call.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 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 d5b758de824..b3a175c327d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -6299,15 +6299,28 @@ fn try_walker_inline_resolved_user_call_inner( // behind us and every exit below reads `result`, so the sequence // `find_biggest_function` pairs off cannot go out of step. The green // key is the callee's function-entry key, the one - // `disable_noninlinable_function` is applied to. + // `disable_noninlinable_function` is applied to — `callee_green_key` + // above, minted from the `PyCode` object. + // + // `w_code`, not the inner `CodeObject` `w_code_get_ptr` hands back: + // the third green is the code OBJECT (`interp_jit.py` `greens = + // ['next_instr', 'is_being_profiled', 'pycode']`), and every other + // mint in the tree — the function-entry key, the back-edge merge + // point, `can_inline_callable` and `disable_noninlinable_function` + // right above — uses it. Keying the log on the inner pointer put + // `find_biggest_function`'s answer in a hash no reader computes, so + // `note_root_trace_too_long`'s huge-function arm filed a cell that + // `can_inline_callable` never found, the same callee was re-inlined on + // the next attempt, and the root took neither the disable nor + // `prepare_trace_segmenting`'s permanent stamp. let subwalk_jd_no = crate::state::note_inline_subwalk_start( ( - crate::driver::make_green_key(raw_callee_code as *const (), 0, is_being_profiled), + callee_green_key, // The callee's greens are in scope here, so the log carries // them: `disable_noninlinable_function` applies to this key, // and it reaches a cell. Some(crate::driver::make_green_key_typed( - raw_callee_code as *const (), + w_code, 0, is_being_profiled, )), From 7c6295759cbfb22790175608b5a0215274185459 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 27 Aug 2026 09:44:17 +0900 Subject: [PATCH 09/10] metainterp: bracket the recursive-call family with the exception protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The portal runner is published behind an `extern "C"` boundary and cannot unwind, so a raise inside it arrives in `BH_LAST_EXC_VALUE` rather than out of the call — where `blackhole.py:351-360`'s blanket `except Exception` catches upstream's real unwind and hands it to `handle_exception_in_frame`. `bhimpl_jit_merge_point`'s recursive-portal arm and the four `handler_recursive_call_*` handlers neither cleared the cell before the call nor tested it after, so the call's value was whatever `bh_portal_runner_c` returns on its error path — `PY_NULL` — the frame left with that NULL installed as the result, and the exception stayed unread in the cell. `check_residual_call_exception_after`'s own doc names the families that owe the check; `recursive_call_*` was absent from it and from the code, while `residual_call_*`, `inline_call_*`, `call_assembler_*` and `cond_call_*` all perform it. No Python-level reproduction was produced: a 400-iteration probe over a guard-failing inlined callee that raises propagates correctly today. Assisted-by: Claude --- majit/majit-metainterp/src/blackhole.rs | 40 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 247cc480c72..b37ac4f1171 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -1215,6 +1215,24 @@ impl BlackholeInterpreter { self.entry_position, ); let result_type = self.jitdrivers_sd[jdindex].result_type; + // The portal runner is published behind an `extern "C"` boundary and + // cannot unwind, so a raise inside it arrives in `BH_LAST_EXC_VALUE` + // instead of out of the call — where `blackhole.py:351-360`'s blanket + // `except Exception` catches upstream's real unwind and hands it to + // `handle_exception_in_frame`. Clear the cell before the call and test + // it after, the protocol `check_residual_call_exception_after` + // documents and every `residual_call_*` / `inline_call_*` / + // `call_assembler_*` / `cond_call_*` family follows. Without it the + // call's value is whatever the runner returned on its error path — + // `PY_NULL` — the frame leaves with that NULL installed as the result, + // and the exception is left unread in the cell for an unrelated + // opcode to clear or for `bhimpl_abort_permanent` to deliver at the + // wrong bytecode. + BH_LAST_EXC_VALUE.with(|c| c.set(0)); + // The post-call position, where the codewriter put the can-raise + // opcode's `-live-` adjacency. Read before the call: the arms below + // do not advance `position`, but the handler search starts here. + let post_call_position = self.position; match result_type { BhReturnType::Void => { self.bhimpl_recursive_call_v(jdindex, gi, gr, gf, ri, rr, rf); @@ -1236,6 +1254,11 @@ impl BlackholeInterpreter { self.return_type = BhReturnType::Float; } } + // Upstream's raise unwinds out of `bhimpl_jit_merge_point`, so the + // frame does not leave on this path: the dispatch loop searches THIS + // frame's handlers at the post-call position first, and only a frame + // without one propagates. + check_residual_call_exception_after(self, post_call_position)?; Err(DispatchError::LeaveFrame) } @@ -11885,9 +11908,12 @@ fn handler_recursive_call_i( ) -> Result { let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) = read_recursive_call_args(bh, code, p); - bh.registers_i[code[p] as usize] = bh.bhimpl_recursive_call_i( + BH_LAST_EXC_VALUE.with(|c| c.set(0)); + let result = bh.bhimpl_recursive_call_i( jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, ); + check_residual_call_exception_after(bh, p + 1)?; + bh.registers_i[code[p] as usize] = result; Ok(p + 1) } // blackhole.py bhimpl_recursive_call_r @@ -11898,11 +11924,14 @@ fn handler_recursive_call_r( ) -> Result { let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) = read_recursive_call_args(bh, code, p); - bh.registers_r[code[p] as usize] = bh + BH_LAST_EXC_VALUE.with(|c| c.set(0)); + let result = bh .bhimpl_recursive_call_r( jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, ) .0 as i64; + check_residual_call_exception_after(bh, p + 1)?; + bh.registers_r[code[p] as usize] = result; Ok(p + 1) } // blackhole.py bhimpl_recursive_call_f @@ -11913,11 +11942,14 @@ fn handler_recursive_call_f( ) -> Result { let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) = read_recursive_call_args(bh, code, p); - bh.registers_f[code[p] as usize] = bh + BH_LAST_EXC_VALUE.with(|c| c.set(0)); + let result = bh .bhimpl_recursive_call_f( jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, ) .to_bits() as i64; + check_residual_call_exception_after(bh, p + 1)?; + bh.registers_f[code[p] as usize] = result; Ok(p + 1) } // blackhole.py bhimpl_recursive_call_v @@ -11928,8 +11960,10 @@ fn handler_recursive_call_v( ) -> Result { let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) = read_recursive_call_args(bh, code, p); + BH_LAST_EXC_VALUE.with(|c| c.set(0)); bh.bhimpl_recursive_call_v( jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, ); + check_residual_call_exception_after(bh, p)?; Ok(p) } From 98591dc58143e1e7b7b3f8a630e597bbb8cadc9a Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 27 Aug 2026 10:34:51 +0900 Subject: [PATCH 10/10] bench/synth: re-record trace_too_long_inline_multiframe's jit-stats The green-key fix makes `disable_noninlinable_function` reach a cell, so the huge inlined callee stops being re-inlined: loops_compiled 2 -> 50 (wasm 0 -> 48) loops_aborted 31 -> 23 (wasm 30 -> 22) fbw_blackhole_adopted_single_frame 17 -> 16 (wasm 16 -> 15) fbw_blackhole_adopted_multi_frame 14 -> 7 check.py reads the two adoption counters as regressions on a fall, because a fall normally means the adoption path stopped firing and the legacy replay path came back. It did not: the adoption total still equals `loops_aborted` exactly on every backend, before (17+14=31, 16+14=30) and after (16+7=23, 15+7=22). The counters fall because there are eight fewer aborts to adopt. Assisted-by: Claude --- .../trace_too_long_inline_multiframe.cranelift.jitstats | 8 ++++---- .../trace_too_long_inline_multiframe.dynasm.jitstats | 8 ++++---- .../synth/trace_too_long_inline_multiframe.wasm.jitstats | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats index 5895a645a10..42d9964ca3e 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.cranelift.jitstats @@ -2,8 +2,8 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=14 -fbw_blackhole_adopted_single_frame=17 +fbw_blackhole_adopted_multi_frame=7 +fbw_blackhole_adopted_single_frame=16 fbw_escape_plain_fallback=0 fbw_escape_plain_fallback_unclean=0 fbw_midbody_latch_new_unjournaled=0 @@ -13,6 +13,6 @@ field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=0 internal_compile_panics=0 -loops_aborted=31 -loops_compiled=2 +loops_aborted=23 +loops_compiled=50 retraces_compiled=0 diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats index 5895a645a10..42d9964ca3e 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.dynasm.jitstats @@ -2,8 +2,8 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=14 -fbw_blackhole_adopted_single_frame=17 +fbw_blackhole_adopted_multi_frame=7 +fbw_blackhole_adopted_single_frame=16 fbw_escape_plain_fallback=0 fbw_escape_plain_fallback_unclean=0 fbw_midbody_latch_new_unjournaled=0 @@ -13,6 +13,6 @@ field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=0 internal_compile_panics=0 -loops_aborted=31 -loops_compiled=2 +loops_aborted=23 +loops_compiled=50 retraces_compiled=0 diff --git a/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats b/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats index 8bd30019c65..9181aed5ae4 100644 --- a/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats +++ b/pyre/bench/synth/trace_too_long_inline_multiframe.wasm.jitstats @@ -2,8 +2,8 @@ bridges_compiled=0 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 -fbw_blackhole_adopted_multi_frame=14 -fbw_blackhole_adopted_single_frame=16 +fbw_blackhole_adopted_multi_frame=7 +fbw_blackhole_adopted_single_frame=15 fbw_escape_plain_fallback=0 fbw_escape_plain_fallback_unclean=0 fbw_midbody_latch_new_unjournaled=0 @@ -13,6 +13,6 @@ field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=0 internal_compile_panics=0 -loops_aborted=30 -loops_compiled=0 +loops_aborted=22 +loops_compiled=48 retraces_compiled=0