perf: remove residual Wolf ECS admission overhead - #8848
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe IC miss handler now supports validated ChangesArray subclass length lookup
Uint32Array admission cache
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR makes localized runtime performance optimizations with documented validation and no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description clearly explains the purpose, implementation, performance results, and validation commands. It does not use every template heading, but it provides the required core information and is mostly complete. Full details: Title checkExplanation The title clearly identifies the performance-focused removal of residual Wolf ECS admission overhead. It covers the primary objective, although it does not mention the related Array-subclass length optimization. ✨ 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 |
* perf: cache owning Uint32Array admissions * perf: fast-path Array subclass length misses * perf(codegen): route proven Array length writes * perf(runtime): bulk-truncate ordinary dense arrays * chore: add array truncation changelog * runtime: add Node-API host core * docs: add Node-API host changelog fragment * runtime: harden Node-API host contracts * runtime: bound Node-API UTF-16 encoding * codegen(calls): pad under-applied same-module direct calls with undefined (#8770) A same-module direct call with fewer arguments than the callee's declared parameter count lowered only the provided args, leaving the remaining FP argument registers holding caller-saved garbage — which the callee then read as JS values. The cross-module twin (extern_func.rs, the issue #608 arm) has always padded missing trailing args with TAG_UNDEFINED; the same-module plain arm sat "one else away" (#7154's own words) unpadded. On the Claude Code bundle — one giant module, so EVERY direct call resolves through the same-module arm — `aP([q])` for `function aP(q, K = !1, _)` handed K/_ whatever d1/d2 held after js_array_from_values: impossible-NaN bit patterns (0xffffffffffffffff) that flowed into truthiness tests and method receivers (`_.get(A)`) and faulted in shape_is_url_search_params / js_is_truthy (~60% of `cc -p` runs SEGV), or silently corrupted the async iteration ("Detected unsettled top-level await", most of the rest). With the padding, `cc -p` runs 30/30 clean: 0 SEGV, 0 hangs, 0 unsettled awaits, node-identical output on every run. The GC-knob correlations the long #8770 investigation chased (scavenge pacing, conservative-scan "fixes") were register-content side effects of the missing padding, not collector bugs. Regression test: an under-applied direct call to a 3-param callee must emit all three double args, the omitted two as the TAG_UNDEFINED literal. Claude-Session: https://claude.ai/code/session_01Ay8VyLkKbm8Hkc1xmvTEsP * fix(async_hooks): complete node suite parity * chore: batch-landing fixes (node-api scoped ptrs, header/ic_miss splits, fmt) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
|
Landed on |
1 participant
Summary
Uint32Arrayadmissions in the existing direct-mapped TypedArray cache and invalidate the entry whenever the live receiver ceases to satisfy that ownership contract.lengthmisses through the existing exact-ShapeId dense-subclass proof while retaining ordinary lookup for accessors, prototype overrides, sparse layouts, and unrelated objectsWhy
After #8847,
noctjs/wolf-ecs/simple_iterhad a branch-light generated swap loop, but every invocation still paid two avoidable runtime costs:Uint32Arrayregistry admission for the same component columns; andQuery/Archetype.lengthreads because Array subclasses useObjectHeader, notGC_TYPE_ARRAY.The
.lengthmiss was the material residual. The cache is a smaller supporting improvement.Performance
Pinned Apple-silicon Mac mini, unchanged strong Wolf oracle, precompiled binaries, alternating order,
taskpolicy -t 0 -l 0:A separate aggregate entity-range proof plus explicit loop-unroll experiment was tested and rejected: its quiet-host 9-pair paired median was 0.55% worse with 4/9 wins. None of that experiment is included here.
Validation
cargo fmt --all -- --checkgit diff --checkcargo check --release -p perry-codegen -p perry-runtimecargo test --release -p perry-runtime --lib owning_u32_admission_cache_skips_registry_and_invalidates -- --nocapturecargo test --release -p perry-runtime --lib array_length_fast_path_tests -- --nocapture(2/2)cargo test --release -p perry-runtime --lib array::subclass_tests -- --nocapture(10/10)Summary by CodeRabbit
Bug Fixes
lengthhandling for array subclasses while preserving standard object behavior.Performance
Uint32Arrayaccess through caching.Tests