You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/10/09 17:32:07 UTC

[PR] perf: bulk-append long nullable primitive arrays in SparkUnsafeArray conversion [datafusion-comet]

andygrove opened a new pull request, #6824:
URL: https://github.com/apache/datafusion-comet/pull/6824

## Which issue does this PR close?

Closes #4626.

## Rationale for this change

When Comet converts a Spark `UnsafeArray` of primitives to Arrow, as JVM columnar shuffle does for every array column, it appends a nullable array element by element, checking the null bitset for each one. #6604 appends a nullable array without nulls with one `append_slice`, but an array that holds a null still takes the loop.

This continues #4672 by @sandugood, which replaces that loop with one copy of the values and a validity buffer built from Spark's null bitset. #4672 has had no activity since its last review, so this branch keeps its commits, merges main, and addresses the review feedback. GitHub credits @sandugood as co-author when the PR is squash merged.

## What changes are included in this PR?

- An aligned nullable array of at least 64 elements that holds a null is appended with `PrimitiveBuilder::append_array`: one copy of the values plus a `NullBuffer` made by inverting the bytes of Spark's null bitset. Spark sets the bit of a null element and Arrow the bit of a valid one. This covers every type `impl_append_to_builder` generates: the integer and floating-point types, `Date32`, `Time64` and `Timestamp`.
- Shorter arrays and unaligned ones stay on the per-element loop. Building the two buffers takes four allocations, which cost as much as appending 40 to 55 elements one by one. `MIN_BULK_NULLABLE_APPEND_ELEMENTS` records the measurements. This addresses sunchao's review of #4672, which found one-element arrays 11 times slower on the bulk path.
- `append_array` requires the array's data type to match the builder's, and Comet's timestamp builders carry the column's timezone. So the timestamp appender takes the timezone, and the list path in `row.rs` passes it through.
- A nullable array without nulls keeps the `append_slice` path from #6604. The boolean appender reads its bytes through a slice.

## How are these changes tested?

- `nullable_primitive_arrays_round_trip_on_every_append_path` also covers arrays of 63 and 64 elements with nulls at both ends, on either side of the new cutoff, and a timestamp column with a timezone. It runs every element width, with arrays on and off an 8-byte boundary. Dropping the timezone from the bulk path makes `append_array` panic.
- The tests from #4672 cover arrays whose last null sits in a partial byte, all-null arrays and all-valid arrays, now at lengths on both sides of the cutoff. Skipping the bit inversion fails four tests.
- `nullable_arrays_append_into_one_builder` appends several arrays into one builder, as the list path does, so a validity buffer is appended at an offset that is not a whole byte.
- The shuffle crate's tests pass (176), as do `cargo clippy --all-targets --workspace -- -D warnings` and `cargo +nightly miri test -p datafusion-comet-shuffle --lib 'spark_unsafe::'`.

The table shows the per-row cost in ns of appending a list column's arrays, with one null per array. I measured release-build copies of the append method on an M3 Max with the system allocator, driven the way `append_list_column_batch` drives it: a `ListBuilder` whose values builder starts at capacity 100, and 8192 rows cycling through 64 distinct 8-byte-aligned arrays. Each number is the median of 41 interleaved batches for `i32` and 31 for `i64`. The machine was shared with other jobs.

| Elements per array | main, `i32` | this PR, `i32` | main, `i64` | this PR, `i64` |
| ------------------ | ----------- | -------------- | ----------- | -------------- |
| 1 | 6.7 | 6.4 | 6.9 | 6.6 |
| 16 | 36.0 | 36.2 | 36.3 | 36.4 |
| 32 | 71.0 | 70.6 | 74.3 | 74.9 |
| 64 | 148 | 95 | 198 | 170 |
| 256 | 600 | 147 | 799 | 320 |
| 1024 | 2402 | 258 | 3223 | 1149 |

Arrays without nulls cost the same as on main. Taking the bulk path for every aligned nullable array, as #4672 did, a one-element array cost 85 ns and a 16-element array 87 ns.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org