-
Notifications
You must be signed in to change notification settings - Fork 19
jit: stop truncating lowering at abort_permanent, attach the missing catch edge, and let residual calls re-enter the JIT #860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a35b628
6ba59c6
6e4443f
eff66df
23b9418
f5cd058
a514d7b
9eaaecd
9d36419
456577e
ae456d1
a5d05ec
e530069
baedd4c
973f89b
9cc4940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2053,40 +2053,65 @@ impl MiniMarkGC { | |
| } | ||
|
|
||
| fn seed_major_root(&mut self, gcref: GcRef) { | ||
| if !gcref.is_null() && self.is_managed_heap_object(gcref.0) { | ||
| let hdr = unsafe { header_of(gcref.0) }; | ||
| // SAFETY: header_of returns a raw pointer; keep each access | ||
| // short-lived to avoid creating overlapping exclusive borrows. | ||
| let newly_marked = unsafe { | ||
| if !(*hdr).has_flag(flags::VISITED) { | ||
| (*hdr).set_flag(flags::VISITED); | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| }; | ||
| if newly_marked { | ||
| self.incr_state.gray_stack.push(gcref.0); | ||
| self.note_nonmoving_nursery_mark(gcref.0); | ||
|
|
||
| // incminimark.py:1322-1340 requires marking worklists to | ||
| // contain no nursery objects after a minor. Pyre JITFRAME | ||
| // gcmap spills are collector metadata, not SETFIELD_GC | ||
| // writes: a frame first grayed in STATE_SCANNING can leave | ||
| // the active shadow stack before the next minor without a | ||
| // mutator barrier. Arm this newly seeded old root in the | ||
| // existing old_objects_pointing_to_young shape once, so that | ||
| // minor forwards any such spill before resetting nursery. | ||
| if !self.is_in_nursery(gcref.0) | ||
| && unsafe { (*hdr).has_flag(flags::TRACK_YOUNG_PTRS) } | ||
| { | ||
| unsafe { (*hdr).clear_flag(flags::TRACK_YOUNG_PTRS) }; | ||
| self.remembered_set.push(gcref.0); | ||
| } | ||
| if gcref.is_null() | ||
| || !self.is_managed_heap_object(gcref.0) | ||
| || !self.may_enter_marking_worklist(gcref.0) | ||
| { | ||
| return; | ||
| } | ||
| let hdr = unsafe { header_of(gcref.0) }; | ||
| // SAFETY: header_of returns a raw pointer; keep each access | ||
| // short-lived to avoid creating overlapping exclusive borrows. | ||
| let newly_marked = unsafe { | ||
| if !(*hdr).has_flag(flags::VISITED) { | ||
| (*hdr).set_flag(flags::VISITED); | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| }; | ||
| if newly_marked { | ||
| self.incr_state.gray_stack.push(gcref.0); | ||
| self.note_nonmoving_nursery_mark(gcref.0); | ||
|
|
||
| // incminimark.py:1322-1340 requires marking worklists to | ||
| // contain no nursery objects after a minor. Pyre JITFRAME | ||
| // gcmap spills are collector metadata, not SETFIELD_GC | ||
| // writes: a frame first grayed in STATE_SCANNING can leave | ||
| // the active shadow stack before the next minor without a | ||
| // mutator barrier. Arm this newly seeded old root in the | ||
| // existing old_objects_pointing_to_young shape once, so that | ||
| // minor forwards any such spill before resetting nursery. | ||
| if !self.is_in_nursery(gcref.0) && unsafe { (*hdr).has_flag(flags::TRACK_YOUNG_PTRS) } { | ||
| unsafe { (*hdr).clear_flag(flags::TRACK_YOUNG_PTRS) }; | ||
| self.remembered_set.push(gcref.0); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// `incminimark.py:2739-2753 _collect_obj`: an object in the nursery is | ||
| /// never appended to `objects_to_trace` — "such an object is handled by | ||
| /// minor collections and shouldn't be specially handled by major | ||
| /// collections" — and `visit` (:2797-2799) asserts | ||
| /// `not self.is_in_nursery(obj)` on every popped entry. The worklist | ||
| /// outlives the mutator resuming, so a nursery address left on it is read | ||
| /// back after the next `reset_nursery` has recycled those bytes: the | ||
| /// header decodes to a garbage `type_id`. | ||
| /// | ||
| /// A young object stays reachable without the entry. When the minor | ||
| /// promotes it, `drag_out_root` greys the promoted copy | ||
| /// (`_trace_drag_out1_marking_phase`); when it is only reachable from an | ||
| /// old parent, that parent is in the remembered set and gets requeued | ||
| /// while MARKING (`_add_to_more_objects_to_trace`). | ||
| /// | ||
| /// The non-moving oldgen major is the one mode that marks the nursery in | ||
| /// place — it leaves those bytes untouched by contract, and | ||
| /// [`Self::note_nonmoving_nursery_mark`] clears the marks afterwards. | ||
| #[inline] | ||
| fn may_enter_marking_worklist(&self, addr: usize) -> bool { | ||
| self.oldgen_nonmoving_active || !self.is_in_nursery(addr) | ||
| } | ||
|
|
||
| /// Record a nursery object greyed during a non-moving major so its | ||
| /// stale `flags::VISITED` is cleared as the strictly-last collection step. | ||
| /// No-op (just a range check) outside a non-moving major; the normal | ||
|
|
@@ -2552,7 +2577,7 @@ impl MiniMarkGC { | |
| /// type system guarantees every `Ptr(GcStruct)` is GC-managed; it converges | ||
| /// away once every `gc_ptr_offsets` target is a real GC allocation. | ||
| fn grey_child(&mut self, addr: usize, holder_addr: usize, slot_addr: usize, site: &str) { | ||
| if self.is_managed_heap_object(addr) { | ||
| if self.is_managed_heap_object(addr) && self.may_enter_marking_worklist(addr) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When marking finishes through the standalone AGENTS.md reference: AGENTS.md:L194-L196 Useful? React with 👍 / 👎. |
||
| let hdr = unsafe { header_of(addr) }; | ||
| let type_id = unsafe { (*hdr).type_id() }; | ||
| if type_id as usize >= self.types.len() { | ||
|
|
@@ -2586,6 +2611,12 @@ impl MiniMarkGC { | |
| // varsize GC-pointer array is never buffered (the gray stack already | ||
| // retains the live children); only the bounded fixed-field offsets are | ||
| // copied into the reused `mark_offsets` buffer. | ||
| // incminimark.py:2797-2799 `visit`: `ll_assert(not | ||
| // self.is_in_nursery(obj), "nursery object in 'objects_to_trace'")`. | ||
| debug_assert!( | ||
| self.may_enter_marking_worklist(obj_addr), | ||
| "nursery object {obj_addr:#x} in the marking worklist", | ||
| ); | ||
| let type_id = unsafe { (*header_of(obj_addr)).type_id() }; | ||
| // A non-moving major can trace a live nursery object without first | ||
| // copying it into its reserved old-gen shadow. Keep that unoccupied | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Regression oracle: nested try/except where each handler reads | ||
| # `sys.exc_info()`. After an inner handler unwinds, POP_EXCEPT must restore the | ||
| # slot to the prev its matching PUSH_EXC_INFO saved (the outer ValueError), and | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Clarify the Line 3 contains the malformed phrase “the prev its matching,” which makes the documented restoration behavior difficult to understand. Rewrite it to state that 🤖 Prompt for AI Agents |
||
| # after the outer handler to None. Expected per-iteration signature | ||
| # 2*1 + 1*10 + 0*100 = 12 → 360000. | ||
| # | ||
| # Was `_pending/`: the JIT answered 3320000 (trait) / 3360000 (FBW walker) | ||
| # because the nested handlers' POP_EXCEPT restores were not lowered to the EC | ||
| # `sys_exc_value` slot — the in-handler `sys.exc_info()` may-force ended the | ||
| # authoritative walk, so the slot kept the inner exception after the handler | ||
| # exited. Both tracers now answer 360000, matching the interpreter and CPython. | ||
| import sys | ||
|
|
||
| N = 30000 | ||
|
|
||
|
|
||
| def classify(t): | ||
| if t is ValueError: | ||
| return 1 | ||
| if t is KeyError: | ||
| return 2 | ||
| if t is None: | ||
| return 0 | ||
| return 9 | ||
|
|
||
|
|
||
| def run(n): | ||
| acc = 0 | ||
| i = 0 | ||
| while i < n: | ||
| try: | ||
| raise ValueError("outer") | ||
| except ValueError: | ||
| try: | ||
| raise KeyError("inner") | ||
| except KeyError: | ||
| acc += classify(sys.exc_info()[0]) * 1 | ||
| acc += classify(sys.exc_info()[0]) * 10 | ||
| acc += classify(sys.exc_info()[0]) * 100 | ||
| i += 1 | ||
| return acc | ||
|
|
||
|
|
||
| def main(): | ||
| print(run(N)) | ||
|
|
||
|
|
||
| main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1334,6 +1334,30 @@ pub fn call_user_function( | |
| call_user_function_with_eval(frame, callable, args, eval_fn) | ||
| } | ||
|
|
||
| /// Residual-call sibling of [`call_user_function_plain`] that keeps the | ||
| /// JIT-aware eval function. | ||
| /// | ||
| /// `blackhole.py:1225 bhimpl_residual_call_r_i` is `cpu.bh_call_i(func, ...)` | ||
| /// — it invokes the *translated function*, and when that function's graph | ||
| /// reaches a `jit_merge_point` (`execute_frame` does) the JIT is entered | ||
| /// normally. "Opaque to the trace" does not mean "the JIT is off inside": | ||
| /// upstream has no flag that disables it for the extent of a residual call. | ||
| /// `bhimpl_recursive_call_*` (`blackhole.py:1095-1132`) is not the only way | ||
| /// to reach the portal — it is the path the codewriter emits when the callee | ||
| /// is *statically* the portal graph. | ||
| /// | ||
| /// Re-entrant tracing is prevented where upstream prevents it, on the green | ||
| /// key: `warmstate.py:473-477` skips a hot back-edge while `JC_TRACING` is | ||
| /// set, which pyre mirrors with the `driver.is_tracing()` guard in | ||
| /// `maybe_compile_and_run`. | ||
| pub fn call_user_function_residual( | ||
| frame: &PyFrame, | ||
| callable: PyObjectRef, | ||
| args: &[PyObjectRef], | ||
| ) -> PyResult { | ||
| call_user_function_with_eval(frame, callable, args, get_eval_fn()) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a compiled AGENTS.md reference: AGENTS.md:L16-L18 Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| /// Plain interpreter-only user-function call. | ||
| /// | ||
| /// JIT residual helpers should use this instead of the injected eval override. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.