perf(codegen): inline masked string array lengths - #9171
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 (20)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThis change adds guarded fast-path lowering for masked ChangesString array length optimization
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds a guarded fast path for masked string-array length reads while retaining the generic implementation when validation fails. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant LoopLowering
participant RuntimeGuard
participant StringWindow
participant StringLength
LoopLowering->>RuntimeGuard: Validate array and index window
RuntimeGuard-->>LoopLowering: Return eligibility
LoopLowering->>StringWindow: Register validated window fact
StringWindow->>StringLength: Load boxed string and classify length
StringLength-->>LoopLowering: Emit inline SSO or heap length
LoopLowering-->>LoopLowering: Merge fast path with generic fallback
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description covers the optimization, performance impact, linked issue, testing performed, and fallback behavior. It does not reproduce every template heading or checklist item, but the required technical information is present. Full details: Docstring CoverageExplanation Docstring coverage is 56.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 19 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
f16242a to
488b786
Compare
|
Merged via a merge train — cherry-picked with three other PRs onto one branch and validated together in a single build. Combined validation: codegen 1357 passed, runtime 2844 passed (exit 0, 0 abort markers), perry --bins 1066, fmt clean, 20 files, all covered by the combined codegen and runtime suites above. |
Summary
string[]length-accumulation loops behind a one-time plain-array, bounds, element-tag, and accumulator guardPerformance
On
perrymaster.skelpo.net, the issue-shapedstrings[i & 3].lengthbenchmark improves from 5.58 ns/access to 1.14 ns/access (4.9x faster). Node measures 0.63 ns/access on the same host, reducing the gap from about 8.9x to 1.8x.Testing
cargo test -p perry-codegen --test string_array_length_9160cargo test -p perry-runtime typed_feedback --libtest_gap_string_array_masked_length_9160scripts/local_binding_type_audit.pyscripts/check_test_registration.pyscripts/check_file_size.shcargo fmt --all -- --checkFixes #9160
Summary by CodeRabbit
Performance
Bug Fixes
Tests