batch: land #8876, #8884 - #8887
Merged
Merged
Conversation
Accumulated codex ECS campaign work (v40–v74) on top of the two prior commits on this branch: Array-subclass dense-tail fast paths and validated-object prototype-override reads (v72), pre-statepoint inlining of compact exact-receiver ($pshape) guarded specializations using the lowered LLVM IR size (v74), plus the supporting collectors/tests. Details, rejected experiments and measurements are in secret-tests/ECS_PERFORMANCE_HANDOFF_2026-08-27.md. Mac mini (taskpolicy -t 0 -l 0, 11 alternating pairs) at v74: wolf-ecs add/remove 0.5562 ms/op, entity-cycle 0.4988 ms/op (Node 26.5.1: 0.1337 / 0.1492). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
… store `this.vals[i] = v` has no writeback slot: once the array grows past its initial capacity the object field keeps the pre-grow forwarding stub, and the guarded property-receiver STORE tier rejected the stub on every later store (`!GC_FLAG_FORWARDED`), sending the whole store out of line through the extend helper and the allocator/registry resolver. The READ tier already followed one edge inline; mirror it: `deref` selects the stub's forwarding word (heap-band checked), a new `deref.live` block re-validates the destination header, and the fast arm stores into the live head. wolf-ecs (Mac mini, 11 pairs): add/remove -9.15% (11/11), entity-cycle -13.67% (11/11). Test: index_set_barrier_tests::the_guarded_property_receiver_store_follows_one_forwarding_edge_inline Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…dispatch - index_set_guarded.rs: the fast arm only calls js_array_note_numeric_write when the live head's `_reserved` word (already loaded by `deref.live`) has a raw-f64 bit set; the note is exactly "clear those bits if the value is not a Number" and was re-resolving the receiver through the tracked resolver on every pointer store. - header.rs: js_array_note_numeric_write returns early for Number values and for already-clear live headers before paying clean_arr_ptr. - indexing.rs: js_array_get_f64 dispatches a GC_TYPE_TYPED_ARRAY-tagged, registered receiver to js_typed_array_get before clean_arr_ptr (a guaranteed tracked miss for a typed array). wolf-ecs (Mac mini, 11 pairs): add/remove -4.86% (11/11), entity-cycle -5.50% (11/11). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…ed-array reads - compare.rs: `typeof local === "number"` / `!==` decides the definitely-Number cases inline (top 16 bits outside 0x7FF9..=0x7FFF, not the untagged raw typed-array pointer shape, outside the Web Streams id band) and keeps js_value_typeof_tag on the slow arm, so the two routes can never disagree. A 33-kind differential probe matches Node byte-for-byte. - index_get/inline_dyn_typed_array.rs: the inline dynamic typed-array read brands the receiver off its GC_TYPE_TYPED_ARRAY header and reads the element kind from the TypedArrayHeader instead of probing the 64-slot direct-mapped PERRY_TA_KIND_CACHE, which every ordinary-array registry miss also writes negative entries into (hot typed arrays kept being evicted and missed the tier). PERRY_TA_VIEW_GUARD still gates the whole tier. wolf-ecs (Mac mini, 11 pairs): add/remove -1.28% (11/11), entity-cycle -0.73% (11/11). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…clean_arr_ptr An ordinary-object receiver (the object-backed `class X extends Array` instance behind wolf-ecs' `packed[sparse[x]]`) can never be an ArrayHeader, so clean_arr_ptr's tracked-allocation resolver was a guaranteed miss on every js_array_get_f64 call for it. Ask array_subclass_fast_index_get_raw first when the header tag already read for the Map/Set probes says GC_TYPE_OBJECT; every rejected case still reaches the complete resolver and spec-generic Get. wolf-ecs (Mac mini, 11 pairs): add/remove -2.03% (11/11), entity-cycle -2.39% (11/11). Cumulative vs v74: -16.5% / -20.9% (0.4645 / 0.3944 ms/op). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…ad tiers A declared-array receiver read with an `Any`-typed key (`packed[sparse[x]]` in the wolf-ecs SparseSet, `a[b[i]]` in general) always took the out-of-line `js_array_get_index_or_string` route because the key carried no integer array-index proof. Test the key inline — nonnegative, below 2^32, and equal to its own fptosi/sitofp round trip — and on a hit take exactly the tiers a statically proven index takes: the inline typed-array read, the dense Array-subclass `arrlike.ic` shape cache, then the complete `js_packed_arraylike_index_get` → `js_dyn_index_get` dispatcher. Fractional, negative, NaN and out-of-range keys keep the previous route. wolf-ecs (Mac mini, 11 pairs): add/remove -2.37% (11/11), entity-cycle -2.89% (11/11); the js_array_get_index_or_string → js_array_get_f64 → array_subclass_fast_index_get_raw chain (4.4% of the add/remove profile) is gone. Cumulative vs v74: -18.5% / -23.1% (0.4531 / 0.3836 ms/op). Test: index_get_claim_tests::any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_index Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
… inline lowering
`x === {…}` with a proven-Number left operand now lowers to an inline
`fcmp oeq` (every non-Number NaN-box reads as a NaN double, so the object
compares unequal exactly as `js_eq` answered), leaving no `js_eq` call for
the test to find. Keep the test's actual claim — the non-pointer left
operand stays in the register produced above the right operand's
allocation instead of being rooted/re-read — on the fcmp operands, and pin
that no runtime equality call remains.
Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
Merge main (#8857–#8874) into the ECS optimization branch and make the branch pass the PR-tier CI gates locally: - conflicts: keep both `js_array_push_u31_with_length` (ours) and `js_array_push_f64_spec` (main); take main's `uitofp` for the fused push length; main moved the `header.rs` GC slot helpers into `header_gc_slots.rs` and the ic_miss length tests into `ic_miss_array_length_tests.rs`, so drop our copies and add codex's `note_array_slot_resolved_flags` to the new module. - semantics parity with main's #8858: the u31 push fast path now declines descriptor-bearing / prototype-invalidated / sparse receivers and routes them through the descriptor-aware `js_array_push_f64_spec`. - 2,000-line file gate: split `indexing.rs` (keyed entry points → `indexing_keyed.rs`), `subclass.rs` (loop guards → `subclass_loop_guard.rs`), `array/tests.rs` (→ `tests_strict_dense.rs`), `codegen/method.rs` (typed clones → `method_typed.rs`), `collectors/ptr_shape.rs` (→ `ptr_shape_numeric.rs`) and `expr/property_get.rs` (composed ICs → `property_get/composed_ics.rs`), each as a `use super::*` child module. - GC store-site inventory: mark the eight raw slot writes in the dense Array-subclass tail helpers and the resolved-flags slot note. - `-D warnings`: remove the inherited unused imports. Verified locally: cargo fmt; workspace `cargo check --all-targets` under `RUSTFLAGS=-D warnings` with CI's host-compatible exclusions; perry-codegen 1309/1309; perry-runtime 2732/2732 (single-threaded); file-size gate, GC store-site inventory, binding audits; default-defer semantics probe and a 33-kind typeof probe byte-identical to Node; wolf-ecs Mac mini parity screen vs the pre-merge build: +0.06% / +0.07% (noise). Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…iers, changelog fragment - addr-class ratchet/allowlist and raw-handle debt ceilings: the sites that the 2,000-line split moved from `array/indexing.rs` into `array/indexing_keyed.rs` keep their existing justification under the new path (indexing 4→3 / 13→7, indexing_keyed 1 / 6); lower the stale `field_set_by_name/fast_paths.rs` handle-floor count 3→2. - shape-descriptor census: refresh the exact call-site multiset for the moved `property_get/composed_ics.rs` sites and the new `stmt/cached_field_index_return.rs` / `generic_dispatch.rs` header-size reads, and pin the scanner's rooting gate as `descriptor.old_carrier || descriptor.cache_carrier` — a runtime optimization cache that can reinstall a historical shape is a strong metadata owner a minor cannot enumerate (see `ShapeDescriptor:: cache_carrier`), so its keys array must be rooted and rewritten before weak pruning. The sabotage self-test is updated to the new gate. - changelog.d/8876 fragment. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
Second merge round after the #8872/#8875/#8877 batch landed. Conflicts and their resolutions: - codegen/method.rs: keep this branch's guarded-falsy/index/pshape-arg clone handling and add main's `!arguments_length_clone` exclusions. - expr/property_get.rs: keep both the Symbol-then-named-field IC dispatch (ours) and main's synthetic `arguments.length` fast path. - property_get/generic_dispatch.rs: main's native Map/Set `size` split ahead of the object PIC, with this branch's `is_object_kind` naming. - lower_call/method_override.rs: `direct_call_fn` (main, argument-length clone) is consulted first, then the pshape+index clone (ours); the two are mutually exclusive by construction. - array/element_shape.rs: adopt main's demand-driven proofs (no eager `establish` on the first store) inside this branch's `note_element_store_with_bit` / `_resolved_flags` split; the now-unused `element_identity_of_bits` goes with it, and the renamed `pushes_do_not_create_an_unrequested_element_shape_proof` test replaces the eager-establishment one. - array/header_gc_slots.rs + mod.rs: keep both resolved-head store helpers (`note_array_slot_resolved_flags` ours, `store_array_slot_resolved` main). - array/push_pop.rs: `js_array_push_f64_resolved` now stores through main's `store_array_slot_resolved`. - array/indexing.rs: the strict setter keeps this branch's dense fast path first, then main's resolved-head strict path; main moved the numeric-range helpers into `array/numeric_range.rs` (byte-identical bodies), so the in-file copies and their keepalive anchors are dropped; main's fused strict store in `js_array_set_index_or_string_strict` is ported into `indexing_keyed.rs`. - expr/index_get_claim_tests.rs: union of imports/constants and both test sets (main's canonical-i32 split tier and this branch's `Any`-key tier are complementary arms). - lower_call/property_get/dynamic_dispatch.rs grew past the 2,000-line gate; the tower-of-pshape routing moved to `dynamic_dispatch_tower.rs`. Verified locally: fmt; perry-codegen and perry-runtime lib + test targets build warning-free; both suites green; file-size, GC store-site, addr-class, raw-handle, shape-descriptor census, binding and architecture audits pass. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…ses densely After merging main (#8878 / #8872's canonical-i32 read split), a declared-array receiver with a non-static key takes the guarded plain-array tier first. On an object-backed `class X extends Array` receiver (wolf-ecs `Archetype`, `packed[sparse[x]]` in SparseSet.has/remove) that guard always misses and `js_typed_feedback_array_index_get_fallback_boxed`'s GC_TYPE_OBJECT arm stringified every index into a by-name lookup (from_utf8 + string alloc + reflection ladder per read): both wolf-ecs benchmarks regressed ~2.2x. The fallback now asks `array_subclass_fast_index_get` for a canonical (plain or INT32-boxed) non-negative index before its registry probes and the by-name path; receivers without a dense proof keep the established route. Mac mini 11-pair screen vs the pre-merge build: add/remove +0.5%, entity-cycle -1.2% (from +126% / +121%); semantics probe byte-identical to Node. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…aw-handle debt -6)
The file split moved six bare `get_raw_{mut,const}_ptr` reads into
`indexing_keyed.rs`, which the raw-handle ratchet rejects as a module that was
not listed at the merge base. Every site had the sanctioned shape already —
root the receiver, run the allocating stringify / symbol store, reload — so
they now use `across_const` / `across_mut`. `indexing_keyed.rs` needs no
ceiling; the baseline ratchets 970 -> 964.
Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…ecycle the cache-carrier gate Review follow-ups on #8876: - `js_array_push_u31_with_length` stays allocate-but-never-reenter: it now answers null for receivers whose push can run user code (indexed descriptors / prototype indices, Proxy traps, foreign families) instead of calling the spec / public push itself; the generated caller takes the complete guarded push (`js_array_push_guard` + `js_array_push_f64`) in a new `apush.u31.generic` block. Test: the fused-push runtime test declines a typed-array receiver; the composed-clone IR test pins the hot path / fallback split. - `js_packed_ecs_u32_loop_guard` declines admission when a component column is shorter than the admitted bound the receiver guard published (`out[6]`), so the fused loop cannot read past a column's payload. Test added. - `object_hot_for_owner` validates the cached table pointer against the current thread's `RuntimeState` before reuse. - `cache_carrier` gets a lifecycle: noted only after an entry naming the pair was inserted, and recomputed from live table occupancy after every full trace (`recompute_cache_carriers_after_full_trace`, called beside the old-carrier rotation) so a descriptor whose entries were evicted stops being rooted. Test: carrier bits follow live occupancy across a recompute. - `js_object_get_symbol_then_field_ic_miss` is declared with the runtime's pointer parameter type. - Minor: parenthesized mixed `&&`/`||` assertion, unique test class ids, `function_this_safe` visited-key includes the terminal-`this` allowance, exhaustive `UnaryOp` match, changelog fragment restated as shipped behavior. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ
…pe path ids_by_facts was the only shape-table map still on std's RandomState. Profiling `claude -p` showed RandomState::hash_one at 17 self-samples inside shapes:: alone (57 across the process) — pure hashing overhead on a lookup that runs on every descriptor install and retire. Its sibling maps already moved off SipHash (#8125). The standing comment argued only against PtrHasher, whose write_* methods OVERWRITE the accumulator — correct for a single-word key, and wrong for this five-field one, which would collapse to its last field. That objection does not apply to FastKeyHasher: it implements only `write`, so the derived Hash's write_u32/write_u64 calls all forward there and FOLD with FNV-1a, reaching every field. The key is internal shape state, never program input, so DoS-resistant hashing buys nothing — the same rationale already applied to the descriptor side tables. Test pins the folding property by varying one field at a time and requiring a distinct hash. Sabotage-checked against PtrHasher: it fails with 'changing keys alone must change the hash'. Suite 2717 passed.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (101)
📝 WalkthroughWalkthroughThis change adds guarded codegen specializations, Array-subclass and array indexing fast paths, Symbol and property inline caches, shape-transition caches, optimized truthiness and comparison lowering, and runtime GC/cache validation. Extensive tests cover optimized and fallback paths. ChangesCodegen specialization and dispatch
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: ✨ Finishing Touches📝 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 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.
Batch landing of two reviewed PRs, validated once as a single merged tree.
ids_by_facts#8885 is not in this batch — it conflicts with #8876 in 6 files (both are ECS follow-ups on the same paths) and needs a rebase onto this.
#8876 — the blocking test, resolved correctly
I held this PR because its caveat claimed
strict_eq_reuses_a_non_pointer_left_operand_across_an_allocating_right_operandwas a pre-existing failure. It was not: the test passed on both currentmainand the PR's own merge-base, so the failure originated in the PR's own commits.The author diagnosed it correctly rather than editing the assertion away. The accumulated commit lowers
provenNumber === {…}inline (lower_strict_eq_against_number), so thejs_eqcall the test looked for no longer exists — the lowering changed, not the invariant.The updated test is stronger, and I verified that rather than taking it on report:
fcmp oeq doubleline, extracts both operands, and assertsleft_producer < right_producer— the non-pointer left operand stays in the register produced above the right allocation, exactly what the original test protected.!ir.contains("@js_eq(") && !ir.contains("@js_strict_eq(")).The rooting argument holds: a non-pointer needs no root, and the pointer-bearing operand is the freshly allocated right value. Nothing was weakened. This is the third failing-test-on-a-perf-PR today, and unlike #8833 and #8858 — where the tests were catching real weakenings — this one was a genuinely stale expectation, established by evidence rather than assertion.
The PR has also been rebased since I flagged the conflicts: merge-base is now
59a4c9aa5, three commits behind rather than twenty.Validation (merged tree)
perry-codegen1322,perry-runtime2743,perry-stdlib124,perry-hir340 — all 0 failedstrict_eq_reuses_a_non_pointer_left_operand_across_an_allocating_right_operandpassesdfchecked before and after; no result produced under ENOSPCNote: the PR's own checklist has
pr-gateunchecked, blocked by a CI-cancel loop from another session on the dev box — an environmental issue, not a code one. The local validation above stands in for it.Summary by CodeRabbit
Performance
Bug Fixes
Tests