Skip to content

jit: converge the build-time and runtime halves of the descr universe - #831

Closed
youknowone wants to merge 26 commits into
mainfrom
descr-universe
Closed

jit: converge the build-time and runtime halves of the descr universe#831
youknowone wants to merge 26 commits into
mainfrom
descr-universe

Conversation

@youknowone

Copy link
Copy Markdown
Owner

Draft — do not merge. A performance regression in commit 3 of this
stack is reproducible and isolated but not yet root-caused. See
Known regression below.

pyre keeps two halves of one descr universe: a build-time half that
majit-translate serializes into descrs.bin/jitcodes.bin, and a runtime
half the interpreter mints through GcCache. Upstream has one universe —
descr.py:25-47 setup_descrs numbers a single all_descrs list, and
descr.py:218-239 get_field_descr mints every field descr through one
(STRUCT, fieldname) cache. Where the two pyre halves disagree, the descr
bridgeopt.py:155 metainterp_sd.all_descrs[descr_index] resolves is not the
descr the optimizer recorded.

This branch converges them. It stacks on #787.

Commits

  1. census how many build-time Field descrs converge with the get_field_descr cache
    PYRE_FIELD_IDENTITY_CENSUS=1 counts, per keyed field, whether the
    build-time pool descr and the walker's descr are the same Arc as the one
    GcCache::_cache_field holds.

  2. classify the field-descr identity census by miss reason and by reader
    Splits the misses into pool-vs-cache, walker-vs-cache and pool-vs-walker so
    the residual is attributable.

  3. mint every field descr through GcCache::get_field_descr
    Field descrs were being constructed directly in several places, each
    producing a distinct Arc for the same (STRUCT, fieldname). They now all
    go through the cache, keyed as descr.py:218-239 keys
    cache[STRUCT][fieldname] (the dotted
    '%s.%s' % (STRUCT._name, fieldname) at descr.py:227 is the display name
    only). Census on exception_subclass_attrs: pool-vs-_cache_field
    converged 126/323 → 321/323, walker 273/323 → 317/323.

  4. back metainterp_sd.all_descrs with the process-wide descr registry
    pyre had two live MetaInterpStaticData values — the thread-local
    METAINTERP_SD.canonical and the JitDriver's MetaInterp copy — each
    with its own all_descrs, so a descr_index serialized by one was read
    against the other's list. Both now read the process-wide descr_registry.

  5. ignore shrinking take_back_all_descrs write-backs
    descr.py:28 v.descr_index = len(all_descrs); all_descrs.append(v) makes
    all_descrs append-only, so a shorter write-back is never a new universe.
    unroll.rs hands the list to each phase with std::mem::take and restores
    it on the way out; an early exit between the two leaves the outer
    UnrollOptimizer holding an empty vector, which compile_loop then
    published — invalidating every descr_index already baked into a compiled
    bridge (index out of bounds: the len is 0 but the index is 203 in
    deserialize_optimizer_knowledge).

  6. carry EffectInfo raw descr sets across descrs.bin as gccache keys
    The six raw sets of effectinfo.py:128-145 frozenset_or_none
    (_readonly_descrs_fields, _write_descrs_fields, and the array and
    interiorfield pairs) hold Arc<dyn Descr> and were #[serde(skip)], so
    every deserialized call descr came back with them None — the
    EF_RANDOM_EFFECTS wildcard shape, which effectinfo.py:149-162 reserves
    for random-effects calls only. Each member is now serialized as the gccache
    key the analyzer minted it through (DescrSetMember::{Field, Array, InteriorField}) and looked back up at startup, before
    finish_setup_descrs.

    Resolution is lookup only, never mint: minting through a member would
    publish a parent SizeDescr with an empty all_fielddescrs, win
    _cache_field by first-write, and leave the two describing different
    objects — breaking the positional invariant
    heaptracker.py:76-101 get_fielddescr_index_in asserts. A member whose
    container is absent from this process's universe is dropped (no operation
    here can carry a descr for it); a member whose container is present but not
    under that key degrades the whole EffectInfo to the wildcard rather than
    silently narrowing it.

Known regression (blocks merge)

bench/synth/exception_subclass_attrs runs 15x slower on this stack. Output
stays correct; the JIT thrashes instead:

guard failures bridges compiled user+sys CPU
#787 tip (commit 2 of this stack) 42 0 1.21s
commit 3 and later 71654 331 18.33s

Bisected to commit 3 (mint every field descr through GcCache::get_field_descr)
by building each commit and comparing MAJIT_STATS. Under MAJIT_LOG the bad
side fails one guard (guard=1) 52207 + 4223 times across two green keys,
against 40 failures of a different guard on the good side — so the two sides
compile different loops, not the same loop with a weaker guard.

Refuted so far, each by build-and-measure:

  • Not the EffectInfo rehydration — disabling rehydrate_build_descr_raw_sets
    leaves 71654/331 unchanged.
  • Not the take_back_all_descrs monotonicity guard — removing it leaves the
    counts unchanged.
  • Not a flag disagreement on a get_field_descr cache hit. Instrumenting the
    hit path to compare the caller's offset/size/type/immutable/quasi-immutable/
    index_in_parent/virtualizable against the cached descr reports exactly one
    mismatch in the whole run, PyFrame.w_globals: idx_in_parent 4 vs 12.
  • Not that mismatch either. PYFRAME_DESCR_GROUP does list
    "PyFrame.w_globals" twice at the same offset (positions 4 and 12), and
    commit 3 collapses the two onto one cached Arc — but giving the second
    entry a distinct key so they stay separate leaves the counts unchanged.
    (Deleting the entry outright panics, so something does read position 12; that
    duplicate is worth a separate look regardless.)

Verification

  • python3 ./pyre/check.py --backend dynasm: the descr work itself is
    correctness-clean — every failure on this stack is either the pre-existing
    synth/getframe_force_cancel_journal wrong output or a timing gate
    (exception_subclass_attrs from the regression above, plus
    const_arg_call_resume and depth7_inline_chain_typeflip which time out only
    under load).
  • cargo check --workspace --all-targets, cargo fmt --all
  • cargo test -p majit-metainterp -p majit-ir -p majit-translate (the four
    pre-existing test_rbigint_mir failures aside)
  • 94 deserialized EffectInfos carry raw sets: 92 rehydrated, 2 degraded to
    the wildcard. No [s4c-degrade] lines from the build script.
  • PYRE_FIELD_IDENTITY_CENSUS=1 residual is the
    W_ListObject.{strategy,int_items,float_items} inline sub-struct set, which
    needs the jtransform.py:942 rewrite_op_getsubstruct port.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 414a6112-7c64-466d-abe5-20441b4956ad

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch descr-universe

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…GE_ONLY parsing

Review findings from PR #773.

`loop_header_pcs` and the new `loop_header_greens` were inserted on compile but
never removed, so both outlived `compiled_loops` and grew without bound.
`remove_compiled_loop` and the memmgr eviction path in `try_to_free_some_loops`
now drop the retired key from both. `compiled_key_for_greens` already required
`has_compiled_targets`, so a leftover entry could not resolve a bridge onto a
retired loop; this is the growth fix, not a targeting fix.

`MAJIT_BRIDGE_ONLY` parsed its list with `filter_map(.. .ok())`, so a single
unparsable entry was dropped and `MAJIT_BRIDGE_ONLY=oops` yielded an empty
allowlist — which suppresses every bridge, the inverse of the documented
unset-means-all default, with no diagnostic. An unparsable entry now panics.

The two `fieldnums` arity checks in `resume_box_reader.rs` are unconditional
`assert_eq!` rather than `debug_assert_eq!`; release builds indexed past the
end and panicked without the message.

cargo test -p majit-metainterp --features dynasm: 1501 passed, 0 failed.

Assisted-by: Claude
… pool

`JitCodeMachine::run_one_step`'s `BC_NEW` arm allocated every struct with
`std::alloc::alloc_zeroed`, bypassing the GC. blackhole.py:1301-1310
`bhimpl_new` reaches the allocation through `cpu.bh_new(descr)`, so it always
lands in the pool the collector that owns the object manages.

A descr flagged `headerless` says the interpreter owns the struct in its own
collected pool: that is what `headerless_structs` declares, and what compiled
code allocates it from, through `call_malloc_nursery_headerless`. A host-heap
block there is invisible to that collector. aheui's copying collector
range-checks its nursery chunks in `forward_root`, so it neither traces
through such an object nor forwards the references hanging off it, and the
graph below it is left in from-space for the next collection to reuse.

The allocation must not collect. `BC_NEW` runs mid-jitcode with raw object
pointers live in the machine's own register bank -- the `getfield` result that
the `setfield` after the `new` consumes -- and that bank belongs to no root
set; unlike an interpreter-side allocation there is no successor to hand over
as a keep root. `GcAllocator::alloc_nursery_headerless_no_collect` carries
that requirement, defaulting to the collecting form, which is what a
non-moving collector wants.

Non-headerless descrs keep `alloc_zeroed` unchanged and `BC_NEW_WITH_VTABLE`
is untouched. aheui is the only consumer in the tree that declares
`headerless_structs`.

python ./pyre/check.py: dynasm 6 failed / 315 passed, cranelift 6 / 315,
wasm 3 / 315 -- the same failure set, test for test, as the commit this is
built on, confirmed by rerunning it with these three files reverted. The one
difference between the two runs is the measured ratio of the pre-existing
`const_arg_call_resume` perf-gate failure.

Assisted-by: Claude
`clear_compiled_loops`, `mark_all_loops_for_release` and
`invalidate_compiled_trace` removed `compiled_loops` entries without
touching `loop_header_pcs` / `loop_header_greens`. `clear_compiled_loops`
now clears both maps and `mark_all_loops_for_release` routes through it;
`invalidate_compiled_trace` moves to `MetaInterp`, where it drops the
side tables of each removed green key.

`MAJIT_BRIDGE_ONLY` values naming no index (empty, whitespace, bare
commas) produced an empty allowlist that rejected every guard without a
diagnostic. Parsing moves to `parse_bridge_only`, which panics in that
case.

Adds unit tests for the three eviction paths and the four parse cases.

Assisted-by: Claude
The `building_bridge` branch that leaves the export empty instead of
raising InvalidLoop had no trace. Log it under MAJIT_BRIDGE_DEBUG next to
the other `[bridgeB]` lines.

Probed with it: the branch does not fire on the aheui corpus
(logo/99bottles/99dan/quine/pi.jinseo) or on pyre/bench + pyre/extra_tests.

Assisted-by: Claude
`pypyjit_driver_descriptor` left `frame_value_count_fn` at None, so jd0's
`-live-` decode fell back to the process-global slot in
`majit_ir::resumedata`. That slot has two unarbitrated writers — this
crate's `ensure_finish_setup` and majit-metainterp's
`install_state_field_fvc`, each behind its own `Once` — so the last
registration wins, and a decode against the wrong store returns a
mistyped count rather than failing.

Only `ensure_finish_setup` runs today: pyre's jd1 dispatch body does not
lower, so `register_dispatch_jitcode` is skipped and
`install_state_field_fvc` is never reached (measured on
pyre/bench/{nbody,fib_recursive,int_loop} and an unpackiterable drain).

Set the field to `frame_value_count_at`, the same shape jd1 already uses
in `unpackiterable_driver_descriptor`, so `active_frame_value_count_fn`
resolves both drivers off the driver rather than the global.

check.py: dynasm 5/316, cranelift 6/315, wasm 3/315 — the same
correctness failures as HEAD, differing only in the const_arg_call_resume
perf ratio.

Assisted-by: Claude
`MetaInterpStaticData` gains `jitcodes`, the flat table `resume.py:1051`
indexes (`warmspot.py:281-282` installs it there). `register_dispatch_jitcode`
publishes its drained worklist into it through `MetaInterp::install_jitcodes`,
and the two `resolve_jitcode` closures read it, so `JitDriver`'s own
`jitcode_registry` copy is gone.

The portal JitCode moves to `JitDriverStaticData::mainjitcode`, which had no
writer (`call.py:147`), at the driver's own registered slot
(`call.py:46-47 jd.index`). `JitDriver` keeps only that slot index and
`dispatch_jitcode()` reads through it, replacing the driver-local
`Option<Arc<JitCode>>`. `call.py:148`'s back-pointer has no counterpart: the
metainterp-side `JitCode` carries no `jitdriver_sd` slot, only the
translate-side one does.

aheui logo/99bottles/99dan/quine byte-identical between --jit and --no-jit
(logo md5 7fcdbfff0af449c4283c008e3ca317ce); pi.jinseo prefix-identical at
12288 B with 0 FREE/ALLOC-OUTSIDE-CHUNKS; majit-metainterp 1418 passed;
aheui-runtime 18 passed.

Assisted-by: Claude
…ge dead

The preview in `optimize_with_constants_and_inputs_at` exports its virtual
state from `post_force_args` and re-matches that same list, so every
`state[i]` derives from `args[i]` and `make_inputargs_and_virtuals` cannot
raise VirtualStatesCantMatch there. Both arms of the `building_bridge` branch
are therefore unreachable.

Measured: five virtual-carrying fixtures (escaping tuple, escaping instance,
aliased list, varying-length array, nested virtual), two of which compile
bridges, produce zero hits — as do the aheui corpus and pyre/bench +
pyre/extra_tests.

Adds `export_state_re_matched_against_its_own_args_cannot_fail`, which fails
if the preview stops being a self-match. Upstream matches against a different
loop's stored state in `jump_to_existing_trace` (unroll.py:207), so moving to
that shape trips the test and flags the branch as newly live.

Assisted-by: Claude
`Assembler::resuming_build_time_liveness` seeds the runtime codewriter's
`all_liveness` with `jitcode_runtime::all_liveness()`, and
`AssemblerState::new` seeds the reader-side mirror the same way, so a
`publish_state` wholesale replace cannot rewind past the prefix. Every
production `publish_state` caller now publishes a buffer carrying it
(`Assembler::finished`, `encode_liveness_info`); the hand-built-buffer
callers are all `#[cfg(test)]`.

With the build-time bytes addressable from `metainterp_sd.liveness_info`,
`blackhole_resume_via_rd_numb` drops its `novable` pick between the two
pools and reads the one `resume.py:1022` reads, and
`build_time_frame_value_count_at` reads it too — only its jitcode-table
lookup stays per-driver.

The `-live-` operand is 2 bytes, so the pool is capped at 64 KiB; the
build-time prefix is 6952 of those bytes and the overflow assert now
reports both halves.

Adds `assembler_state_resumes_the_build_time_liveness_prefix`: losing the
prefix does not fail loudly, it lands a baked offset inside an unrelated
runtime triple and returns a mistyped value count.

Assisted-by: Claude
`AssemblerState::new` and `Assembler::resuming_build_time_liveness` resumed
the build-time `all_liveness` buffer but started `insns` empty.
`blackhole.py:55-61` recovers `op_live` as `asm.insns['live/']`, so
`MetaInterpStaticData.op_live` stayed at its unset sentinel,
`blackhole_control_opcodes()` returned -1, and `can_decode_live_vars` looked
for 255 as a marker byte and declined every resume against a build-time
jitcode.

Both sides seed from `jitcode_runtime::insns_opname_to_byte()`;
`publish_state` replaces `asm.insns` wholesale, so seeding one side alone
does not hold.

Assisted-by: Claude
…naddrs

The #171 append fold descends `w_list_append` as a sub-jitcode walk, so a
guard exit inside that body is numbered against `w_list_append`'s own jitcode
and resumed there. The resumed body reaches its per-strategy store —
`W_ListObject::object_push`, `IntArray::push`, `FloatArray::push` — each a
`residual_call` whose funcptr the codewriter left as a
`symbolic_fnaddr_for_path` hash, so the blackhole aborted the frame. The jd1
drain then fell back to the interpreter after `next()` had already produced
an item, losing one element per compiled-loop entry
(`bench/synth/unpack_drain_star_raise.py` printed 47941 instead of 48000).

`fnaddr_for_target`'s `CallTarget::Method` fallback keys on
`CallPath::for_impl_method(receiver, name)`, which
`register_macro_helper_trace_fnaddr` derives by stripping the leading crate
segment, hence the `pyre_object::<Type>::<method>` spelling. `object_push`
becomes `pub` because the binding takes its address.

`bhimpl_inline_call_*` calls `cpu.bh_call_*(adr2int(jitcode.fnaddr))`, so the
`w_list_append` and `w_list_len` jitcode shells are bound too.

The symbolic-funcptr decline now names the jitcode and position.

Assisted-by: Claude
…ONLY removal

The rebase onto origin/main takes upstream's deletion of `bridge_only_allows`
/ `parse_bridge_only`; the unit tests for the parser came along with this
branch's hardening commits and no longer name anything.

Assisted-by: Claude
`ensure_finish_setup` runs on every `jitcode_for`, and it built its
arguments by cloning `Assembler.insns` and `Assembler.all_liveness`
whole. Both are now seeded from the build-time tables, so every call
copied the entire opcode and liveness universe and handed it to
`finish_setup_if_needed`, which rewrote the same `op_*` ids and rebuilt
the same `liveness_info` Arc from it.

`assembler.py:29-31` only appends to those two buffers, so equal lengths
mean equal contents. `MetaInterpStaticData` now records the `insns`
length its cached opcode ids were read off, and `ensure_finish_setup`
compares both lengths before taking the snapshot.

bench/synth/depth{2,3}_inline_chain_typeflip run 119873 guard failures
with identical trace structure on both sides, so the copies landed once
per blackhole resume: wall clock was 5.56s/7.38s before this change
against 1.95s/2.54s for the same fixtures at origin/main. After it,
user+sys CPU over depth{2,3,7} (min of 3) is 1.65s/1.85s/3.61s here
against 1.78s/2.47s/4.50s at origin/main.

Assisted-by: Claude
…eld_descr cache

`PYRE_FIELD_IDENTITY_CENSUS=1` walks `all_descrs()` at process exit and reports,
per `BhDescr::Field`, whether the `DescrRef` `make_descr_from_bh` produces is the
same `Arc` `descr.py:218-239 get_field_descr` holds for that
`(STRUCT, fieldname)` key.

`effectinfo.py:465-547 compute_bitstrings` partitions descrs by object identity,
so carrying `EffectInfo`'s raw `_*_descrs_*` sets across `descrs.bin` only means
anything if each rehydrated member lands on the descr the trace itself caches.
Today the census reports 422 Field slots, 325 keyed, 0 converging: the
`_cache_size[key].all_fielddescrs()` list and `_cache_field[key][name]` are
separate mints, and `W_ListObject`'s fields carry dot-qualified build-time names
against bare runtime keys.

Also drops the trailing blank line rustfmt flagged in jitdriver.rs.

Assisted-by: Claude
…eader

The census compared one resolution against `_cache_field` and reported a
single converged count.  Two resolutions exist: `field_descr_ref_from_bh`
(`pyjitpl/dispatch.rs`), which reads `_cache_field[STRUCT][fieldname]` and is
the Arc baked into recorded getfield/setfield ops, and
`field_descr_from_bh_field` (`pyre-jit-trace/src/descr.rs`), which walks
`_cache_size[STRUCT].all_fielddescrs()` and fills the build-time descr pool.
Export the former and report both against `_cache_field`, plus their agreement
with each other and how often the pool Arc came from `all_fielddescrs()`.

Misses are split into `no _cache_size[STRUCT]` / `no _cache_field[STRUCT]` /
`name not in _cache_field[STRUCT]` / `different Arc`, and the samples carry the
owner, `index_in_parent`, the parent's `all_fielddescrs` length and the
`_cache_field` key set.

On `append_hot.py` this reports 422 Field slots, 325 keyed: pool converges 124,
walker 274, pool==walker 124/325, with 42 name misses whose `_cache_field` keys
are inner-struct field names (`block`, `len`) registered under the outer struct
key.

Assisted-by: Claude
`SimpleFieldDescr`, `SimpleFieldDescrSpec` and `BhFieldSpec` gain
`field_key` — `descr.py:227`'s `fieldname` cache key, kept separate from
the display `name` (`'%s.%s' % (STRUCT._name, fieldname)`). The key was
previously recovered by `rsplit_once('.')` on the concatenated name,
which turned `int_items.len` into `len`.

`make_simple_descr_group_keyed_with_headerless` and
`build_object_descr_group_with_def_path` now obtain their fields from
`GcCache::get_field_descr` instead of minting fresh Arcs inside
`Arc::new_cyclic`, and the walker's `field_descr_ref_from_bh` name-miss
branch routes through the same cache-or-mint. `get_field_descr` takes
`index` / `virtualizable`; `SimpleFieldDescr::parent_descr` becomes
interior-mutable so a later `register_keyed_size` can re-point it.
`register_keyed_field` is first-write-wins.

`PyreObjectDescrGroup` carries its own field list instead of indexing
`size_descr.all_fielddescrs()`, which is positional by
`index_in_parent` (`heaptracker.py:76-101 get_fielddescr_index_in`) and
need not agree with the pyre static table's order.

`bh_all_field_specs_for_struct_into` flattens inline sub-structs with
the root owner, a dotted `field_key` and an absolute offset.

Field-identity census on a list-append workload: pool 124/325 -> 320/323
resolving to the `_cache_field` Arc, walker 274/325 -> 319/323.

Assisted-by: Claude
`descr_index` is stamped off the process-global `GcCache`
(`descr.py:28 v.descr_index = len(all_descrs)`), but `all_descrs` was a
per-`MetaInterpStaticData` field. pyre carries two of those objects —
the tracing walker's thread-local one and the one `JitDriver`'s
`MetaInterp` owns — and only the former ran `finish_setup_descrs`, so
the numbering was assigned off a list nothing consumes while the
consumed list stayed empty. `ensure_descr_index` then returned the
already-assigned global index without appending, and
`bridgeopt.py:155 metainterp_sd.all_descrs[descr_index]` indexed a
zero-length vec (`index out of bounds: the len is 0 but the index is 8`
on bridge_branchy_callee, inline_multiframe_drain_journaled_store,
inline_multiframe_module_branch_deopt, fannkuch).

The storage moves to `descr_registry::ALL_DESCRS`;
`MetaInterpStaticData::all_descrs()` is the accessor. Upstream keeps the
list on `metainterp_sd` because there is one `metainterp_sd` built from
one `cpu.setup_descrs()`.

The six optimizer seeds change from `std::mem::take` of the slot to a
clone: emptying it for the duration of an optimize left any reader
inside that window with a zero-length universe.

Assisted-by: Claude
`descr.py:25-47 setup_descrs` numbers `all_descrs` once and
`descr.py:28 v.descr_index = len(all_descrs); all_descrs.append(v)` only
ever appends, so a write-back shorter than the published list is never a
new universe. `unroll.rs` hands the list to each phase with
`std::mem::take` and restores it on the way out; an early exit between
the two leaves the outer `UnrollOptimizer` holding an empty vector, which
`compile_loop` then publishes, invalidating every `descr_index` already
serialized into a compiled bridge (`index out of bounds: the len is 0 but
the index is 203` from `deserialize_optimizer_knowledge` on fannkuch).

Assisted-by: Claude
The six raw sets of `effectinfo.py:128-145 frozenset_or_none`
(`_readonly_descrs_fields`, `_write_descrs_fields` and the array and
interiorfield pairs) hold `Arc<dyn Descr>` and were `#[serde(skip)]`, so
every call descr read back from `descrs.bin` came up with them `None` —
the shape `effectinfo.py:149-162` reserves for `EF_RANDOM_EFFECTS`.
`compute_bitstrings` reads the two shapes oppositely, so a deserialized
concrete EI had its bitstrings cleared instead of classified.

Each member is now serialized as the gccache key the analyzer minted it
through: `DescrSetMember::{Field, Array, InteriorField}` carries the
`(struct_id, field_name)` / `(array_id)` / `(array_id, name)` tuple that
`descr.py:218-239 get_field_descr`, `descr.py:348-378 get_array_descr`
and `descr.py:404-437 get_interiorfield_descr` key their caches on. Both
halves of the split agree on those tuples by construction.

`rehydrate_build_descr_raw_sets` resolves them before
`finish_setup_descrs` and re-derives `single_write_descr_array`
(`effectinfo.py:201-206`, also serde-skipped and read by
`heap.rs force_from_effectinfo`). It first materializes every non-call
pool slot, so each parent publishes its full
`heaptracker.all_fielddescrs(STRUCT)` list before any member is looked
up.

Resolution is lookup-only. Minting through a member would publish a
parent `SizeDescr` with an empty field list and win `_cache_field` by
first-write, breaking the `heaptracker.py:76-101 get_fielddescr_index_in`
positional invariant that `optimizeopt/info.rs force_box` asserts. A
member whose container is absent from this process's descr universe is
dropped — no recorded operation can carry a descr for it; a member whose
container is published but whose key misses degrades the EI to the
wildcard instead.

Measured on the append/loop corpus: 92 EIs rehydrated, 2 degraded.

Assisted-by: Claude
…to the caller's field

`PYFRAME_DESCR_GROUP` named `"PyFrame.w_globals"` twice at
`PYFRAME_W_GLOBALS_OFFSET`, at positions 4 and 12 of the field list, and
`pyframe_w_globals_obj_descr` read position 12. `index_in_parent` is the
position, so the two entries described the same slot under two different
`heaptracker.py:76-101 get_fielddescr_index_in` answers; routing field
descrs through `GcCache::get_field_descr` then collapsed them onto one
cached `Arc` whose `index_in_parent` was whichever minted first. The
duplicate was the last entry, so dropping it shifts nothing; the accessor
moves to position 4.

`descr.py:218-239` derives offset, size, flag, `_immutable_fields_` rank
and `index_in_parent` from `(STRUCT, fieldname)` itself, so a cache hit
upstream cannot describe a different field than the caller means. Pyre
passes them in, so two call sites can disagree and the cache silently
keeps the first mint. `SimpleFieldDescr::describes_same_field` states the
invariant and a `debug_assert` in the cache-hit path enforces it; `index`
is excluded because it is the per-trace codewriter slot id the analyzer
legitimately restamps.

`check.py --backend dynasm` built with `-C debug-assertions=on` reports
no violation over the whole corpus: 2 failed / 329 passed, both failures
pre-existing.

Assisted-by: Claude
…tracer's ref setfield

`runner.rs bh_new` allocated every struct with `libc::malloc`, ignoring the
descr's `type_id` that its `bh_new_with_vtable` sibling already honours. The
two now share `bh_alloc_struct`, which routes a headered GC-managed descr to
the non-moving old generation, a headerless one to the interpreter's own
headerless nursery, and keeps the zeroed malloc for `type_id == 0` and for a
runtime with no allocator hook installed.

`pyjitpl/dispatch.rs BC_NEW` took the same shape one layer up: only the
headerless case reached the GC, and everything else went to
`std::alloc::alloc_zeroed`. A headered GC-managed descr now allocates in the
old generation there too; both GC paths are no-collect and old-gen is
mark-sweep, so the pointer the tracer keeps in its register bank stays valid.

`BC_SETFIELD_GC_R` wrote the field with a raw store and no write barrier,
unlike the `BC_SETARRAYITEM_GC_R` arm next to it and unlike
`bh_setfield_gc_r`. It now notifies the GC on the container.

`BhDescr::is_headerless` replaces the `owner == "__majit_headerless_size__"`
comparison open-coded in `jitcode/assembler.rs` and twice in `dispatch.rs`;
the marker constant moves next to the enum it tags.

Assisted-by: Claude
…alizer

`install_global_build_descr_pool` materialized the whole pool — one clone per
`BhDescr` in the binary, each call descr carrying its `EffectInfo` raw descr
sets, plus a `JitCode::from_canonical` per jitcode entry — and then handed it
to `OnceLock::set`, which drops it once a pool is installed.

`drive_unpack_iterable_trace` calls it before every
`_unpackiterable_unknown_length` walk, so on an unpack-heavy program that
build-and-drop dominated: on `bench/synth/exception_subclass_attrs.py` a
`sample` run put 233 of 2465 main-thread samples in
`install_global_build_descr_pool`, 128 of them in the `Arc<JitCode>` drop of
the discarded pool. Measured CPU (user+sys, min of 3) goes 7.26s -> 4.48s.

`set_global_build_descr_pool(pool)` becomes
`init_global_build_descr_pool(build)`, which runs the closure from inside
`OnceLock::get_or_init`.

Assisted-by: Claude
…oder

`build_object_descr_group_with_def_path` used to build a `PyreSizeDescr`,
whose `w_class_obj` reads `get_instantiate(vtable)` live. Routing it through
`make_simple_descr_group_keyed_with_headerless` made every runtime PyObject
group a `SimpleSizeDescr`, which inherits the trait default `None`.

`OptVirtualize`'s `w_class` getfield arm (virtualize.rs:860-894) folds the
header read off a `new_with_vtable` virtual to that constant, and takes the
"class identity unresolved -> force the virtual" exit when it is `None`. The
forced `W_IntObject` then reads `w_class` out of its own freshly allocated,
uninitialised memory and guards on the `PtrEq`, so the guard fails on most
iterations. On `bench/synth/exception_subclass_attrs.py`: guard failures
71654, bridges 331, CPU 4.5s.

`SimpleSizeDescr::w_class_obj` now goes through
`majit_ir::descr::set_w_class_obj_resolver`, which pyre registers in
`install_jit_call_bridge` alongside the `str`/`unicode` green resolvers, and
`PyreSizeDescr::w_class_obj` calls the same decoder. The hook also covers the
size descrs `size_descr_ref_from_bh` mints inside majit-metainterp, which
could not carry a pyre override at all.

Same corpus entry after: guard failures 42, bridges 0, CPU 0.65s — equal to
the branch base on all three.

Assisted-by: Claude
…id not

`orthodox_list_append_commit` ended with an unconditional `w_list_append` on
the premise that the descended sub-walk records the store as IR without
touching the concrete list. The per-strategy store the arm reaches
(`W_ListObject::object_push`, `IntArray::push`, `FloatArray::push`) is a
`residual_call`, and `try_execute_residual_call_via_executor` executes a
residual whose funcptr resolves to a real address rather than only recording
it, so on a target where the arm keeps them as residuals the sub-walk has
already appended and the fold appends the value a second time.

Re-read the receiver's length and append only when it is unchanged. The rewind
journal entry stays unconditional, so an aborted walk rewinds to `len_before`
whichever side grew the list.

Assisted-by: Claude
…the parent group

`make_descr_from_bh` bridged the codewriter's `W_ListObject` field names to the
canonical `W_LIST_DESCR_GROUP` entries only after the parent-struct lookup, so
whenever the codewriter modeled the parent the field ended up with two descrs:
the parent group's entry for a codewriter-lowered body and the canonical entry
for the walker-native list specializations. `MAJIT_LOG` shows both for
`int_items.len` at offset 48 — index 5 (`index_in_parent` 5) and index
268436224 (`index_in_parent` 3).

The heapcache and the optimizer's heap pass key on descr identity, so the
`w_list_append` sub-walk's `SetfieldGc(int_items.len)` did not invalidate the
`len(xs)` read that followed it, and the read folded to the pre-append length:
one skipped `list.pop(0)` in the first compiled iteration, after which the
steady-state length stays one too high.

Run the bridge before the parent-group lookup. Fixes `list_ops`,
`delete_negative_open_slice_hot`, `exception_residual_raise_caught_in_frame`,
`sre_pattern_methods` on all three backends and wasm
`comprehension_object_append_hot`'s output.

Assisted-by: Claude
…d wasm backends

`register_active_hooks` installed `alloc_nursery_typed` but left
`alloc_nursery_headerless_no_collect` unset on these two backends, so
`majit_gc::alloc_nursery_headerless_no_collect` returned `GcRef(0)` and the
jitcode tracer's `NEW` on a `headerless` descr (`pyjitpl/dispatch.rs` BC_NEW)
fell through to `std::alloc::alloc_zeroed` on the host heap, where the
interpreter's collector cannot see it. The dynasm backend already registers it
(`runner.rs`).

Assisted-by: Claude
…ery bump

The arm called `gc_alloc_nursery_headerless_shim` out of line for every
allocation, spilling the ref roots and installing a gcmap each time. Both
dynasm backends already emit an inline bump for this opcode
(`genop_call_malloc_nursery_headerless`), and the cranelift `CallMallocNursery`
arm right below already emits one for the headered case.

Emit the same shape here, with the headerless deltas: bump by `size` alone (no
`GcHeader::SIZE` reservation), no header word zeroed, result is the old nursery
base. The slow path keeps the existing shim call with its spill / gcmap /
reload. A runtime reporting no bump surface (`nursery_free` / `nursery_top` at
0) stays on the helper.

aheui logo under cranelift, CPU time, 16 interleaved rounds over the runs that
produce the reference output: min 20.66s -> 9.08s, median 21.47s -> 11.18s.
`pyre/check.py --backend cranelift` 334/334; pyre declares no
`headerless_structs`, so the opcode does not occur there.

Assisted-by: Claude
@youknowone
youknowone force-pushed the aheui branch 2 times, most recently from 88992d0 to 1806815 Compare July 28, 2026 09:26
Base automatically changed from aheui to main July 28, 2026 09:26
@youknowone youknowone closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant