perf(codegen): a store emits the class-setter arm only for a name a compiled class declares as a setter - #11859
Merged
Merged
Conversation
added 2 commits
October 3, 2026 23:38
…ompiled class declares as a setter
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (58)
✨ 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.
Part of the key-add creep and binary growth that #11784 introduced.
Cause
#11784 (#10498) emits a class-setter arm (
put.pic.acc) at every static-key store site, on the shape-miss edge ahead of the key-add memo and the ways. Every store that misses its word (a key-add, an overflow store) runs the arm's loads and tag test, and every store site carries the code. The runtime admits a setter entry only for an accessor some compiled class declares (class_chain_has_instance_accessor). So a store whose name no class declares as a setter can never take the arm.Fix
The driver collects the accessor names every compiled class declares, over all modules (
ClassAccessorNames: getters and setters, instance and static, a superset). It hands them to codegen asCompileOptions::program_class_accessor_names, through the same whole-program pathshort_spread_method_candidatestakes, and adds them to the object-cache key. A store site emits the setter arm only for a declared setter name. A compile that does not collect the names (None: standalone, tests) emits the arm everywhere, as before. A store without the arm misses tojs_put_value_set_packed_miss, which asks the same entry first.The read sites' getter arm is NOT gated here, although the same argument applies to it. Gating it cuts tsc's binary by another 7.8 MB and leaves no class-accessor entry primed either way (
read_accessor_primes=0in both arms). But that build moves tsc's collector into a different regime: 233 to 240 full GCs instead of 244 to 247, RSS +4 to +13% (338 to 371 MB against 326 MB), and two of six runs +4% instructions. The same allocation trace diverges later in the collector's pacing. That needs a GC-side look before the getter half lands. Data is in the lane report.Results
Real workloads (n=5, median instructions:u; main = 5b06d69)
Outputs are identical to main's in every run.
Verification
class_setter_arms_are_emitted_only_for_declared_setter_names: with no names collected the store arm is present. A name declared only as a getter gets no store arm, and the read arm is still present. A declared setter keeps the arm.--lib) passed on this branch.perryobject-cache unit tests passed. perry-runtime--lib(RUST_TEST_THREADS=1): 4845 passed, 0 failed.