perf(codegen): skip redundant async frame capture tracking - #8392
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 (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe compiler detects plain async step closures by scanning their statement trees. It skips boxed-capture escape registration for these closures while retaining tracking for nested user closures. Tests verify both behaviors, and a changelog entry documents the change. ChangesAsync box capture selectivity
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to This change narrows async frame capture tracking while preserving nested user-closure capture behavior, with targeted tests and broad validation reported as passing; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Independently validated on macOS — every number in the report reproduces. Merging. Reproduced the A/B myselfInterleaved best-of-11 against my own reference binaries, byte-exact vs the Node oracle on every run:
Within 1.9% of pre-regression on wall, and peak RSS lands below it. Both metrics move The mechanism is counted, not inferred
Every one of the 192,859 released cells was stuck, and the cell pool was recycling The new gate is non-vacuous — I checkedI neutered The predicate itself reads correctly: #8213 protectionAll three named retention tests pass individually: Suites and gates
One flake, unattributedMy first |
Summary
Root cause and measured confirmation
#8303 correctly changed plain async terminal release to name the complete frame, but the generated step closure also captures that complete frame. Codegen declared all of those compiler-private captures through
js_closure_set_box_capture_ptr, even though queued/running step lifetime is already covered by the async activation token.On unmodified main,
PERRY_GC_DIAG=1confirmed the result on asyncpipe: 192,859 cells were allocated and terminal-released, but 0 published; 192,859 stayed resident and the JS/i32/bool registries retained 120,496/24,121/48,242 cells. With this change, all 192,859 publish, resident cells fall to 1,626, and all three registries finish at 0.asyncpipe performance
Required static-wrapper build in both arms:
PERRY_NO_AUTO_OPTIMIZE=1,PERRY_NO_CACHE=1, byte-exact output checked on every run. Wall is best-of-15 interleaved; instructions and RSS are best-of-9 from/usr/bin/time -l.The after arm is close to the pre-regression reference measured in the same interleaved run (0.0934 s best).
#8213 retention protection
escaped_closures_defer_activation_cell_publication_until_gc_deathpassesfull_trace_treats_drained_closure_boxes_as_ephemeron_edgespassescompleted_activation_residue_is_bounded_not_linearpassesjs_closure_set_box_capture_ptredgeperry-runtime --lib: 2,591 passed, 4 ignored, 0 failedValidation
cargo test --release -p perry-runtime --libcargo test --release -p perry-transform— 84 passedcargo test --release -p perry --bin perry— 1,003 passedcargo test --release -p perry-codegen --test release_boxes_lowering— 4 passedbash scripts/run_lint_gates.sh— all 50 reported gates passed (48 script gates plus compile tier)Closes #8384
Summary by CodeRabbit
Bug Fixes
Tests
Documentation