HarshilMital opened a new pull request, #7201:
URL: https://github.com/apache/datafusion-comet/pull/7201
## Which issue does this PR close?
Part of #7080.
## Rationale for this change
`row_columnar` only covered Int64 primitives, structs, `List<Int64>` and
`Map<Int64, Int64>`, with no nulls. Strings are the most common shuffled type
and take a different path (offset/size pointers into the variable-length
region, and an optional dictionary builder); map keys are often strings; and
real data has nulls. List cases are left out on purpose: #6855 asks that the
`list_conversion/elements_100` anomaly be understood before new List cases are
added here.
## What changes are included in this PR?
All through `process_sorted_row_partition`:
- `string_conversion`: 10 Utf8 columns.
- `plain`: `prefer_dictionary_ratio` 1.0.
- `dictionary`: ratio 10.0 (Comet's default) over 16 distinct values per
column, so the output stays `Dictionary(Int32, Utf8)`.
- `dictionary_fallback`: ratio 10.0 over distinct values, so the column is
built as a dictionary and cast back to plain, which is what a high-cardinality
string column costs under the default.
- `map_utf8_conversion`: `Map<Utf8, Int64>` at 10 and 100 entries, the same
grid as `map_conversion`.
- `nullable_conversion`: 100 Int64 fields with 10% null, and `Map<Int64,
Int64>` with 10% of values null at 10 and 100 entries. Nulls fall by a hash of
row and position.
- New UnsafeRow builders: `build_string_row`, `build_utf8_map_row` (string
slots relative to the key array start, as `SparkUnsafeObject::get_string` reads
them) and `build_primitive_row`.
- `run_benchmark` takes `prefer_dictionary_ratio` instead of hardcoding 1.0,
and no longer calls `set_not_null_at` on every field (every builder already
zeroes its bitset, and clearing would erase the new null bits). `build_map_row`
takes a row offset and a value null fraction; the existing map cases pass `(0,
0.0)`, which reproduces their previous bytes exactly.
### Results
**Machine:** 8 vCPU Arm Neoverse-V2 (aarch64), 31 GiB RAM, rustc 1.99.1
**Commit:** `d66cbba`. Median with Criterion's 95% interval.
Reproduce from `native/`:
```shell
cargo bench -p datafusion-comet-shuffle --bench row_columnar --
'string_conversion|map_utf8_conversion|nullable_conversion' --save-baseline main
```
| Group | Case | 1000 rows | 10000 rows |
|---|---|---:|---:|
| string_conversion | plain_cols_10 | 436.39 µs | 3.72 ms |
| string_conversion | dictionary_cols_10 | 243.50 µs | 1.71 ms |
| string_conversion | dictionary_fallback_cols_10 | 618.75 µs | 5.32 ms |
| map_utf8_conversion | entries_10 | 432.81 µs | 3.99 ms |
| map_utf8_conversion | entries_100 | 3.70 ms | 36.06 ms |
| nullable_conversion | cols_100_null_10pct | 2.79 ms | 28.20 ms |
| nullable_conversion | map_entries_10_value_null_10pct | 473.40 µs | 3.81
ms |
| nullable_conversion | map_entries_100_value_null_10pct | 3.23 ms | 33.94
ms |
Under the default ratio, a high-cardinality string column costs 43% more
than building it plain (5.32 ms against 3.72 ms at 10,000 rows); a
low-cardinality one is 54% cheaper (1.71 ms). Every case scales about linearly
from 1,000 to 10,000 rows. All intervals are within ±0.1%.
## How are these changes tested?
This PR only adds benchmark code under `native/shuffle/benches/`; no
production code changes.
- `cargo fmt --all -- --check` and `cargo clippy -p datafusion-comet-shuffle
--all-targets -- -D warnings` pass.
- `cargo bench -p datafusion-comet-shuffle --bench row_columnar --no-run`
builds.
- Every new case runs once in Criterion's test mode: `cargo bench -p
datafusion-comet-shuffle --bench row_columnar -- --test
'string_conversion|map_utf8_conversion|nullable_conversion'`.
- Before timing, every new fixture is run through
`process_sorted_row_partition`, read back with `read_ipc_compressed`, and
compared value for value (null for null, and output type for the dictionary
cases). Corrupting a string pointer locally makes this panic before any timing.
- The whole `row_columnar` bench (50 cases, including the sorted-spill
cases) also runs in test mode, since `run_benchmark` and `build_map_row`
changed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
