perf: remove local copy and scalar root-store overhead - #10264
proggeramlug wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (15)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughChangesGeneric function overhead optimization
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant TypeScript
participant perry_transform
participant perry_codegen
participant ARM64_Census
TypeScript->>perry_transform: compile functions with local copies
perry_transform->>perry_transform: remove eligible aliases and inert assignments
perry_transform->>perry_codegen: pass cleaned function body
perry_codegen->>perry_codegen: select scalar store or root-barrier store
perry_codegen->>ARM64_Census: emit ARM64 object code
ARM64_Census->>ARM64_Census: count probe instructions and write JSON
Merge Risk: ⚪ Minimal · up to No concrete correctness, GC-safety, or availability defect remains established for this change. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 13 files. (2 skipped: 2 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 |
|
Landed via merge train #10276 (v0.5.1570). Source changes preserve authorship, and the merged main tree matches the validated train exactly. |
Summary
Plain local copies currently turn into string-sharing and root-shading work that survives LLVM optimization. Remove provably redundant bindings before codegen, and omit incremental root shading when a global/static-field store is proven to contain a scalar.
Against
4945fc1f7498debc76e9f861d7cf1517da5e67c9, the exported three-alias probe drops from 43 to 3 ARM64 instructions, an alias live across a call from 70 to 54, and a constant global write from 11 to 6 under the default-Ospolicy. These are static instruction counts, including cold blocks and excluding callees; they are not throughput claims.Changes
-Os/-O3results inbenchmarks/generic-overhead/README.md.Callback dispatch is an unchanged control. A separate trial reduced caller instructions but regressed ordinary-callback CPU time by 54%, so it is excluded. Broader callback/runtime, closure, arguments, and leaf-root optimizations remain follow-up work.
Related issue
n/a
Test plan
Built the compiler and matching runtime/stdlib static archives from this worktree with LLVM 22.1.4 on Apple M1 Max. Cargo commands used a dedicated target directory and
CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16.cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-staticcargo test --release -p perry-codegen --lib: 1,535 passed, 1 ignoredcargo test --release -p perry-transform --lib: 137 passedtest_gap_generic_function_overhead.ts, including the canonical parity harness.static_fieldparity fixtures pass.-Osand-O3; unknown-value store and callback control counts unchanged.test-files/and both affected crates.scripts/pre-tag-check.sh --quickhas one pre-existing failure: the published benchmark artifact is stale relative to its inputs. The same check fails on the pristine baseline with identical source and harness fingerprints. This PR does not change those benchmark inputs or regenerate the published artifact.Checklist
changelog.d/10264-generic-function-overhead.md.Summary by CodeRabbit
Performance
Reliability
Documentation