viirya opened a new pull request, #56156:
URL: https://github.com/apache/spark/pull/56156
### What changes were proposed in this pull request?
Follow-up to #56072 (SPARK-57024) and #56082 (SPARK-57036).
`WritableColumnVector.putNotNulls(rowId, count)` clears a run of the
nulls bitmap. It is called once per batch from
`WritableColumnVector.reset()` (when `numNulls > 0`) and from the
`appendNotNulls()` path. Both OnHeap and OffHeap implementations are
per-element byte loops — the same degenerate pattern that SPARK-57024
fixed for `putNulls` and SPARK-57036 fixed for `putBytes` / `putBooleans`.
This change applies the same intrinsic substitutions:
| Method | Substitution |
| --- | --- |
| `OnHeapColumnVector.putNotNulls(rowId, count)` | `Arrays.fill(byte[], ...,
(byte) 0)` |
| `OffHeapColumnVector.putNotNulls(rowId, count)` |
`Platform.setMemory(addr, (byte) 0, count)` with small-count fallback |
The OffHeap variant reuses the existing `SET_MEMORY_THRESHOLD = 128`
constant introduced for `putNulls` in SPARK-57024. Below the threshold,
an inline byte loop avoids the JNI fixed cost of `Unsafe.setMemory`; at
or above, `setMemory` dominates.
Also extends `WritableColumnVectorBulkFillBenchmark` (added in
SPARK-57042 / #56084, extended for `putNulls` in SPARK-57036 / #56082)
with a `putNotNulls` case mirroring the existing `putNulls` case, so
this change has direct before/after numbers in the benchmark.
### Why are the changes needed?
`putNotNulls` runs once per batch in the vectorized reader path
(`WritableColumnVector.reset()` is called from `ColumnarBatch.close()`
and via `releaseColumns()`, both of which happen at batch boundaries).
The original per-byte loop is meaningfully slower than a `memset` for
the typical batch capacity (4096 elements).
Measured on Apple M4 Max + OpenJDK 21 via
`WritableColumnVectorBulkFillBenchmark` (the `putNotNulls` case added
by this PR), Rate (M elements/s):
| count | OnHeap baseline | OnHeap patched | OffHeap baseline | OffHeap
patched | OffHeap delta |
| ------: | --------------: | -------------: | ---------------: |
--------------: | ------------: |
| 1 | 228 | 228 | 182 | 218
| +20% |
| 8 | 1,548 | 1,524 | 647 | 1,404
| +117% (within small-count fallback) |
| 64 | 11,651 | 11,155 | 990 | 3,480
| +3.5x (still below threshold; JIT noise) |
| 512 | 27,060 | 26,491 | 1,055 | 12,205
| **+11.6x** |
| 4,096 | 78,952 | 80,531 | 1,029 | 30,784
| **+29.9x** |
| 65,536 | 241,689 | 238,644 | 1,000 | 42,647
| **+42.7x** |
OnHeap is at parity across the count sweep: the C2 compiler already
auto-vectorizes the original byte loop near the byte memory-bandwidth
ceiling on this hardware, so `Arrays.fill` adds no measurable
throughput; the change is kept for idiomatic consistency with the
other byte-fill methods (`putNulls`, `putBytes`, `putBooleans`) which
all use `Arrays.fill`.
GHA `Run benchmarks` numbers will land on this PR shortly via the
auto-commit workflow (same flow as #56072 / #56082).
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing tests; no behavior change. Ran locally:
- `VectorizedRleValuesReaderSuite`
- `ColumnVectorSuite`
- `ColumnarBatchSuite`
- `ParquetIOSuite`
237 tests, all pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.7)
--
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]
