Avoid row index canonicalization for trivial masks - #9717
Conversation
Merging this PR will degrade performance by 4.73%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | arrow_checked_add_u32_neon[16384] |
13.4 µs | 20.3 µs | -34.28% |
| ❌ | Simulation | cached_indices_i128[0.01] |
48.1 µs | 61.7 µs | -22.04% |
| ❌ | Simulation | compress_fsst[(500, 64, 8)] |
482.1 µs | 536.2 µs | -10.09% |
| ⚡ | Simulation | cached_indices_i32[0.01] |
51.6 µs | 37.5 µs | +37.4% |
| ⚡ | WallTime | words_gather_scalar_avx512[1024] |
124 ns | 100 ns | +24% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing dk/row-idx-mask-sequence-fast-path (130193d) with develop (e7512cf)3
Footnotes
-
206 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. ↩
-
4 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩
-
No successful run was found on
develop(e3923a5) during the generation of this report, so e7512cf was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Pull request was converted to draft
|
Need to fix duckdb exporter first. |
2bb4dc7 to
107552d
Compare
2dea8d5 to
f01ebf0
Compare
107552d to
9cc572c
Compare
Signed-off-by: Daniel King <dan@spiraldb.com>
9cc572c to
130193d
Compare
| .into_array() | ||
| .filter(mask)?; | ||
| let mut ctx = session.create_execution_ctx(); | ||
| filtered.execute::<Canonical>(&mut ctx)?.into_array() |
There was a problem hiding this comment.
why do we do this here? why not just return filtered?
Follow-up to #9717. The row index projection eagerly canonicalized the filtered sequence for value masks. The filter is already lazy and its optimizer reduces all-true masks to the sequence and all-false masks to an empty array, so return the filtered array as-is and let the consumer execute it alongside the expression. This drops the unused session parameter from row_idx_array_future. Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015bULRkXtSPXUAw9sFpoo6y
Follow-up to #9717 Signed-off-by: "Claude" <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
Summary
SequenceArraythrough expression applicationu64canonical array directly for all-false masksStacked on #9720, which handles unsigned
SequenceArrayexport for DuckDB.Tests
cargo +nightly fmt --allcargo test -p vortex-layout row_idx_array -- --nocapturecargo test -p vortex-layout layouts::row_idx -- --nocapturecargo clippy -p vortex-layout --all-targets --all-featurescargo test -p vortex-sqllogictest --test sqllogictests -- slt::duckdb::duckdb/projection_expression_pushdown.slt --nocapture