PyFrame drops execution_context and w_globals; the #1372 review follow-ups - #1388
PyFrame drops execution_context and w_globals; the #1372 review follow-ups#1388youknowone wants to merge 15 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 131 files, which is 31 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (131)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/d55c4a45240b258f3ddc930f2082da6b2c13a511/pyre-interpreter/src/pyframe.rs#L2980
PyFrame::new now installs the execution context in TLS, but pyre-wasm-test/src/main.rs:25-36 calls PyFrame::new_with_context directly without doing so. Since this commit removes the frame-owned context, that launcher executes every frame with a null context: eval_frame_plain_with_resume skips ExecutionContext::enter/leave, and APIs such as sys.settrace silently become no-ops. Install the supplied context in that launcher or make the entry adapter establish it before execution.
AGENTS.md reference: AGENTS.md:L66-L77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit dedd760). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptationsNone. |
`PyFrame` no longer carries the execution context, and this launcher never installed the one it constructs in the OS-thread locals, so `space.getexecutioncontext()` answers null for every program it runs. `eval_frame_plain_with_resume` then takes its context-free arm: no `ExecutionContext::enter`/`leave`, no `call_trace`/`return_trace`, so `sys.settrace` and the rest of the state reached through the context are inert. `pyrex/src/lib.rs`, `pyre-wasm/src/lib.rs` and `PyFrame::new` each install it themselves; the comment in `PyFrame::new` named the first two as the whole set and now names this one too. Reported by Codex on #1388. Assisted-by: Claude
d55c4a4 to
dedd760
Compare
Confirmed and fixed in
One neighbouring gap left alone deliberately: this launcher also does not call Also on this push: the branch is rebased onto current — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dedd760b49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let frame_stores_global = unsafe { | ||
| crate::w_code_frame_stores_global(_roots.get(root_base), _roots.get(root_base + 1)) | ||
| }; |
There was a problem hiding this comment.
When two threads first call functions that share one code object but have different __globals__, both can reach this new branch concurrently. w_code_frame_stores_global performs an unsynchronized null check and write on PyCode.w_globals, so both calls may return false and neither frame creates the debugdata override; now that PyFrame.w_globals is removed, one frame's LOAD_GLOBAL then reads whichever shared pointer wins, in addition to the raw Rust data race. Make the first-globals publication atomic/locked or preserve the supplied globals on each frame when publication races.
AGENTS.md reference: AGENTS.md:L19-L24
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 21a9aea.
The race is real and this PR is what makes it a wrong answer rather than a wasted store: false
is the answer that sends a frame to the code object for its globals, so two threads that both
read the null and both store both answer false, and with PyFrame.w_globals gone the loser's
frame reads the winner's namespace.
w_code_frame_stores_global now publishes by compare-exchange and computes its answer against
whichever pointer won — the loser gets true and stores its own override. w_code_get_w_globals
and w_code_set_w_globals reach the slot through the same atomic view. This is the shape
w_code_getconstant already uses for co_consts_w ("PyPy's GIL serializes first access... Pyre
is free-threaded, so every reader and writer uses the AtomicPtr element").
The field keeps its raw-pointer type rather than becoming an AtomicPtr: the JIT reads this word
directly at CODE_W_GLOBALS_OFFSET and folds it, so the layout is left alone and only the
mutator-side accesses are made atomic. walk_w_globals_stamped_code_roots keeps its plain
&mut — it runs with every mutator at a safepoint, which its doc comment already states.
— commented by Claude
`compute_gcmap` opens with `if arg is None: continue`, before it reads `arg.type`. `collect_guards` read only the type, so a `None` fail arg reached the slot list. The skip is not vacuous here: `infer_fail_arg_types` and `resolve_fail_arg_types` both type an `OpRef::NONE` as `Ref`, because a virtual leaves a hole in fail_args and a virtual object is a GCREF. The type test alone therefore marked the hole's slot, which owns no root. The comment claimed no hole could reach the map, on the grounds that `spill_guard_fail_args` resolves every fail arg through `resolve_opref`, which asserts on `OpRef::NONE`. That assert exists, but the three `spill_guard_fail_args` call sites are all on the `call_may_force` / `call_release_gil` paths, so the argument does not cover a guard in general. Replaced with the reason the skip is load-bearing. Reported by CodeRabbit on #1372. Assisted-by: Claude
`JitDriver::decode_descriptor_values`, `decode_exit_layout_values`, `resume_layout_with_descriptor_slot_types`, `compile::terminal_exit_layout_for_trace` and `decode_values_with_layout` have no caller in either the lib build or the test build. The driver returns raw guard-failure data by design — "State restoration and bridge/blackhole decision happen in the caller's handle_fail()" — so the decode step these performed belongs to the caller. `pyre-jit`'s `eval.rs` carries its own `decode_exit_layout_values` with three callers; the copy here was a byte-identical twin of it. None of the five carries a doc comment or an upstream citation, and no `rpython/`/`pypy/` identifier matches their names under case- and underscore-insensitive comparison. The `exit_types` comment that named `decode_values_with_layout` now names the property instead, and records that the gcmap is not a consumer of that typing: `compute_gcmap` drops a `None` failarg before reading its type. Assisted-by: Claude
`Nursery::reset_range` is a safe `pub fn` that writes raw bytes at caller-supplied addresses, and its three bounds checks were `debug_assert!`. A release build performed none of them, so an out-of-range `end` wrote outside the arena and a `start` greater than `end` wrapped `end - start` into a fill of nearly `usize::MAX` bytes. It now intersects the requested range with the arena and returns when the result is empty; the assertions stay for debug builds. `reserve_nursery_gap` derives `next_free` from `size_for_typeid`, which decodes the pinned object's extent from its header. An extent that overstates the object pushed `free` past the barrier being published in the same block, and `Nursery::alloc` then compared against that bound and returned memory beyond the gap. `next_free` is now clamped to `next_top`. `Nursery::set_free_ptr` and `set_top_ptr` keep their `debug_assert`s: both are `unsafe fn` whose documented contract puts the bounds on the caller. Reported by CodeRabbit on #1372. Assisted-by: Claude
None has a caller in either the lib build or the test build. - `GuardOpt::set_guard` wraps a single `IndexMap::insert`. - `visit_value`, a nested helper in the const-ptr walk, lost its caller while its siblings `visit_operands` / `visit_op` / `visit_op_info` keep theirs. - `is_virtual_concat` and `is_virtual_slice` are `.is_some()` wrappers over `get_concat_info` / `get_slice_info`, which are called directly. - `ResumeStorage::rd_consts_mut_for_gc` names `MetaInterp::walk_rd_consts_refs` as its only caller. That walker exists and is wired, but reaches the pool through `SharedConstPool::as_mut_vec_for_gc` directly, so the accessor sits beside the path rather than on it. `Opencoder::refresh_from_gc` is left in place. It is also unreached, but its doc calls it the required write-back point for the `_refs` shadow-stack adaptation, and `rooted_ref_indices` is populated by `_encode_ptr` and otherwise read only by `release_roots`. Deleting it would remove the record of missing wiring rather than dead code. Assisted-by: Claude
…t minor `do_collect_nursery` sampled `any_pinned_object_kept` into a discarded local and always announced `ExtraRootWalkKind::Minor`. The walkers that read the kind — `walk_rd_consts_refs`, the front `TargetToken` walk, the method cache and mapdict scans — each skip a clean area on a minor. `collect_roots_in_nursery` derives `use_jit_frame_stoppers` from that same flag and passes it as `walk_roots(is_minor=...)`: a pinned object created before the previous minor is still in the nursery and was never promoted, so the skip drops the only edge reaching it. The sampled flag now selects the announced kind. Assisted-by: Claude
…test items `finish_trace_for_parity_preserves_captured_snapshots` carries a full `assert_eq!` body but no `#[test]`, so `#[allow(dead_code)]` was silencing the fact that it never ran. It runs and passes. `may_force_test_lock` in `pyjitpl` has no caller in any cfg; the cranelift backend has its own copy, which is the one every `may_force` test takes. The `TID` constant in `a_named_field_resolves_by_name_through_an_ambiguous_offset` is unused — that test calls `field_specs_from_layout` directly. Assisted-by: Claude
`interp_jit.py PyFrame._virtualizable_` lists `pycode`, `valuestackdepth` and `debugdata` as the virtualizable scalars; `ec` is a red in `PyPyJitDriver.reds`, not a frame field. `PyFrame.execution_context` is removed and the sites that read it now take the current activation's context, as `space.getexecutioncontext()` does. `normalize_raise_varargs_fn` no longer needs the frame pointer for it. `PyFrame.w_globals` is likewise removed. Upstream `bcd8653e5ec` dropped it when `PyCode.frame_stores_global` landed, and `InstanceRepr._parse_field_list` skips a `_virtualizable_` name with no concrete field, so the entry produced no scalar. `get_w_globals` reads the code object, and `FrameDebugData` carries the snapshot the debug path needs. `PYFRAME_W_GLOBALS_OFFSET` is replaced by `FRAME_DEBUG_DATA_W_GLOBALS_OFFSET`; the vable scalar table loses its fourth entry and `failed_attr_cleanup` narrows to `u8`. `majit_metainterp::backend_runtime` collects `set_jitframe_gc_type_id` and `install_gc_standalone` behind the same feature selection that constructs `BackendImpl`, so a frontend does not repeat the test and disagree under workspace feature unification. `WarmRunnerDesc.make_cpu` constructs one CPU and `MetaInterpStaticData.cpu` owns it. Assisted-by: Claude
Records the kind each registered walker sees across two minors with a pinned root alive. Without the preceding commit the pair reads `[Minor, Minor]`; the second walk now announces `Major`. Assisted-by: Claude
`pyframe.py get_w_globals` ends in `jit.promote(self.pycode).w_globals`. The port read `self.pycode` unpromoted while the doc comment claimed the promoted read, which was harmless only while `w_globals` was still a virtualizable scalar on `PyFrame`. With that field gone the code object is the sole source and nothing pins it to a trace constant, so the globals read becomes a load the optimizer cannot fold. The promote binds a local: `lower_promote_stmt` takes a single-ident LHS, so a field assignment would drop it silently. Assisted-by: Claude
`quasiimmut.py QuasiImmut` registers the owning `JitCellToken`, and `invalidate` sets `looptoken.invalidated` and asks the CPU to activate every still-unpatched `GUARD_NOT_INVALIDATED` that token owns. The registry held one `Arc<AtomicBool>` per compiled artifact instead, so an invalidation reached only the fragments registered by then. It now holds `Arc<dyn QuasiImmutLoopToken>` and calls `invalidate_for_quasi_immut`, which the backend implements on its own token. `can_inline_callable` and `disable_noninlinable_function` are reached through their typed green-key forms, so a recursive inline decision is looked up under the key shape the warm state stores. `Trace` gains `recorded_ops_total`, which `cut` does not rewind, and the bridge driver's setup-abort test reads it beside `num_ops`. A body walk that recorded operations and had them cut back to the setup position no longer answers that test as though nothing had been recorded, so a trace-too-long abort stops reading as a deterministic setup failure that permanently declines its source guard. The 80 jit-stats baseline files this moves, over 27 fixtures, are re-recorded. Assisted-by: Claude
`collect_outer_active_boxes` reported a branch guard's kept operand-stack slot as unsourced when the walk mirror held `OpRef::NONE` and no edge-move recovery covered it, without consulting the virtualizable shadow. The resolution below that report already reads the shadow for an operand-stack slot the guard pc's color map does not claim, so the report declined a case its own resolver handles. The decline raises `BranchGuardKeptSlotUnsourced` at a capture point a residual has already run past, which the walk counts as `fbw_rolled_back_with_effects`. Report the hole only when the shadow also holds no live Ref for the semantic slot. On `raise_reg_unbound_jitstress` the walk mirror stands at depth 1 while the branch guard resumes at depth 3: both kept slots read NONE from the mirror and a live `RefOp` from the shadow. The `raise_reg_unbound_jitstress` baselines re-recorded in "jit: align quasi-immutable ownership and recursive bridge state" are restored to their previous values — fbw_rolled_back_with_effects 1 -> 0, loops_aborted 2 -> 1, loops_compiled 8 -> 9. Assisted-by: Claude
… unseeded `replace_movable_load_global_namespace_with_frame_globals` substitutes the codewriter's null LOAD_GLOBAL namespace placeholder with the frame's `get_w_globals()`. For an inline sub-walk it reads that off the callee's `portal_frame_reg`, and where that register is unseeded it returned, leaving the placeholder standing. `try_walker_load_global_cell_fold` then declines on the null namespace before reaching its builtins leg, the residual survives, and `ensure_residual_call_args_bound` aborts the trace on the same unseeded frame register as the call's third argument. The unseeded frame does not by itself block the fold: its builtins leg already handles `frame_ptr == 0` by deriving the builtin module from the namespace's `__builtins__` cell. Only the null namespace does. The callee's own `__globals__` is recorded in `inline_callee_consts`, so name it directly rather than return. Immovable namespaces only, matching `guard_current_frame_globals_identity` where both fold legs end: a movable one declines there anyway, so substituting it would bake a pointer the GC may forward into the surviving residual. The root frame is still never borrowed. `polymorphic_binary_receiver` returns to the values it carried before "jit: align quasi-immutable ownership and recursive bridge state": bridges_compiled 2 -> 3, guard_failures 845 -> 788, loops_aborted 1 -> 0. The `retraces_compiled` entry those baselines gained there is kept. Assisted-by: Claude
Twelve upstream citations this branch adds name a line number. `scripts/check-new-line-citations.py` reports them against `origin/main`, which now carries the check (#1408). `warmstate.py:446/458/473/483/491/511` all fall inside `maybe_compile_and_run`; `assembler.py:46` is `compute_gcmap`; `pyframe.py:132` is `get_w_globals`; `interp_jit.py:25-30` is `PyFrame._virtualizable_` and `:67` is `PyPyJitDriver.reds`. Assisted-by: Claude
`PyFrame` no longer carries the execution context, and this launcher never installed the one it constructs in the OS-thread locals, so `space.getexecutioncontext()` answers null for every program it runs. `eval_frame_plain_with_resume` then takes its context-free arm: no `ExecutionContext::enter`/`leave`, no `call_trace`/`return_trace`, so `sys.settrace` and the rest of the state reached through the context are inert. `pyrex/src/lib.rs`, `pyre-wasm/src/lib.rs` and `PyFrame::new` each install it themselves; the comment in `PyFrame::new` named the first two as the whole set and now names this one too. Reported by Codex on #1388. Assisted-by: Claude
`pycode.py frame_stores_global` reads `w_globals` and stores into it under the GIL, so the pair is indivisible. The port did the same read and store unsynchronized. Two threads first running one code object in different globals can both read the null and both store, and both then answer `false` — which is the answer that makes a frame take its globals from the code object. With `PyFrame.w_globals` removed the loser's frame reads the winner's namespace instead of its own, and the unsynchronized store is a data race in its own right. The publication is now a compare-exchange, and the answer is computed against whichever pointer won. `w_code_get_w_globals` and `w_code_set_w_globals` reach the slot through the same atomic view. `walk_w_globals_stamped_code_roots` keeps its plain `&mut`: it runs with every mutator at a safepoint. Reported by Codex on #1388. Assisted-by: Claude
dedd760 to
21a9aea
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21a9aea328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.concrete_namespace = frame.w_globals; | ||
| self.concrete_execution_context = frame.execution_context; | ||
| self.concrete_namespace = frame.get_w_globals(); | ||
| self.frame_w_globals = ctx.const_ref(self.concrete_namespace as usize as i64); |
There was a problem hiding this comment.
When one PyCode is first compiled under module globals A and later invoked with an equal-shaped module globals B, the warm key remains (code, pc) and compatibility can admit the existing loop. Making frame_w_globals a constant here causes guard_current_frame_globals_identity to skip its runtime GuardValue, so the module-cell fold remains bound to A even though B's frame carries its override in debugdata; LOAD_GLOBAL can consequently return A's value. Keep this value derived from the live red frame/debugdata rather than the recording frame's namespace.
AGENTS.md reference: AGENTS.md:L26-L33
Useful? React with 👍 / 👎.
| if frame_stores_global { | ||
| frame.set_w_globals(w_globals); |
There was a problem hiding this comment.
When frame_stores_global is true, such as executing a shared code object with a second movable globals dictionary, FrameBox::new may collect before this branch runs. Its root set no longer includes w_globals, and the code object only retains the first globals dictionary, so the local pointer can be relocated or reclaimed before set_w_globals stores it in debugdata. Root and reread the override across the frame allocation; otherwise f_globals and subsequent namespace accesses can use a stale pointer.
Useful? React with 👍 / 👎.
1 participant
Follow-up to #1372. Seven of the first eight commits answer review comments left on that PR after it was pushed; the eighth removes two
PyFramefields. The six that follow keep the fold that removal cost, correct two jit-stats baselines it re-recorded as improvements when they were regressions, convert this branch's line-number citations to symbols, and answer the review on this PR.IncrementalMiniMarkGC— a surviving pin has to disable the minor root-walk skipdo_collect_nurserysampledany_pinned_object_keptinto a discarded local and then always announcedExtraRootWalkKind::Minor. Four walkers read that kind and each skips a clean area on a minor:walk_rd_consts_refs, the frontTargetTokenwalk, the method cache scan, and the mapdict scan.collect_roots_in_nurseryderivesuse_jit_frame_stoppers = not any_pinned_object_from_earlierfrom that same flag and hands it towalk_rootsasis_minor. A pinned object created before the previous minor is still in the nursery and was never promoted, so the skip drops the only edge reaching it. The sampled flag now selects the announced kind, which makes all four walkers conservative together rather than patching each gate.The test records the kind each registered walker sees across two minors with a pinned root alive. Without the fix the pair reads
[Minor, Minor].compute_gcmap— the cranelift site actually needed the skip#1372 documented
if arg is None: continueat the three gcmap sites and claimed cranelift could not reach a hole becausespill_guard_fail_argsroutes throughresolve_opref. The assert is real but all three of its call sites arecall_may_force/call_release_gil, andinfer_fail_arg_typestypesOpRef::NONEasRef. Both type resolvers mintReffor a virtual's hole, so a type-only test marks a rootless slot. The skip is now implemented, not just described.Nursery::reset_rangeand the pinnednext_freereset_rangeis a safepub fnthat writes raw memory and upheld its bounds withdebug_assertonly; it now clamps to the nursery extent and returns on an empty range.next_freeis clamped tonext_topbefore the new barrier bounds are published.PyFrame.execution_contextandPyFrame.w_globalsinterp_jit.pylistspycode,valuestackdepthanddebugdataas the virtualizable scalars, andecis a red inreds = ['frame', 'ec'], not a frame field.execution_contextis removed and its readers take the current activation's context asspace.getexecutioncontext()does;normalize_raise_varargs_fnno longer needs a frame pointer for it.w_globalsgoes the same way. Upstreambcd8653e5ecdropped it whenPyCode.frame_stores_globallanded, andInstanceRepr._parse_field_listskips a_virtualizable_name with no concrete field, so the stale list entry produced no scalar.get_w_globalsreads the code object andFrameDebugDatacarries the snapshot the debug path wants.PYFRAME_W_GLOBALS_OFFSETbecomesFRAME_DEBUG_DATA_W_GLOBALS_OFFSET, the vable scalar table loses its fourth entry, andfailed_attr_cleanupnarrows tou8.majit_metainterp::backend_runtimecollectsset_jitframe_gc_type_idandinstall_gc_standalonebehind the same feature selection that constructsBackendImpl, so a frontend does not repeat the test and disagree under workspace feature unification.Smaller review items
finish_trace_for_parity_preserves_captured_snapshotscarries a fullassert_eq!body but had no#[test], so its#[allow(dead_code)]was hiding the fact that it never ran. It runs and passes.may_force_test_lockinpyjitplhas no caller in any cfg — the cranelift backend has its own copy, which is the one everymay_forcetest takes. TheTIDconstant ina_named_field_resolves_by_name_through_an_ambiguous_offsetis unused; that test callsfield_specs_from_layoutdirectly.compile.rs,jitdriver.rs,optimizeopt/{guard,unroll,vstring}.rsandresume.rs, continuing dead_code audit: a _json rooting gap, gcmap comments, per-item lint allows, and the retired SSARepr allocator #1372's audit.opencoder::refresh_from_gcis deliberately kept:rooted_ref_indicesis populated by_encode_ptrand read only byrelease_roots, andwalk_active_trace_refsnever touches_refs, so_refscan retain pre-move addresses until a caller wires it.get_w_globalspromotespycodepyframe.py get_w_globalsends injit.promote(self.pycode).w_globals. The port readself.pycodeunpromoted, which was harmless only whilew_globalswas still a virtualizablescalar. With the field gone the code object is the sole source, so without the promote the
globals read is a load the optimizer cannot fold.
Quasi-immutable ownership and the recursive bridge's setup-abort test
quasiimmut.py QuasiImmutregisters the owningJitCellToken; the registry held oneArc<AtomicBool>per compiled artifact, so an invalidation reached only the fragmentsregistered by then. It now holds
Arc<dyn QuasiImmutLoopToken>.can_inline_callableanddisable_noninlinable_functionare reached through their typed green-key forms.Tracegainsrecorded_ops_total, whichcutdoes not rewind. The bridge driver'ssetup-abort test reads it beside
num_ops, so a body walk whose speculative operations werecut back to the setup position no longer reads as a deterministic setup failure that
permanently declines its source guard.
80 jit-stats baseline files over 27 fixtures move with it.
Two of those re-recorded baselines were regressions, not improvements
raise_reg_unbound_jitstresswentfbw_rolled_back_with_effects0 -> 1. The kept-slot holereport in
collect_outer_active_boxesconsults the walk mirror and the edge-move recoverybut not the virtualizable shadow, which the resolution directly below it already treats as
authoritative — so it declined a case its own resolver handles, at a capture point a residual
had already run past. Restored to fbw 0,
loops_aborted1,loops_compiled9.polymorphic_binary_receiverlost a bridge (3 -> 2,guard_failures788 -> 845).replace_movable_load_global_namespace_with_frame_globalsreturned without substituting thenull
LOAD_GLOBALnamespace placeholder when the inlined callee's frame red was unseeded, sotry_walker_load_global_cell_folddeclined on the null namespace before reaching itsbuiltins leg — which already handles
frame_ptr == 0. The callee's own__globals__is ininline_callee_consts; it is now named directly, immovable namespaces only. Restored tobridges 3,
guard_failures788,loops_aborted0.Review follow-up (#1388)
w_code_frame_stores_globalreadPyCode.w_globalsand stored into itunsynchronized.
pycode.py frame_stores_globaldoes that pair under the GIL; pyre isfree-threaded, so two threads first running one code object in different globals could both
read the null and both answer
false— andfalseis the answer that makes a frame take itsglobals from the code object. With
PyFrame.w_globalsgone the loser's frame then read thewinner's namespace. The publication is now a compare-exchange, answered against whichever
pointer won, matching what
w_code_getconstantalready does forco_consts_w.pyre-wasm-test's launcher calledPyFrame::new_with_contextwithout installing thecontext in the OS-thread locals. With the frame-owned context gone that left
space.getexecutioncontext()null for every program it runs, soeval_frame_plain_with_resumetook its context-free arm — noenter/leave, nocall_trace— andsys.settracewas inert. Fixed, and the enumeration inPyFrame::new'scomment now names all three launchers.
Citations
Twelve line-number citations this branch added are converted to symbols
(
maybe_compile_and_run,compute_gcmap,get_w_globals,PyFrame._virtualizable_,PyPyJitDriver.reds), whichscripts/check-new-line-citations.pynow enforces on PRs.Rebased onto
5bf59e1f008. #1400'sec_seedfallback that readsPyFrame.execution_contextoff the root frame is dropped — that field no longer exists — while its two other consumers
(
sym.execution_contextandargboxes_r[ec_reg]) and its bridge-setup tally are kept.bridge_global_fold_invalidate_hot, which both sides re-recorded, is resolved to the new base'svalues and measured there.
Local gate on the rebased tree: dynasm 456/456, cranelift 456/456, wasm 448/448, zero jit-stats
diffs.