Replace filter-and-scatter RowFn fallback with filtered valid-row execution - #9645
Replace filter-and-scatter RowFn fallback with filtered valid-row execution#9645robert3005 wants to merge 2 commits into
Conversation
|
Need to go over this carefully but I think this is a better sparse execution mode |
Merging this PR will improve performance by 22.81%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | arrow_checked_add_u32_neon[16384] |
20.5 µs | 13.5 µs | +51.91% |
| ⚡ | WallTime | infallible_bool_neon[i32, PerRowPerRow] |
4 µs | 3.6 µs | +10.79% |
| ⚡ | WallTime | subtract_shapes_neon[(16384, PerRowNullableConstant)] |
12.3 µs | 11.2 µs | +10.06% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/rowfn-kernel-execution-fqw0ue (9c85e69) with develop (68e2aee)
Footnotes
-
106 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. ↩
|
I don't think we can make the skipped row initializer required, and if you rebase on the latest changes from #9623 that should explain why This only is fine if we have types that work with completely arbitrary data that has been allocated, and the spatial types are a counterexample |
…cution When a partially valid batch cannot execute directly over the original inputs, batch execution previously filtered every input to the valid rows, ran the dense kernel over the compact domain, and scattered the compact output back with a nullable-index take. Batch execution now dispatches a filtered valid-row execution instead: it still filters the inputs (required when a representation cannot decode null payloads), but the row loop reads consecutive compact rows and writes each result directly at its original row index into a full-length output, so the kernel output never needs a columnar scatter. Owned outputs place default placeholders in skipped positions and sinks run their skipped-row initializer, after which batch execution masks the skipped rows exactly like direct skip-invalid execution. A sink without a skipped-row initializer can no longer execute a partially valid batch, because every skip-invalid strategy now writes into the original row domain. Both in-tree sinks already initialize skipped rows. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161JW9aU6W4zmzRQsnjbE3Q
|
I don't understand how scatter is physically different than this. I think codex is bullshitting here. I think I can make this work |
813e082 to
37ea28f
Compare
Skipped-row initialization was an optional capability because the old filter-and-scatter fallback could serve a sink without one by running it densely at the compact length and scattering afterwards. With that fallback gone, a sink without an initializer failed partially valid batches at runtime. Every skip-invalid strategy now writes into the original row domain, so skipped rows must always be initialized. Instead of requiring each sink to spell that out, OutputSink::initialize_skipped_rows gains a default implementation that zero-initializes the rows through the new FillDefault bound on OutputSink::Rows: plain slices of Default elements fill themselves, a custom row view implements the filling for its own storage (UninitElementRows, FixedSizeRows), and rows that are fully initialized at construction wrap themselves in Preinitialized to make it a no-op. The Option-returning skipped_rows_initializer, the sink decline in direct skip-invalid execution, and the runtime error in filtered execution are all gone. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0161JW9aU6W4zmzRQsnjbE3Q
37ea28f to
9c85e69
Compare
|
Ok, instead of requiring an initaliser we provide FillDefault for Rows type of the sink. This should be the same as take since that take still MUST initialise memory to something. Instead we pull this functionality up to the output write instead of creating another copy through scatter |
3 participants
Instead of performing a gather/execute/scatter add a filtered execute method that avoids majority of intermediate state