Implement RowFn row execution - #9353
Conversation
Merging this PR will degrade performance by 12.34%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=1024/indices=1000] |
5.7 ms | 6.6 ms | -14.2% |
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=1024/indices=256] |
2.2 ms | 2.5 ms | -13.6% |
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=1000] |
9.6 ms | 11 ms | -12.95% |
| ❌ | Simulation | compress_fsst[(1000, 64, 8)] |
1 ms | 1.2 ms | -12.2% |
| ❌ | Simulation | take[core/shuffled/primitive/nonnull/chunks=1024/indices=10000] |
14.3 ms | 16.2 ms | -11.87% |
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=1024/indices=16] |
278.6 µs | 312.8 µs | -10.94% |
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=256] |
3.2 ms | 3.5 ms | -10.54% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ct/row-fn-framework (d65a9da) with develop (42a72d9)
Footnotes
-
89 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. ↩
92e8d51 to
2ceb889
Compare
2ceb889 to
899b19a
Compare
899b19a to
6fdcd8e
Compare
583e721 to
71ad756
Compare
f677e59 to
91f1511
Compare
91f1511 to
4a1d66c
Compare
4a1d66c to
944b540
Compare
8fa974c to
7fddaa2
Compare
6c65ed1 to
6c04398
Compare
| if let Some(views) = views { | ||
| if !Args::view_lens_match(&views, row_count) { | ||
| decoded_length_error(row_count)?; | ||
| } |
There was a problem hiding this comment.
this is from a bad InputElement impl only right?
There was a problem hiding this comment.
yes, but it also helps LLVM know that the loop bound is correct (see owned.rs). I guess I can try and see if removing it changes anything but obviously it is not going to improve perf in any way, and it makes more sense to be defensive here imo
There was a problem hiding this comment.
actually I also want to check if we bound it by the new ViewLen trait I added if that changes the codegen, if not I will change it
There was a problem hiding this comment.
ok I changed it to be an equality check. We definitely still need this check though
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
f14f51f to
d65a9da
Compare
Rationale for this change
RowFnover Vortex arrays #9130RowFnAPI #9129Implements typed row execution for the contracts in #9386. Batch planning and orchestration are split into #9450.
What changes are included in this PR?
Adds owned-output and sink-writing row loops, prepared and deferred execution, the
ViewLencontract, and decoding validation. This layer does not yet connect the row loops toexecute_rows.What APIs are changed? Are there any user-facing changes?
Extends the existing
unstable_row_fnsAPI. There are no stable API changes.