land #8885: ECS command path, composed with #8876 - #8888
Merged
Conversation
added 2 commits
August 27, 2026 20:05
|
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 (65)
📝 WalkthroughWalkthroughThis change adds compiler passes and runtime fast paths for branded type aliases, closure locals, dynamic values, arrays, maps, write barriers, and GC layout bookkeeping. It also adds targeted IR, runtime, and regression tests. ChangesCompiler and type lowering
Runtime changes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Compiler
participant Codegen
participant Runtime
Compiler->>Codegen: lower aliases and optimized expressions
Codegen->>Runtime: call guarded fast paths
Runtime-->>Codegen: return resolved values or fallback results
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.
Lands #8885, rebased onto the
mainthat now contains #8876 and #8872. All six conflicts resolved here rather than asking for a rebase.The conflicts, and how they were resolved
Two were genuine design overlaps between #8885 and #8876, and both compose — neither side was discarded:
builtins/arithmetic.rs— perf(ecs): guarded store follows forwarding edge, inline typeof/typed-array/subclass fast paths (wolf-ecs -16.5% / -20.9%) #8876 splittypeofclassification into aValueTypeofTagenum with ajs_value_typeof_tagfast path; perf: ECS command path follow-up to #8872 — −40% on the codehz/ecs 5k-entity sync row #8885 collapsed eight separatethread_local!slots into oneperry_thread_local!array indexed by slot constants, removing a_tlv_get_addrresolution per call. Kept perf(ecs): guarded store follows forwarding edge, inline typeof/typed-array/subclass fast paths (wolf-ecs -16.5% / -20.9%) #8876's enum and classifier, adopted perf: ECS command path follow-up to #8872 — −40% on the codehz/ecs 5k-entity sync row #8885's array, rewrote the call sites to pass slot constants.expr/compare.rs— perf(ecs): guarded store follows forwarding edge, inline typeof/typed-array/subclass fast paths (wolf-ecs -16.5% / -20.9%) #8876 handles "exactly one operand statically proven numeric"; perf: ECS command path follow-up to #8872 — −40% on the codehz/ecs 5k-entity sync row #8885 handles "neither proven, but both dynamically numbers". Complementary, so the static-proof path runs first and falls through to perf: ECS command path follow-up to #8872 — −40% on the codehz/ecs 5k-entity sync row #8885's inlinefcmpinstead of the bare helper call.Four resolved toward
main, including one where #8885's side was simply empty (it predates #8858's frozen-array throw helpers).Four follow-on defects surfaced and were fixed: a stale
array_has_named_propertiesimport (the mergedheader.rsonly defines_resolved), a droppedAtomicBool/AtomicU8import, a dropped localQNAN_PREFIX_I64const, and — the substantive one — five statics that #8885 relocated to a newindexing_support.rswere restored inindexing.rsby hunk-level resolution, producing duplicate#[no_mangle]symbols. Hunk resolution cannot reconstruct a file-level reorganisation when both sides edit the same files.The soundness test: the assertion was over-broad, not the code unsound
noncanonical_bitwise_method_does_not_gain_raw_number_truthinessfailed. It is not pre-existing — it exists onmainbut not in #8885's base, so a clean rebase would hit it too.Diagnosis:
fcmp onehad exactly one source when that test was written — the constructive-proof shortcut (ConstructiveMethodTruthiness::RawNumber), which is genuinely unsound for an arbitrarya & breturn. #8885 introduces a second, unrelated source: a general dynamic-truthiness diamond that emitsfcmp oneonly inside its owntruthy.numblock, after a bit test has proved the value is a plain untagged non-NaN double. There it is exactly correct, andtruthy.slowstill callsjs_is_truthyfor strings, BigInt, pointers, handles, int32 boxes and NaN.I did not invent that distinction.
type_analysis/numeric/tests.rs:796already encodes it, with a doc comment stating it verbatim: "decides a plain (untagged, non-NaN) double inline withfcmp one— but only inside itstruthy.numblock… Anfcmp oneanywhere else is the unguarded numeric claim these tests forbid." The failing test's assertion was mirrored onto that helper.The test's claim is preserved. It still forbids unguarded numeric truthiness, and its first assertion — that
@js_is_truthy(must still appear — is untouched and passing. Only the mechanism of checking changed, to account for a second legitimate emitter.match_u32_bitset_test, the structural proof the test guards, is byte-identical tomainand still rejects a barea & b.Validation (merged tree)
perry-codegen1322,perry-runtime2749,perry-stdlib124,perry-hir348 — all 0 failedPERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1): 16 failed on the batch and 16 on cleanmain— same-commit A/B, none introduceddfchecked before and after; no result produced under ENOSPCPerformance claim needs re-measuring
#8885's −40% was measured without #8876, and both campaigns optimise the same ECS hot paths —
compare.rs,lower_conditional.rs,array/indexing.rs,arithmetic.rsall took changes from both. Whatever this composed tree performs at, it is not that number. The functional result stands on its own; the figure should be re-taken on the bench mini before it is repeated.Summary by CodeRabbit
Performance
Bug Fixes
Reliability