perf: add bounded polymorphic object-write PIC#6823
Merged
proggeramlug merged 3 commits intoJul 24, 2026
Conversation
📝 WalkthroughWalkthroughStatic object-write inline caches now support four bounded shape/slot entries. IC backing globals expand to hold the entries, code generation adds guarded hit and miss paths, and native regressions, benchmark results, and changelog documentation are updated. ChangesPolymorphic write PIC
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant WriteSite
participant InlineCache
participant Runtime
WriteSite->>InlineCache: Check cached shape and slot entries one through four
InlineCache-->>WriteSite: Return matching slot or miss path
WriteSite->>Runtime: Call js_put_value_set_ic_miss with the selected token
Runtime-->>WriteSite: Return miss result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-codegen/src/expr/proxy_reflect.rs (1)
397-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the stale "second entry" comment for the new 4-entry cascade.
This comment still only describes a two-entry design, but the block below it is now the first link in a four-guard chain (guard2 → dispatch3 → guard3 → dispatch4 → guard4). Worth expanding to describe the full bounded-cascade behavior so the control flow here isn't misread as two-entry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/expr/proxy_reflect.rs` around lines 397 - 399, Update the comment preceding the cache cascade in the proxy-reflection code to describe the full four-entry bounded cascade, including the guard2 → dispatch3 → guard3 → dispatch4 → guard4 progression. Remove the outdated two-entry wording while preserving the explanation that the initial entry is filled first and later distinct shapes advance through the cascade without changing the miss ABI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/tests/native_proof_regressions.rs`:
- Around line 13485-13489: Extend the write PIC assertions in the regression
test to require both put.pic.miss2 and put.pic.miss3 in addition to the existing
miss checks. Keep the guard2, guard3, guard4, and miss4 assertions unchanged so
all four bounded shape miss paths are validated.
---
Nitpick comments:
In `@crates/perry-codegen/src/expr/proxy_reflect.rs`:
- Around line 397-399: Update the comment preceding the cache cascade in the
proxy-reflection code to describe the full four-entry bounded cascade, including
the guard2 → dispatch3 → guard3 → dispatch4 → guard4 progression. Remove the
outdated two-entry wording while preserving the explanation that the initial
entry is filled first and later distinct shapes advance through the cascade
without changing the miss ABI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6ea45d9-5f6b-4134-b5ea-c67e73ccded6
📒 Files selected for processing (7)
benchmarks/object-write-6812/followup-polymorphic-shapes.mdcrates/perry-codegen/src/codegen/closure.rscrates/perry-codegen/src/codegen/entry.rscrates/perry-codegen/src/codegen/function.rscrates/perry-codegen/src/codegen/method.rscrates/perry-codegen/src/expr/proxy_reflect.rscrates/perry-codegen/tests/native_proof_regressions.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/perry-codegen/src/codegen/entry.rs
- crates/perry-codegen/src/codegen/method.rs
Comment on lines
+13485
to
+13489
| ir.contains("put.pic.guard2") | ||
| && ir.contains("put.pic.guard3") | ||
| && ir.contains("put.pic.guard4") | ||
| && ir.contains("put.pic.miss4"), | ||
| "the write PIC should retain four bounded shape entries" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the second and third miss paths too.
The existing generic put.pic.miss check only covers the first entry, while this new assertion checks only put.pic.miss4 explicitly. Add put.pic.miss2 and put.pic.miss3; otherwise either path could regress while the “four bounded shape entries” assertion still passes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/tests/native_proof_regressions.rs` around lines 13485 -
13489, Extend the write PIC assertions in the regression test to require both
put.pic.miss2 and put.pic.miss3 in addition to the existing miss checks. Keep
the guard2, guard3, guard4, and miss4 assertions unchanged so all four bounded
shape miss paths are validated.
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.
Summary
Evidence
The two-shape cell improved from roughly 5.3 s to 667 ms in the measured release run, with matching checksums. Monomorphic writes remain 123 ms vs Node 133 ms. The final four-shape release sweep (15 alternating Node/Perry pairs) measured Node median 110 ms and Perry median 461 ms, with 60,000,000 writes and checksum 62,876,400 on every sample.
Validation
cargo fmt --all -- --checkgit diff --checkcargo test -p perry-codegen --test native_proof_regressions— 250 passedSummary by CodeRabbit