Merge train 207: hit-path wave 2, GC relocation and weak-holder facts, module path memo, globalThis new, Windows link (v0.5.1585) - #10398
Merged
Merged
Conversation
Base: 6c9e2a6 (PR #10371's head), which this stacks on. `layout_transfer` runs for every evacuated object on every copying minor, both old-generation evacuations and `js_array_grow`. All four callers copy the source header's `_reserved` into the destination first — the minor through `reserved_with_copied_survival_age`, which rewrites only the age bits — so every layout fact a header carries has already arrived: the layout state, `GC_LAYOUT_ALL_POINTERS`, the raw-f64 / holes flags, `GC_ARRAY_ELEMENT_SHAPE` and `GC_OBJ_TYPED_LAYOUT_INTACT`. The funnel re-derived that half anyway, per object: two header classifications, a rewrite of bits that were already equal, an out-of-line call per array and per object, the #7510 flag-and-filter gate evaluated twice, and — for every intact object — a ShapeId-keyed `SHAPE_LAYOUTS` probe whose answer a relocation cannot change. Measured with gdb `stepi` on the #10362 retained-graph fixture: 160 instructions per moved array, 245 per moved object, 518M in total (4.2% of the run), none of which reached a side-table record. Both per-object maps held one key (`PERRY_LAYOUT_DIAG`). The contract the callers always satisfied is now the funnel's stated contract, asserted in test and debug builds, and the funnel moves only what a header cannot carry: the element-shape record (#7480), the residual static-prototype owner registry (#9304), and the per-object `TYPED_LAYOUTS` / `LAYOUT_SLOT_MASKS` entries (#7510). Each is gated inline by the bit or latch that governs it, and the record moves themselves live in a `#[cold]` slow path. * `gc/layout/transfer.rs` (new): the funnel, its gates and the contract assertion. `gc/layout.rs` drops to 1919 lines, off the 2000-line cap. * `gc/layout_tables.rs`: `per_object_layouts_may_hold_either` answers the #7510 gate for both maps and both addresses in one hot-slot resolution. * `object/prototype_chain.rs`: the registry latch is readable without the call. * `array/header.rs`, `array/element_shape.rs`: `transfer_array_numeric_layout` and `clear_element_shape_ptr` were only ever called by the header half of the funnel and are deleted. That also drops a spurious `invalidate_representation_change` a hole-tolerant array took on every move (the transfer cleared the flag it was about to set again); the counter feeds `PERRY_TYPED_FEEDBACK_TRACE` output only. Behaviour: one change, the lazy intact downgrade. The bit is a fact of the object and of two tables a move does not touch, so re-asking at move time could only downgrade objects that happen to move. The state it cleared — intact while no descriptor is reachable — is legal and handled: `shape_install_shared` poisons a shape's shared entry and leaves "any still-INTACT siblings" to fall back, #8115 clears the bit at the first contradicting store, the trace falls back to `GC_LAYOUT_UNKNOWN` and scans every slot, and the query helpers answer "no descriptor". An unmoved sibling keeps its bit today, so nothing could have depended on the move clearing it. `test_poisoned_shape_intact_and_per_object_record_survive_a_copying_minor` builds that exact state and drives it through a real copying minor: the moved receiver keeps the bit, answers every query as it did before the move, and its child survives and is rewritten, while the sibling that poisoned the shape keeps its per-object record across the move. It fails on the parent commit (the old funnel clears the bit) and fails again under a sabotaged funnel that does not move the per-object records. `test_layout_transfer_requires_the_relocation_header_copy` pins the contract. instructions:u, min of 5, base 6c9e2a6 vs this: gc3 12,275,577,365 -> 11,880,548,143 -3.22% w1000 1,081,647,372 -> 1,054,559,901 -2.50% w5000 2,018,014,139 -> 1,919,532,121 -4.88% w20000 4,994,632,943 -> 4,797,474,484 -3.95% oldyoung 1,503,368,405 -> 1,471,395,073 -2.13% alloc-only 320,266,214 -> 320,265,756 0.00% That is 154 instructions per relocation on gc3 (2,560,042 relocations on both arms, counted with uprobes), and 71% of the ceiling a full knockout of the funnel measured. The cold path is entered 1,213 times, 0.05% of relocations.
object_alloc_class_inline_keys_impl calls register_class whenever parent_class_id != 0, and codegen ALSO emits one js_register_class_parent per inheriting class in the init prelude, so by the time user code allocates, the edge is always already published. Re-publishing it bumped the process-global prop_plan epoch -- discarding every cached store plan in the program -- then took a write lock on CLASS_REGISTRY and re-inserted the same pair. prop_plan_epoch_bump's own contract says its callers are rare cold paths by construction; an allocation is not one. An unchanged edge now answers from the dense parent mirror, the same indexed load every chain walk already uses, and returns. A new or CHANGED edge falls through to the full publication, so re-parenting still flushes -- test_gap_subclass_alloc_registration pins that, and the same test covers a re-parent through Object.setPrototypeOf and a second class sharing the parent. This carries NO measured win, and that is deliberate to state. Every allocation shape I could build either takes the inline allocator -- which never calls register_class, so the fixture is vacuous -- or measures the same in both arms: a subclass allocated through the dynamic-class entry is 7,298 instructions before and 7,296 after, and the child-minus-parentless difference is +3,362 before and +3,370 after. The work removed is real at the source level; what it is worth in a running program is unmeasured here.
array_subclass_fast_pop_validated bumped the process-global prop_plan epoch on every pop of an Array subclass, discarding every cached store plan in the program. The bump sits right after clear_packed_subclass_numeric_proof, which is idempotent: a pop loop retires a proof on its FIRST iteration and nothing afterwards, so every later pop paid a program-wide invalidation for a change that did not happen. The retire now reports whether it actually retired one, and only that answer flushes. The shape-version install below it needs no bump of its own: the sibling push path (array_subclass_fast_push_one_validated) performs the same install_cache_carried_object_shape_version and has never bumped, and a per-object shape version is not an input to the store-plan verdict, which is keyed on (class_id, interned key) and invalidated by vtable mutation, descriptor/prototype changes and GC. Unit test pins the contract in both directions: the first retire reports true, later ones report false, and retiring nothing leaves the epoch where it was. The gap fixture interleaves pops with stores through the same plans, adds a prototype setter mid-loop (the change the flush exists to expose), freezes a receiver after pops, and mixes element kinds.
A class-typed parameter is validated by walking every declared field on its inheritance chain by name. Measured at ~326 instructions per field, so a 3-field class pays ~1,000 per call and an 8-field class ~2,600. For a field declared `number` the walk re-derives what the object header already states. `expr/class_field_inline_guard.rs` relies on the same implication to skip its guard call: "intact bit set + class_id/keys match" implies "slot K is raw-f64". So when EVERY field on the chain is a raw-f64 candidate, (class chain reaches C, GC_OBJ_TYPED_LAYOUT_INTACT) carries the whole proof, and the descriptor emits OP_CLASS_NOMINAL instead: two header facts, no field names serialized at all. One non-numeric field puts the whole chain back on the walk. The intact bit is a raw-f64 claim; it says a string field's slot is in the POINTER mask, which is not "it holds a string" — and a clone that inlines `s.length` trusts exactly that. A fieldless class is excluded too: it has no value fact to carry, so demanding the bit could only reject receivers the walk accepts. Instructions per call, both arms re-run in the same window against base1579 (dynamic-dispatch driver, 2e6 calls, best of 3): 1 number field 5,309 -> 4,952 -6.7% 3 number fields 6,174 -> 5,436 -12.0% 8 number fields 7,808 -> 5,838 -25.2% string + number 6,822 -> 6,749 -1.1% (control: stays on the walk) The fast route is proven entered rather than inferred: three receivers with identical field values and identical output cost 5,571 (class-allocated), 8,646 (Object.create(C.prototype)) and 12,507 (a real instance whose intact bit a string store retired) instructions per call. A guard that rejected everything could not produce that spread. Also corrects the #8099 note that identity alone "bought nothing". That verdict is real but local to tree/tree_wide, whose reference-typed fields route both bodies through js_typed_feedback_class_field_get_guard. Codegen never reads these field nodes: the clone is compiled with SpecParamGuard::proof, which is `param.ty`, and forcing `fields` empty leaves all 24 emitted clone bodies across a 16-function probe set unchanged. The descriptor is the runtime ENFORCEMENT of the proof, not the proof.
`declaration_guards` refused a descriptor whose validation work grows with
the input — unless the body contained a loop, on the theory that array
reducers and similar consumers amortize validation over their own traversal.
They do not. The walk is a SECOND full pass over the same array, and the
clone's saving per element is smaller than the walk's cost per element, so
the guarded arm loses at every length and loses by MORE the longer the array
gets — the opposite of what amortization predicts, which is why no array
length rescues the rule.
Instructions per call against base1579, both arms re-run in one window
(dynamic-dispatch driver, best of 3):
1600 elements Pt[] 1,539,568 -> 410,171 -73.4%
string[] 604,460 -> 477,884 -20.9%
16 elements Pt[] 19,191 -> 7,624 -60.3%
string[] 10,308 -> 8,737 -15.2%
Controls, same window, same binaries — the identical bodies taking an
unproven parameter, which never had a descriptor to lose:
1600 elements Pt[] via any 555,495 -> 551,116 -0.8%
string[] 531,291 -> 529,172 -0.4%
Refusing is the win: the fallback is the generic body, and a refused
parameter still keeps its declared type, so it lands BELOW the `any` twin
rather than at it.
There is no O(1) substitute to reach for instead. A raw-f64 layout flag could
settle `number[]`, but that case never had a guard to speed up — wave 1's
`spec_clone_consumes_no_proof` already drops it, because an index loop over
a number array lowers identically with and without the proof. The cases that
still carried a walk were `string[]` and `C[]`, and no header bit claims
"every element is a string".
The body is no longer an input to the decision, so `declaration_guards` no
longer takes one and `body_contains_loop` is deleted. That makes the old
behavior unexpressible rather than merely untested.
A call site packing trailing arguments into a rest or `arguments` bundle emitted `js_array_alloc` plus one `js_array_push_f64` per element. Every push re-classified the receiver, re-resolved forwarding, re-noted the slot layout and re-checked the barrier — for a three-element bundle, 857 instructions of construction for numbers and 1,839 for objects. An array literal of the same width has been built inline since #5391: one bump allocation, a header that already claims pointer-free (and raw-f64 when every element is a plain double), then N stores. A bundle is the same shape with its values already lowered, so it now uses the same emitter, extracted as `emit_array_from_lowered_values`. Rooting is unchanged and still load-bearing (#7154): every element is re-read from the group's slots before the allocation, whose slow arm collects, and the finished array is adopted into the same scope so a second bundle's allocation cannot sweep the first. Bundles wider than the inline threshold keep the push loop. Per call at a static call site: `f(1, 2, 3)` into `...xs` 857 -> 92, `f(o, o, o)` 1,839 -> 494.
`Array.prototype.map` filling a plain result array paid the ownership and forwarding proof of its own receiver three times per element: `clean_arr_ptr` inside the raw-f64 canonicalization, an `addr_class::try_read_gc_header` inside the layout-note elision check, and a third header read inside the numeric-layout note. The caller has just re-derived the live head from its root for this iteration, so one read answers all three. `fill_resolved_array_slot` keeps the protocol `note_array_slot_layout_only` runs — canonicalize under a raw-f64 layout, store, retire the numeric claim on a non-number, note the slot layout unless that note is provably a no-op, and keep the born-old remembered-set edge — and falls back to the fully re-classifying helper for any head it cannot prove from that one read (unrecognized, or forwarded). `a.map(v => v + 1)` over 16 elements: 6,265 -> 4,234 instructions. The fixture covers both this and the rest-bundle change: element kinds, holes, -0/NaN, object identity, a callback that mutates and grows its source, a result longer than the 64-element branch, frozen and subclass receivers, and `arguments`. It matches node 26.5.1 normally and under the seeded moving-GC stress (4,028 copying minors on the object path).
…being on `js_typed_feedback_numeric_array_push_guard` is called on every `a.push(v)` that takes the guarded numeric tier. It built an `Observation` — a `gc_header_for_user_addr` lookup, a length read, a `classify_array` walk over the receiver's element layout and a `stable_value_kind` — and handed it to `guard_observe`, which throws it away and returns `contract_valid` unchanged whenever typed-feedback recording is off. Recording is off by default, so that was the whole cost of the call. This is #5094's gate. Every sibling array guard already carries it (`plain_array_index_get_guard_impl`, the four packed loop guards, both index set guards, `js_typed_feedback_array_get_f64`); the push guard and two declared-but-unemitted wrappers were the last ones that did not. The gated branch returns exactly what `guard_observe` would have returned in that mode, so the observing path and the recorded feedback are untouched. Measured on the dynamic-dispatch census, best-of-5 over 200k calls, minus the zero-iteration run and the same-arity identity baseline: arrPushPop 970.3 -> 861.3 -109.0 (-11.2%) No other probe moved: arrSet +0.3, arrLen -1.4, arrSumForOf +0.3, arrSumIndex +0.8, anyArrGet -1.6, objArrFieldGet +0.7, f64Get -0.5. `typed_feedback_enabled()` is hardcoded `true` under `#[cfg(test)]`, so the runtime unit tests only ever take the observing path and cannot cover the new branch. `test_gap_numeric_push_guarded.ts` covers it end to end instead, where recording is off: it drives every receiver shape the guard declines — frozen, sealed, non-extensible, non-writable length, an index accessor, sparse, a subclass, a Proxy, a mid-program `Array.prototype` index setter — plus a growing dense array and a TypedArray/Buffer receiver, and matches node both normally and under GC stress (8 copying minors, 6720 objects moved, from-space quarantine armed, evacuation verified). Two cases assert resulting state rather than a throw, both pre-existing gaps that behave identically before this change: Perry does not throw when pushing to a non-extensible array, and `map` does not preserve a subclass receiver.
…eceiver The packed-numeric fast clone re-derived the element base on every iteration: reload the rooted slot through the `asm "", "=r,0"` launder, mask the handle, load `size` at `-4` and `capacity` at `+4`, shift, add, subtract — about twenty instructions to reach one `load double`. The launder is opaque to LLVM by design, so LICM could not hoist any of it even though none of it varies. The clone already publishes a pre-masked receiver handle for exactly this: the poll-refreshed receiver cache `acc_scope.hoist_receivers` installs, which the packed STORE path (`expr/index_set_packed_loop.rs`) has read through `receiver_descriptor_handle_i64` since it was added. The read path was simply never converted. It is now, and the element-base chain hangs off a plain `i64` alloca nothing in the clone writes, so LLVM hoists it into the preheader. Soundness is #9379's, not a new claim: the matcher admits no call, closure or await; reads and writes lower to bare `double` load/store on existing slots, so no growth, no realloc and no barrier; and the back-edge poll is suppressed for this clone for exactly that reason. With no safepoint the receiver cannot move and its header words cannot change for the clone's whole dynamic extent. The fact is dematerialized before the slow clone is lowered, so nothing leaks past the clone it was proved for, and a receiver with no hoisted cache still gets the inline bitcast-and-mask from the same helper. Measured on the dynamic-dispatch census, best-of-5 over 200k calls, minus the zero-iteration run and the same-arity identity baseline (16-element arrays): arrSumIndex 659.7 -> 584.4 -75.3 (-11.4%) arrSumForOf 892.9 -> 845.7 -47.2 ( -5.3%) No other probe moved: arrSet -1.2, arrLen +0.2, arrPushPop -0.5, anyArrGet +0.2, objArrFieldGet -0.5, f64Get +0.8, arrMapInc -11. `test_gap_packed_loop_cached_receiver.ts` covers every shape that leaves the clone — a side exit on a non-numeric element, a hole read, a foreign index, a receiver grown during the loop, an allocating body that puts a real safepoint back, plus frozen, subclass and typed-array receivers — through both the indexed and the `for…of` form. It matches node normally, under GC stress (136 copying minors, objects moved, from-space quarantine armed, evacuation verified) and at `PERRY_GC_SCHEDULE_RATE=1` with quarantine depth 64. `cargo test --release -p perry-codegen --tests`: 36 suites, 0 failures. Root-dominance corpus: 196 modules, 15430 root stores, exactly the 2 known `test_gap_gc_regexp_receiver_rooting` violations, 0 moving-minor reachable.
`for (const v of a)` over a packed numeric array paid an incremental-mark root-shading test on every element. `const v = a[i]` is an array alias, so `enable_persistent_shadow_slot_for_array_alias` gives it a persistent shadow slot, and the only per-store cost of such a slot is `emit_persistent_shadow_root_barrier` — an atomic load of `PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT`, a compare and a branch, plus the block split, once per element. `expr_is_known_non_pointer_shadow_value` exists to skip exactly that for a value that cannot be a heap reference, and it already admits a masked-window element read on this reasoning. The packed-numeric loop fact is the same class of proof and was simply not listed: the entry guard proved a dense raw-f64 (or packed i32/u32) plain Array, the clone it scopes has no safepoint and no growth (#9379), and the fast condition bounds the counter by the length read at loop entry — so `arr[i]` reads a raw numeric word and shading it is a no-op. The fact is dematerialized before the slow clone lowers, so this never leaks past the clone it was proved for. Restricted to offset 0. `arr[i ± c]` is in bounds only under a range-validated fact, and an out-of-bounds element read consults the prototype chain, where `Array.prototype[7] = {}` is a genuine heap reference that must stay rooted. The counter read cannot leave the array; the offset read can, and keeps its barrier. Measured on the dynamic-dispatch census with both arms re-run in the same window (best-of-5 over 200k calls, minus the zero-iteration run and the same-arity identity baseline, 16-element arrays): arrSumForOf 895.0 -> 813.9 -81.1 (-9.1%) of which -31.5 is this change and the rest the element-base hoist it stacks on. `arrSumIndex` is unaffected (-76.8, the hoist alone) because an indexed loop binds no element local. No probe regressed: largest increase +6.1 (strTemplate, untouched), and the five largest are +4.8..+6.1 — the noise floor. `packed_loop_shadow_barrier_tests.rs` pins both directions in emitted IR, and each fails without this change (1 shading test where 0 is required): the counter read's binding shades nothing in `for.packed_f64_fast.body`, the offset read's binding still shades exactly once in the `packed_f64_loop.foreign.inbounds` block its bounds check creates, and a third test puts both in one clone so neither arm can pass vacuously. Every test panics if its block was not emitted, so a count cannot be taken over a clone that never ran. `test_gap_packed_loop_proto_index_rooting.ts` is the end-to-end half: it installs an object at `Array.prototype[7]`, has an `a[i + 3]` loop over a length-5 array read it out of bounds, retains that capture, then churns the nursery for 60 rounds re-running both loops and asserts the object's identity and payload survive. It matches node normally and under GC stress with `PERRY_GC_FROMSPACE_SCAN_ABORT=1`: seed=37 rate=0.2 2524 from-space scans, all clean, dangling=0, missing_rewrites=0; 5048 copying minors, max 6720 objects moved; live set up to 33170 objects / 160486 words seed=91 rate=1.0 12694 from-space scans, all clean, dangling=0, missing_rewrites=0 `cargo test --release -p perry-codegen --tests`: 36 suites, 0 failures. Root-dominance corpus: 168/168 sources, 196 modules, 15430 root stores, 0 violations on the `--moving-only` CI arm with 40/40 seeded violations caught.
`s[i]` walked js_string_index_get_boxed -> js_string_index_get -> js_string_char_at -> ascii_char_string: a thread-local canonical-table lookup returning a heap StringHeader that the caller immediately NaN-boxed, and, for a short-string receiver, a full materialization of the receiver onto the heap first just to index it. An ASCII receiver's character is one byte, which is exactly a short-string value, so both ends pack inline: 272 -> 223 instructions per read on a heap receiver and 158 -> 109 on a short one. The value is unchanged — a short and a heap string with the same bytes compare equal everywhere — and two equal characters now share one bit pattern instead of one pointer.
ordinary_has_property asked object_static_prototype up front, spending a
shape/registry probe on every [[HasProperty]] — including the common walk that
finds an own key on the first hop and returns from the loop. Its only consumer
is the class-vtable fallback reached after the whole walk misses, and the walk
runs no user code, so the answer cannot change in between.
"a" in {a,b}: 957 -> 905 per call; a 40-key own hit 970 -> 917; a miss
4,773 -> 4,746.
The template desugaring wraps every substitution in StringCoerce so it is toString-first rather than +'s valueOf-first (#6078), but js_string_concat_chain formats each part itself. For a part already proven a string the wrapper is the identity, and the coerce only mints an intermediate heap string for the helper to copy and drop. `${s}:${n}` 1,437 -> 1,411 instructions per call (one of its two js_string_coerce calls is gone). The number substitution keeps its wrapper: a declared-number parameter is not provably non-pointer — an annotation can lie — and String(obj) and the helper's slow path can disagree for an object with both valueOf and toString.
`in` was the last common operator with no cache slot at all: `"k" in o` lowered to a bare `js_in_operator` call that re-derived the receiver's keys array from its ShapeId (a shape-slab probe) and re-scanned it, every time. That is 955 instructions for an own-key hit and 980 for one on a 40-key object, against ~15 for reading the same key through the property PIC. The answer is a property of the SHAPE, not of the object — two objects with the same ShapeId have the same keys array — so a site with a literal key caches one ShapeId and answers `true` inline when the receiver still carries it. Everything else calls `js_in_operator_presence_ic`, which computes the real answer (including the TypeError a primitive right operand owes) and may arm the site; the inline path can only ever produce `true`. Only positives are cached, and they need no prototype-chain epoch: the cached claim is about an OWN key, so `Object.setPrototypeOf`, a late `Proto.x = 1` and a `delete Proto.x` cannot falsify it. A negative would be a claim about the whole chain and there is no chain epoch in this runtime to key one on, so `"zz" in o` still calls the runtime every time. Invalidation of a positive needs only that losing the key moves the receiver off the guard: a compacting delete publishes a new ShapeId, and a tombstoning delete (#9064) keeps the ShapeId but sets OBJ_FLAG_STABLE_TOMBSTONES, which the guard rejects. Shape ids are allocated monotonically and never reused, and their range is disjoint from every class id, so a stale stamp can only miss. The cache holds two integers and no heap pointer, so it is not a GC root. Instructions per call, base v0.5.1579 vs this branch, both arms rebuilt and re-measured in one window (3M iterations, best of 3, minus a zero-iteration run; controls identity 177.0 -> 177.1, id2 149.1 -> 145.9): "a" in {a,b} 967 -> 41 "k39" in <40 keys> 980 -> 41 "zz" in {a,b} 4,783 -> 4,668 (miss: answer is not cacheable) "toString" in o 2,377 -> 2,351 (inherited: site declines after 8 tries) test_gap_in_operator_presence_cache.ts proves the invalidation against a warm cache: delete, re-add, 500 delete/re-add cycles, a prototype swapped for another and for null, a key appearing and disappearing on the prototype, an own key deleted so the prototype shows through, eight shapes through one site, descriptors, accessors, a Proxy `has` trap that answers false for a key the target has, and a delete performed inside the hot loop. It matches Node under `PERRY_GC_FROMSPACE_SCAN_ABORT=1` with seed 37: 3,932 copying minors, 3,932 clean from-space scans, dangling=0, missing_rewrites=0.
`o instanceof C` answers a miss by falling through a ladder of built-in
probes. Two steps into that ladder sat this pair:
let candidate_proto = class_decl_prototype_object(cur);
let target_proto = class_decl_prototype_object(class_id);
if !candidate_proto.is_null() && !target_proto.is_null()
&& object_has_user_prototype_override(candidate_proto) && ...
Both are class-registry reads — thread-local + RwLock + map — and both ran
eagerly, on every call that reached the ladder, which is every MISS. They
exist for one case: `util.inherits(Derived, Base)`, which re-points a
prototype without creating an extends edge between the constructors. The
question they set up to ask, `object_has_user_prototype_override`, is cheap:
two dependent loads off the receiver's meta record. The expensive half was
only there to find an object to ask it about.
`OBJECT_META_FLAG_USER_PROTO_OVERRIDE` is set at exactly one site, so a
process-wide latch stored just before it answers for every receiver at once.
A program that never re-points a prototype — which is nearly all of them —
now pays one acquire load instead of two registry probes. Set, never cleared,
and published before the flag it guards (the discipline `OBJECT_PROTOTYPES_-
NONEMPTY` above it already uses), so it is conservative in the safe
direction: a false positive costs a probe pair, a false negative is
impossible.
Instructions per call, base v0.5.1579 vs this branch, both arms rebuilt with
identical flags and measured in one window (3M iterations, best of 3, minus a
zero-iteration run; control `idle` 11.0 -> 11.0):
a instanceof B (miss) 669 -> 498
c3 instanceof B (miss, 4 deep) 1,062 -> 891
a instanceof A (hit) 74 -> 74
c3 instanceof A (hit, 4 deep) 270 -> 270
map/error/plain-object misses flat
No class-id band test is involved, which is the better outcome: the
"is this a user class id" question is not on the path at all. (For the
record, it would have been sound — all 87 reserved class-id constants fall
inside the two documented bands, 4 in 0x7FFF_FF00..=0x7FFF_FFFF and 83 at or
above 0xFFFF_0000, while user ids are a dense sequence from 1.)
test_gap_instanceof_miss_ladder.ts covers the latch's own hazard — a
`setPrototypeOf` performed AFTER the sites are hot, and `util.inherits` with
a method resolved through the linked prototype — plus `Symbol.hasInstance` in
both its static-method and defineProperty forms, a Proxy, a bound
constructor, Map/Error/Promise/Array/Function/Object, subclasses of Map and
Error, a 4-level chain, structurally identical twins, null-prototype
receivers, primitives, and a non-callable right operand. Matches Node under
`PERRY_GC_FROMSPACE_SCAN_ABORT=1` with seed 37: 2,182 copying minors, 4,364
clean from-space scans, dangling=0, missing_rewrites=0.
The fixture deliberately does not assert five behaviours it found to diverge
from Node on this commit's PARENT; each carries a comment saying so, and they
are reported separately rather than fixed here.
std::fs::canonicalize is a full realpath - a readlink per path component, every time - and module registration called it once per module. Sibling modules share every ancestor, so opencode --version issued 86,745 readlink calls over 9,467 distinct paths: 98.7% of every syscall it made, and 0.32s of system time. Directories are resolved once and reused, taking a 400-module fixture from 4,010 readlink calls to 405. Paths with . or .. components, and basenames that really are symlinks, still go through std::fs::canonicalize. This is a wall-clock fix, not an instruction-count one: it moves instructions:u by 0.04%.
… binding shadows X (#10359) `globalThis.X` names the global object's property, never a module binding, but every arm that lowered the qualified construct by NAME resolved it against the module's bindings. With `import { Event } from "./ev"` in scope, `new globalThis.Event("ping")` built the imported class, while the aliased `const E = globalThis.Event; new E()` form was correct. Four by-name paths, all now back off when a binding shares the name: - the #6726 re-dispatch through the bare-identifier arm ignored the shadow only for the dedicated intrinsic nodes (SetNew, ErrorNew, ...); names with none (Event, Request, MessageChannel, a multi-argument typed array) reached the by-name tail (`New { class_name }` / FuncRef / LocalGet). The tail now builds `NewDynamic` over the global property instead; - `lower_new_member_native`'s globalThis fetch and MessageChannel arms; - `lower_new_non_ident`'s global-object fetch arm (`const g = globalThis; new g.Headers()`); - codegen's `try_static_class_name` folded a `globalThis.X` callee onto a same-named module class, import or class alias (`class Widget {}` plus `globalThis.Widget = class {...}` built the module class).
… `new globalThis.X()` (#10359) When `try_static_class_name` declines a `globalThis.X` callee because a module class, class alias or import shares the name, the construct fell back to reading the property and constructing its runtime value. That is right for Event/Request/Headers, but several intrinsics are only complete through codegen's builtin table: ReadableStream/WritableStream/ TransformStream came back without methods and WebSocket without readyState. Route the declined global-object callee through `lower_builtin_new` (bypassing module classes) — the construct the unshadowed form reaches — and keep the runtime read only for names no builtin arm owns.
…kend
Two Windows-only breaks on main (CI run 35103968637).
windows-arm64-build, 7 x LNK2019 on js_lru_cache_{new,get,set,has,delete,
clear,peek}: perry-runtime's lru_subclass module declares the cache ABI as
extern "C" and leaves it to whichever provider the PROGRAM links. A Rust
binary that links perry-runtime without one still carries the references,
and the Windows legs build two such binaries -- the perry compiler and the
crate's own --lib test harness. Elsewhere that is invisible because ld64
-dead_strip / ld --gc-sections drop the thunks (and their references) before
the linker reports; link.exe resolves before /OPT:REF, so the same inputs
are hard unresolved externals there.
A Cargo feature cannot express "this link has no provider": the job builds
-p perry -p perry-runtime-static -p perry-stdlib-static in ONE invocation, so
perry-stdlib's perry-runtime/stdlib is unified onto the copy of perry-runtime
that the compiler links, and anything gated on it (stdlib_stubs, an
external-*-symbols flag) is compiled out in exactly the failing configuration.
Use MSVC's weak default instead: an #[cfg(all(windows, target_env = "msvc"))]
module emits one /ALTERNATENAME:js_lru_cache_<op>=perry_lru_cache_absent_<op>
directive per symbol through .drectve, with no-op fallbacks that report via
stub_diag. link.exe substitutes an alternate only for a symbol still undefined
after every input is read, so a link that does carry perry_stdlib.lib or the
ext archive binds the real implementation -- unlike an unconditional
definition, which would duplicate or silently shadow it.
windows-build, E0425 cannot find function `reorder_child` in module `widgets`:
perry-ui-windows-winui #[path]-includes perry-ui-windows' ffi/mod.rs, so
widget_layout_extras.rs resolves widgets:: against winui's own widgets.rs,
which had add_child_at / remove_child / clear_children but no reorder_child.
Add it in that module's shape -- delegate to the Win32 backend when Fluent is
inactive, otherwise reorder the node's child list under with_node_mut, with
the Win32 implementation's guards. perry_ui_widget_reorder_child is a live
UI dispatch-table entry that every other backend implements, so cfg'ing the
caller out would be a regression rather than a fix.
…10362) Base: 33690c5 (main). `CopyingNurseryCollector::visit_slot_with_parent` called `weakref::is_weak_target_trace_slot(parent, slot)` for every slot of every traced object: an out-of-line call that re-reads the parent's `obj_type` and `class_id` and then rejects on class, for every slot of every ordinary object. The per-object form already existed — #10182 gave `weakref::is_weak_holder_header` to the full mark in `gc/trace.rs`, and the copying minor never got it. The fact is read LAZILY, on the first slot that needs it, not eagerly per object. Eagerly regressed all six fixtures (+0.88% to +5.09% instructions): a great many traced objects — strings, pointer-free arrays — have no slot to visit at all, and paid two lookups for an answer nobody then asked for. instructions:u, min of 5, same host, base vs this: gc3 12,572,426,882 -> 12,447,923,597 -0.99% w5000 2,049,874,315 -> 2,016,880,468 -1.61% w20000 5,094,865,215 -> 5,025,205,279 -1.37% oldyoung 1,525,472,951 -> 1,508,726,393 -1.10% w1000 1,090,121,743 -> 1,081,323,176 -0.81% alloc 320,285,196 -> 320,285,242 0.00% Peak RSS within 0.1% on all six. Max pause (min of 5, w20000 interleaved over 9) better or flat on all six. On a control isolating the slot term — 60k records whose fields all point at one shared object, against the same records holding doubles — the pointer-slot-attributable instruction count falls 4.4% at K=2, 5.9% at K=8 and 6.3% at K=16, i.e. 25.5 instructions per pointer-slot visit. The slot visit moves to a new `gc/copying_parent_facts.rs`; the 2000-line file lint required the split, and `gc/copying.rs` ends up smaller than before. Tested: `gc::tests::copy_slot_hoists` pins the behaviour with a collection — a target reachable only through a rooted WeakRef's weak slot must die in the nursery — and its sabotaged twin makes the per-object fact read false, which evacuates through the weak slot and keeps the target alive. NOT included, deliberately: the same hoist for `barrier_parent_needs_remembering`'s generation clause. It measured as the half that makes a 1-pointer-slot object pay (oldyoung +0.198% with it, -1.10% without), and no sabotage could be made to fail for it — sticky dirty-page coverage carries an old->young edge independently of the remembered-set re-insertion, so forgetting the fact changes nothing observable. It needs its own witness before it is worth landing.
#10375's split added `builtin::lower_global_intrinsic_new` after the `new::` group; rustfmt orders it before `field_init`. Whitespace only.
`CANONICAL_MODULE_PATHS` and `CANONICAL_MODULE_DIRS` are new identity-ratcheted thread-locals, so the holders gate fails until each carries a verdict. Both memo filesystem text only — `String` keys with `String`/`PathBuf` values — with no JSValue, NaN-boxed word or arena pointer anywhere in either map, so neither is a GC root. Recorded as researched `not_a_gc_pointer` verdicts rather than pinned as frontier debt, because a frontier pin is explicitly not a GC-safety verdict.
Resolving #10381's conflict with #10387 in `array/mod.rs` dropped `transfer_array_numeric_layout` (deleted by #10381 along with its only caller) and kept `reclassify_array_numeric_layout_from_slots` (added by #10387). One fewer symbol refills the list differently, so rustfmt rewraps it. Whitespace only.
This was referenced Sep 17, 2026
Closed
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (77)
📝 WalkthroughWalkthroughChangesThe pull request bumps the version to Estimated code review effort: 5 — 90 minutes Change: Other Estimated code review effort: 5 (Critical) | ~90 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
This was referenced Sep 17, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This train lands #10378, #10381, #10347, #10375, #10384 and #10388 as v0.5.1585, on
fdc437f966."k" in o, nominal parameter guards for all-number-field class params, inline rest-bundle construction, a hoisted packed-loop element base, and anany_user_prototype_override()latch that letsinstanceof's common miss path skip two ~130-instruction registry probes. 97-probe census 20,308 → 16,996 instructions (−16.3%).gc/layout.rsdown from exactly 2000 lines — the file-size cap — to 1919, with the moved code in a newgc/layout/transfer.rs.--versionissued 86,745readlinkcalls over 9,467 distinct paths; sibling modules re-walked every shared ancestor.new globalThis.X()constructs the global when a binding shadows X (#10359) #10375 —new globalThis.X()constructs the global when a local binding shadowsX.js_lru_cache_*LNK2019s) and the WinUIreorder_childgap. Both changes are fully#[cfg]-gated to Windows.26 source commits. Two were already on main (#10381 stacks on #10371, which landed earlier) — confirmed by finding their patch-ids on main, not assumed.
Train repairs
A conflict with #10387, landed an hour earlier in train 206. Both it and #10381 edit
array/mod.rs's re-export list: #10387 addsreclassify_array_numeric_layout_from_slots, #10381 removestransfer_array_numeric_layoutalong with the function and its only caller atgc/layout.rs:1274. The merge keeps the first and drops the second. A sorted union — the usual reflex for an entry list — would have kept a symbol that no longer exists and broken the build; this was verified by compiling. rustfmt then rewraps the list, since it holds one fewer symbol; that is carried as a separate whitespace-only commit.cargo fmt --check: #10375's split addedbuiltin::lower_global_intrinsic_newafter thenew::group, where rustfmt orders it beforefield_init. Whitespace only.The holders gate: #10347's
CANONICAL_MODULE_PATHSandCANONICAL_MODULE_DIRSare new identity-ratcheted thread-locals, solintfails until each carries a verdict. Both memo filesystem text only —Stringkeys withString/PathBufvalues, no JSValue, NaN-boxed word or arena pointer — so neither is a GC root. Recorded as researchednot_a_gc_pointerverdicts rather than pinned as frontier debt, because the inventory states plainly that a frontier pin is not a GC-safety verdict.Validation
Validated head
9ecf2b0ffd. Five-package release build pinned and hash-verified, and re-verified after the gap run so nothing rebuilt underneath the fixtures.main's one known runtime failure (gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds). The counts rise exactly where these PRs add tests, which is the positive check that the new tests are registered and running.main.instanceof,prototype,param,template,subclass,weak,array,string,global,module,spread,class,concat.Every red attributed
--filteris a substring match over all fixtures, so these runs also selecttest_issue_*/test_ws_*/test_perry_*names that CI's gap suite (--filter test_gap_) never runs; stale snapshot entries for those surface as noise. Each was chased to a mechanism:main's own artifact set —weakref_finalization_gc,state_at_module_init,native_module_after_async_resume,perry_gc_module,prototype_borrow,sqlite_stmt_all_bound_params,table_concat,table_array,guarded_raw_numeric_arrays. All nine behave identically on both arms, with distinct build stamps asserted per arm so neither can silently fall through to the wrong archives. The first four and the last matter most: they sit squarely in perf(module): memoize module path canonicalization per directory #10347's, perf(gc): read the copying minor's weak-holder fact once per object (#10362) #10388's and perf(gc): relocation moves only address-keyed layout records (#10362) #10381's blast radii.main— the two fastify and two mysql fixtures declare// parity-skip(background server / live MySQL), andtest_issue58_object_stringis listed verbatim inrun_parity_tests.sh'sSKIP_TESTS. The harness skips all five before compiling anything.test_message_channel_globaltimes out under the pinned v26.5.1 (aMessageChannelport keeps the loop alive), and the three class fixtures exit 1 inside node's own module resolution (run_main,package_json_reader,esm/resolve).test_gap_json_lazy_defineproperty_index, issue JSON.parse lazy array: Object.defineProperty index accessor is bypassed by reads #10097.Assembly verification
Every one of the 26 source commits is accounted for by patch-id and authorship: 22 exact matches, 2 conflict-resolved with the reason recorded per commit, 2 confirmed already on main by patch-id lookup. An unaccounted commit fails the proof rather than being labelled "probably already landed".
Before merging, the pushed head and unchanged main are checked again. After merging, the rewritten commits are checked for preserved authorship and the main tree must match the validated train exactly.
Summary by CodeRabbit
Bug Fixes
new globalThis.X()so it consistently uses the global constructor even when a local binding has the same name.inchecks, string indexing, and packed numeric loops.Performance
Compatibility
Version