Skip to content

Implement RowFn row execution - #9353

Merged
connortsui20 merged 5 commits into
developfrom
ct/row-fn-framework
Aug 18, 2026
Merged

Implement RowFn row execution#9353
connortsui20 merged 5 commits into
developfrom
ct/row-fn-framework

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 11, 2026

Copy link
Copy Markdown
Member

Rationale for this change

Implements 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 ViewLen contract, and decoding validation. This layer does not yet connect the row loops to execute_rows.

What APIs are changed? Are there any user-facing changes?

Extends the existing unstable_row_fns API. There are no stable API changes.

@codspeed-hq

codspeed-hq Bot commented Aug 11, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 12.34%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 7 regressed benchmarks
✅ 1993 untouched benchmarks
⏩ 89 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

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)

Open in CodSpeed

Footnotes

  1. 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.

@connortsui20
connortsui20 marked this pull request as ready for review August 11, 2026 16:05
@connortsui20
connortsui20 changed the base branch from develop to ct/lane-kernel-sources August 11, 2026 16:07
@connortsui20
connortsui20 marked this pull request as draft August 11, 2026 16:24
Base automatically changed from ct/lane-kernel-sources to develop August 11, 2026 17:12
@connortsui20 connortsui20 added the changelog/feature A new feature label Aug 11, 2026
@connortsui20
connortsui20 marked this pull request as ready for review August 11, 2026 18:48
@connortsui20
connortsui20 marked this pull request as draft August 12, 2026 18:12
@connortsui20
connortsui20 changed the base branch from develop to ct/row-fn-types August 12, 2026 18:12
@connortsui20 connortsui20 changed the title Add the RowFn scalar function framework Implement the RowFn execution backend Aug 12, 2026
@connortsui20
connortsui20 force-pushed the ct/row-fn-framework branch 2 times, most recently from f677e59 to 91f1511 Compare August 12, 2026 19:23
@connortsui20
connortsui20 marked this pull request as ready for review August 12, 2026 19:25
@connortsui20
connortsui20 marked this pull request as draft August 12, 2026 20:26
Comment thread vortex-array/src/scalar_fn/unstable/row/types/sink.rs Outdated
Comment thread vortex-array/src/scalar_fn/unstable/row/batch/execute/filter_scatter.rs Outdated
Comment thread vortex-array/src/scalar_fn/unstable/row/batch/execute/filter_scatter.rs Outdated
@connortsui20
connortsui20 force-pushed the ct/row-fn-framework branch 4 times, most recently from 8fa974c to 7fddaa2 Compare August 17, 2026 19:17
@connortsui20 connortsui20 changed the title Implement the RowFn execution backend Implement RowFn row execution Aug 17, 2026
@connortsui20
connortsui20 force-pushed the ct/row-fn-framework branch 5 times, most recently from 6c65ed1 to 6c04398 Compare August 17, 2026 22:00
Comment thread vortex-array/src/scalar_fn/unstable/row/execute/sink.rs Outdated
if let Some(views) = views {
if !Args::view_lens_match(&views, row_count) {
decoded_length_error(row_count)?;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is from a bad InputElement impl only right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I changed it to be an equality check. We definitely still need this check though

Comment thread vortex-buffer/src/bit/buf.rs
Comment thread vortex-buffer/src/bit/buf.rs
Comment thread vortex-buffer/src/bit/buf.rs

@joseph-isaacs joseph-isaacs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

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>
@connortsui20
connortsui20 merged commit d4dc3f2 into develop Aug 18, 2026
94 of 96 checks passed
@connortsui20
connortsui20 deleted the ct/row-fn-framework branch August 18, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants