Skip to content

perf(codegen): pack monomorphic method shape guards - #8676

Closed
proggeramlug wants to merge 2 commits into
perf/dynamic-add-tree-guard-after-forwardingfrom
perf/packed-method-shape-guard
Closed

perf(codegen): pack monomorphic method shape guards#8676
proggeramlug wants to merge 2 commits into
perf/dynamic-add-tree-guard-after-forwardingfrom
perf/packed-method-shape-guard

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Summary

  • collapse the monomorphic direct-method GC header checks into one masked i32 load
  • compare adjacent class and ShapeId fields as one packed i64 word
  • retain both prototype invalidation latches, pointer-band validation, ShapeId-domain validation, and the unchanged generic fallback
  • add anti-drift tests for the packed mask, supported little-endian layout, and emitted guard contract

This is a general codegen optimization. It does not recognize ECS names or source locations.

Why

The unchanged codehz/ecs query loop invokes the same guarded helper twice per entity. The existing inline guard loaded GC type, forwarding flags, descriptor flags, class id, and ShapeId separately on every invocation. These fields are contiguous in the runtime ABI, so the same proof can be expressed with two receiver loads while retaining every semantic rejection condition.

Correctness

  • perry-codegen library suite: 1,193 passed, 0 failed, 1 ignored
  • focused packed-mask, endian-layout, and emitted-IR tests pass
  • unchanged method-invalidation fixture matches in normal and PERRY_GC_FORCE_EVACUATE=1 runs
  • unchanged upstream ECS fixture: 7/7 tests and exact 50,005,000 checksum in normal and forced-evacuation runs
  • the package-wide typed_feedback integration assertion also fails on the exact parent; it is unrelated to this diff

Mac mini A/B

Host: Apple M1 Mac mini, macOS 26.5.1, AC power, taskpolicy -t 0 -l 0. Each cohort passed a 60-sample quiet gate before measurement. Control is the exact current stacked parent from PR #8675. Each process used 2 warmups, 6 measured rounds, repeat 64, alternating order, and checked the upstream shim plus checksum.

workload control median candidate median median paired improvement wins oracles
10k forEach accumulation 0.259273 ms 0.249100 ms 3.957% 11/11 22/22
10k forEach read-only 0.326225 ms 0.315803 ms 3.269% 11/11 22/22

Candidate ECS binary SHA-256: 0cbd1f8ad658e83b9d523b257da53254d7f964fcc354b27ddde99b118ce60ab6.

This is a repeatable improvement, not a parity claim. The latest host-matched Node measurements still leave substantial work after this patch.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ed08bf9-3a71-4b1e-a458-8364fb73d043

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

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.

proggeramlug added a commit that referenced this pull request Aug 24, 2026
…ape guards (#8682)

Lands three reviewed PRs as one squash: #8674, #8675, #8676.

- #8674: compress array forwarding chains (`clean_arr_ptr` multi-hop walk).
- #8675: share numeric guards across dynamic add trees.
- #8676: pack monomorphic method shape guards.

These three were authored as a stack on top of #8672, but their contents
touch disjoint files, so they are cherry-picked onto main on their own.
#8672 is NOT included: it defines its own
`is_bound_native_method_closure_value` (true for any bound native-module
export with a non-empty module name), which #8662 superseded on main with
the strictly narrower `is_bound_native_constructor_closure_value` (gated
on explicit constructor metadata). Those predicates have different truth
sets, so the substitution is a behavioural change at every call site and
is left to the author to rebase.

Also splits `array/tests.rs`, which #8674 pushed over the 2000-line cap,
into an `array/forwarding_tests.rs` sibling. Pure relocation.

Version bump not included per maintainer policy.

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via #8682 (squash 67f3f7501).

Your branch was stacked on #8672, but the contents touch disjoint files, so this commit was cherry-picked onto main on its own — #8672 itself is held pending a rebase (details on that PR).

Validated on the merged result: 9 ratchet gates + cargo fmt --check pass, perry-codegen --lib 1198/0, perry-runtime --lib 2652/0 at RUST_TEST_THREADS=1.

One adjustment: #8674's new test pushed array/tests.rs from 1998 to 2034 lines, past the 2000-line gate, so the growth-forwarding group was relocated into an array/forwarding_tests.rs sibling (matching the existing subclass_tests pattern). Pure relocation — all 5 moved tests verified still running. Thanks!

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