Fix flat reader subrange decode reuse#8596
Conversation
Signed-off-by: Luke Kim <80174+lukekim@users.noreply.github.com>
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_bool_canonical_into[(1000, 10)] |
15.9 µs | 26.7 µs | -40.29% |
| ❌ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
169.1 µs | 205.8 µs | -17.83% |
| ❌ | Simulation | slice_empty_vortex |
310 ns | 368.3 ns | -15.84% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 215.3 ns | +27.1% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
333.9 ns | 275.6 ns | +21.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[2048] |
427.8 ns | 369.4 ns | +15.79% |
| ⚡ | Simulation | chunked_varbinview_canonical_into[(100, 100)] |
259.6 µs | 224.5 µs | +15.65% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(100, 100)] |
306.8 µs | 271.9 µs | +12.84% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing spiceai:lukim/8587-regression (0b86845) with develop (bdbf6c4)
Footnotes
-
4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
We had this at one point and removed it #4459 because this effectively doubles memory usage of the reader. Any kind of caching like this has to have invalidation logic. We need memory benchmarks to flag changes like these but we have investigated this in the past |
Fixes #8587.
Summary
FlatReader's decoded array future so synthetic subrange scans share the decoded flat segment instead of issuing repeated decode work.Validation
cargo nextest run -p vortex-layout -E 'test(layouts::flat::reader)'cargo nextest run -p vortex-layoutcargo clippy -p vortex-layout --all-targets --all-featuresgit diff --checkcargo bench --workspace/opt/homebrew/bin/uv 0.11.24; core suites passed: Appian, TPCH, TPCDS, ClickBench, ClickBench sorted, FineWeb, and GH Archive via direct binary rerun. PolarSignals/StatPopGen exposed pre-existing benchmark-definition/runtime backend failures, and bare Public BI requires--opt dataset=<name>.