Repository navigation
RowFn framework and machinery - #9319
connortsui20 wants to merge 3 commits into
Conversation
Introduce typed row execution with self-contained kernel arguments and explicit execution contracts. Validate decoded lengths once per batch while preserving specialized mixed-constant loops. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Cover RowFn executor overhead and strict validity policies with focused microbenchmarks. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Merging this PR will improve performance by 26.84%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
c626a33 to
84837ad
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
joseph-isaacs
left a comment
There was a problem hiding this comment.
i'll continue tomorrow.
i would be interested in keeping the internals private so we can iterate
| #[derive(Clone, Copy)] | ||
| pub struct KernelArgs<'a> { | ||
| /// The input arrays for this kernel invocation. | ||
| pub arrays: &'a [ArrayRef], |
There was a problem hiding this comment.
does this struct not exist?
| } | ||
|
|
||
| /// One batch of inputs and the metadata needed before its row kernel runs. | ||
| pub struct Batch { |
There was a problem hiding this comment.
does it need to be pub?
| Dense, | ||
|
|
||
| /// Evaluate all rows, retrying only valid rows if a deferred error is raised. | ||
| DenseWithRetry, |
There was a problem hiding this comment.
shall we remove this initially?
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub enum RowPolicy { | ||
| /// Evaluate all rows and mask the result. | ||
| Dense, |
There was a problem hiding this comment.
another option is zipping output with rows and combing valid and fail?
| pub trait RowFn: 'static + Sized + Clone + Send + Sync { | ||
| /// Options for this function, if any. Use [`EmptyOptions`](crate::scalar_fn::EmptyOptions) | ||
| /// for none. | ||
| type Options: 'static + Send + Sync + Clone + Debug + Display + PartialEq + Eq + Hash; |
There was a problem hiding this comment.
should we give this a name and use it with scalar fn and here
| /// - The output **must not** introduce a null where every input is valid. | ||
| /// | ||
| /// The framework skips this hook for nullary functions. | ||
| fn reduce_encoded( |
| ArgColumnKind<T>, | ||
| ); | ||
|
|
||
| enum ArgColumnKind<T: InputElement> { |
There was a problem hiding this comment.
we should use this throughout all of vortex
Tracking Issues: #9129, #9130
Adds the
RowFnAPI, its private batch executor, and focused executor benchmarks.TODO