Optimize MaxMap sparse range iteration with a lazy realized-key cache#94
Optimize MaxMap sparse range iteration with a lazy realized-key cache#94peter941221 wants to merge 3 commits intosigp:mainfrom
Conversation
|
Pushed a small follow-up after a deeper review pass. It adds an early return for empty out-of-range The main optimization is unchanged; this just removes the most obvious one-shot cold-start objection without changing the public API or the lazy materialization model. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 70.15% 70.81% +0.65%
==========================================
Files 22 22
Lines 1280 1326 +46
==========================================
+ Hits 898 939 +41
- Misses 382 387 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This looks pretty good, albeit maybe on the slightly complicated side. I'll do a bit of independent investigation and see what I can come up with. If I reach the same conclusion I'll be sure to merge your PR! Btw, guessing you used AI quite heavily here? Not an issue, just we are trying to track AI contribs to LH. |
|
Thanks — that sounds good, and I appreciate the independent pass. And yes, this was AI-assisted. I used AI mostly to speed up codebase exploration, benchmark/test iteration, and to sanity-check implementation ideas, but the root-cause investigation, design choice, final code changes, and local validation were mine. I also tracked the Ubuntu CI failure down to the DHAT helper in If it helps your LH tracking, I'm happy to add a short AI-assistance note to the PR description as well. |
|
Quick follow-up now that I am not trying to force this into the just-ship-the-release path. The current head here is still If the generic If, after |
Summary
This fixes the sparse-update hot path behind repeated
for_each_range(...)calls onMaxMap<VecMap<_>>.The core issue was that range iteration over sparse updates still paid for dense scans across large logical intervals during apply-time tree rebuilding. This patch makes
MaxMaplazily cache realized update keys and answer subsequent range scans via binary search over that sorted key set.What Changed
MaxMapfor_each_range(...)get_cow_with(...)lazy materialization semanticsmax_index()behavior compatible with the existing tests/proptestsWhy This Shape
UpdateMapAPI unchangedTree::with_updated_leaves(...)andPackedLeaf::update(...)Cowmodel instead of forcing eager materializationValidation
cargo checkcargo check --all-targetscargo check --benchesget_mut_with,get_cow_with, and sparse apply pathscargo test --releaseapply_updatesBenchmark Snapshot
sparse_two3.72-4.08 ms2.93-3.07 mssparse_eight4.34-4.58 ms3.03-3.23 msdense_window_51214.70-17.89 ms1.98-2.29 msAdditional Context
Algorithmically, this changes the apply path from repeated dense interval scans to sparse ordered range reporting over realized update keys. The implementation is intentionally simple and local, but the framing is close to classic ordered-set / range-reporting work:
10.1145/1132516.113255110.1145/380752.380842This patch is not attempting to implement those structures directly; it is applying the same high-level move in a minimal crate-local form.