perf(gc): memoize typed layouts by ShapeId - #8301
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 (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe runtime replaces ChangesShapeId typed-layout integration
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This change memoizes typed layouts by immutable ShapeId while retaining the documented validation and GC behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant TypedShapeInstaller
participant ShapeInstallMemo
participant SHAPE_LAYOUTS
TypedShapeInstaller->>ShapeInstallMemo: Look up ShapeId and mask tuple
ShapeInstallMemo-->>TypedShapeInstaller: Return cached state or miss
TypedShapeInstaller->>SHAPE_LAYOUTS: Probe descriptor on miss
SHAPE_LAYOUTS-->>TypedShapeInstaller: Return typed-layout descriptor
TypedShapeInstaller->>ShapeInstallMemo: Record validated entry
Possibly related issues
Possibly related PRs
🚥 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 |
c54def8 to
95917cc
Compare
|
Merging. Responsive to #8289, which named The invariants I wanted are all covered and passing, which matters because a layout memo keyed by an identifier is only sound if the identifier is stable and the memo yields to validation:
Verified: Two notes: A flake, not a regression. One full-suite run showed Census baseline: your later commit already refreshed it. For the record the delta is strictly removals — two raw One fix pushed: the changelog fragment was named |
Summary
ShapeIdcycles.tsissue reproducer, direct fast-path regression coverage, and the reviewed shape-census reductionPerformance
For
benchmarks/issue-8289/cycles.ts, the median of five local runs dropped from 2.177B to 1.804B retired instructions (-17.1%) while preserving output (3999998000000).Testing
cargo test -p perry-runtime(2,572 passed, 4 ignored; doc tests: 7 ignored)RUSTFLAGS="-D warnings" cargo check -p perry-runtime --all-targetscargo fmt --all -- --checkpython3 scripts/shape_descriptor_census.pypython3 scripts/raw_handle_debt.pygit diff --checkNo version or manifest files changed.
Refs #8289
Summary by CodeRabbit
Performance
Reliability
Testing