Walk compose taps with a cursor to recover pyramid_compose - #263
Conversation
#228 made the area-weighted downsample separable, which cut wall-clock ~1.7x but left CodSpeed's simulation gate red: the pull pass indexed a CSR prefix per output bin, so every one of the w bins re-derived both slice ends and paid a range check before summing its 1-3 taps. That cost more instructions than the one-pass scatter it replaced (+24% locally under callgrind), which is what the gate measures. Bins own consecutive runs of the tap array, so hand the resample a cursor instead: `transpose_to_taps` now returns per-bin counts (it already built them before the prefix sum) and the loop splits `n` taps off the front per bin. Same taps, same order, same weights — pure bookkeeping. Measured on the benchmark's shape (2.1M points, 512x384 from the 1024 level), interleaved min-of-9 vs a build of the previous implementation: instructions (callgrind) 22.17M -> 17.97M -19.0% wall-clock (arm64 mac) 0.552ms -> 0.420ms -23.9% That puts instructions back at the pre-#228 count (17.88M) while keeping the wall-clock win, so the gate and the renderer agree again. Output is bit-identical: composed grids fingerprint the same across the benchmark window, the full domain, a cell-aligned subwindow, an offset unaligned window, and an upsample refusal. The cell-aligned bit-exactness contract against bin_2d is untouched.
|
Warning Review limit reached
Next review available in: 29 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will improve performance by 18.85%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_pyramid_compose |
11 ms | 9.3 ms | +18.85% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing alek/compose-tap-cursor (611bb52) with main (989a3a7)
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
1 participant
What
#228 made the area-weighted downsample separable, which cut wall-clock ~1.7x but left CodSpeed's simulation gate red on
test_pyramid_compose(9.8 ms -> 10.9 ms). The cause was bookkeeping, not math: the pull pass indexed a CSR prefix per output bin, so every one of thewbins re-derived both slice ends and paid a range check before summing its 1-3 taps. That is more instructions than the one-pass scatter it replaced, and instructions are what the gate measures.Bins own consecutive runs of the tap array, so hand the resample a cursor instead.
transpose_to_tapsnow returns per-bin counts — it already built them before the prefix sum — and the loop splitsntaps off the front per bin. Same taps, same order, same weights.Numbers
Measured on the benchmark's own shape (2.1M points, 512x384 composed from the 1024 level). Instructions from callgrind on Linux/aarch64, wall-clock from an interleaved min-of-9 A/B against a build of the previous implementation:
That puts instructions back at the pre-#228 count (17.88M) while keeping the wall-clock win, so the gate and the renderer stop disagreeing.
Worth recording a dead end: a 1-D push into the scratch row is cheaper still on instructions (13.8M, -38%) but roughly 2x slower in wall-clock — the store-forwarding chain #228 warned about is real. Optimizing for the counter alone would have shipped a real slowdown.
Correctness
Output is bit-identical, not just close. Composed grids fingerprint the same across five windows: the benchmark window, the full domain, a cell-aligned subwindow, an offset unaligned window, and an upsample refusal. The cell-aligned bit-exactness contract against
bin_2dis untouched.cargo test— 116 passedcargo clippyclean; no newcargo fmtdiffs