perf(json): count a compiled loop's whole-array materialization as traversal evidence - #10249
proggeramlug wants to merge 2 commits into
Conversation
…aversal evidence Traversal feedback (#10150) learns that a program scans its parsed arrays from element reads in lazy_get_rooted. The element-shape loop clone (#10171) materializes a lazy array whole in its preheader through js_array_refresh_local_head before any element is read lazily, so a scan loop served by the clone never produced evidence: every parse built the tape and then materialized every record anyway. On the quiet bench mini records_array_16k:scan and records_array_1m:scan read 1.11x and 1.18x the better of Node and Bun, with a third of the parse samples in build_tape_into. js_array_refresh_local_head now notes one flip's worth of evidence the first time it materializes a lazy array. Its emitters are all cold arms that run about once per receiver, so the added tracked-header probe is not on a hot path.
|
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 (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe runtime now counts the first compiled whole-array materialization of a lazy JSON array as traversal evidence. Tests verify the score changes once for lazy arrays and remains unchanged for plain arrays and repeated refreshes. The changelog records benchmark improvements. ChangesJSON traversal evidence
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change makes the JSON runtime count the first whole-array materialization as traversal evidence, which lets scan-style workloads switch to eager parsing and reduces CPU time on large array benchmarks. No behavior or build problems were identified, so it appears ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
|
Marked ready for review. The remaining hold was waiting on hosted CI, which is not a gate while the GitHub runners are down; the local validation this PR needs is already recorded above: full |
1 participant
Summary
Traversal feedback (#10150) learns that a program scans its parsed arrays from element reads in
lazy_get_rooted, and once it has, eligibleJSON.parsecalls skip the lazy tape and parse eagerly. The element-shape loop clone (#10171) materializes a lazy array whole in its preheader throughjs_array_refresh_local_headbefore any element is read lazily, so a scan loop served by the clone never produced evidence: every parse built the tape and then materialized every record anyway. A symbolizedsampleofrecords_array_1m:scanon the bench mini put a third of the parse samples injson_tape::build_tape_into.js_array_refresh_local_headnow notes one flip's worth of evidence (traversal_feedback::note_compiled_materialization) the first time it materializes a lazy array (traversal_feedback::lazy_array_unmaterializedis read before the materialization, so an already-materialized array is never counted twice). Its emitters are all cold arms that run about once per receiver — the clone preheader and the guarded indexed-read repairs inexpr/index.rs/expr/index_get/guarded_array.rs— so the added tracked-header probe is not on a hot path. Materializing the whole array is valid evidence whatever the loop does afterwards: the tape it was built from is wasted either way.Measurement (quiet bench mini, M1, best of 3 interleaved rounds per engine,
/usr/bin/time -l)mainCPU ms / peak MiBrecords_array_16k:scanrecords_array_1m:scanrecords_array_8m:scanAll other 47 JSON matrix rows unchanged within noise in the same run. Stacked on #10220/#10241 (which reclaim dead promoted trees),
records_array_8m:scanreaches 164.1 ms / 110 MiB (0.96× CPU, 0.83× RSS).Validation
array::refresh_traversal_evidence_tests: a plain array is never evidence; the first materialization of a lazy array is exactly one flip (score 2); a second refresh of the same array adds nothing. Sabotage-checked: removing the evidence call fails it (left: 0, right: 2).RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib: 3794 passed, 1 failed —gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds, which only passes in debug builds and fails under--releaseonmaintoo.scripts/run_lint_gates.sh:main's pre-existing reds (public benchmark evidence freshness,-D warningsdead code inglobal_this_webassembly.rs, API docs drift); the file-size cap, which this change first tripped onarray/header.rs, passes after moving the probe intotraversal_feedback.rs(1996 lines).js_array_refresh_local_head), which is what caught an earlier revision that had moved#[no_mangle]onto the new helper.Summary by CodeRabbit
Bug Fixes
Tests