david-mollitor-db opened a new pull request, #58921:
URL: https://github.com/apache/spark/pull/58921
### What changes were proposed in this pull request?
`UnsafeRow.getMap` / `UnsafeArrayData.getMap` create a fresh `UnsafeMapData`
per access, and its
constructor eagerly allocates two nested `UnsafeArrayData` objects (`keys`
and `values`). This
changes `UnsafeMapData` to build those key/value array views lazily:
- `keyArray()` / `valueArray()` construct (and cache) the `UnsafeArrayData`
on first use instead of
in the constructor.
- `numElements()` reads the key-array element count directly from the layout
(`Platform.getLong(baseObject, baseOffset + 8)`), which equals
`keyArray().numElements()` without
materializing any view.
- `pointTo` stores the key-array byte size as a primitive field and no
longer eagerly points the
key/value wrappers; the eager `keys.numElements() == values.numElements()`
debug assertion is
dropped.
There is no change to the storage format and no call-site changes:
`keyArray()` / `valueArray()`
keep their signatures, and serialization (`Externalizable` / Kryo),
`copy()`, and
`MapData.foreach` are unaffected (they use the accessors or operate on the
raw bytes).
### Why are the changes needed?
For a transient, count-only map access -- `size(map)`, `cardinality`, and
the `size(x) > 0` filter
that `InferFiltersFromGenerate` inserts below every non-outer `explode` /
`inline` -- the nested
`UnsafeMapData` -> `keys` / `values` object graph defeats JIT escape
analysis, so a wrapper plus two
sub-wrappers are allocated per row purely to read an element count that is
already stored inline in
the layout. `UnsafeArrayData` is a flat object, which the JIT already
scalar-replaces, so
`size(array_col)` allocates nothing; only maps carry this per-row garbage.
Making `UnsafeMapData` flat lets escape analysis scalar-replace the
transient wrapper the same way.
On a representative `size(map_col)` workload (whole-stage codegen on,
assertions disabled to match
production), the per-row `getMap` wrapper allocation dropped from ~4022 to
~94 JFR allocation
samples (~98% eliminated), and `UnsafeMapData` leaves the top of the
allocation-by-class profile.
This benefits every short-lived count-only map access, not just `size`.
### Does this PR introduce _any_ user-facing change?
No. This is an allocation / GC-pressure reduction on a hot map-access path;
results, ordering, and
nullability are unchanged. An audit of all `UnsafeMapData` creation sites
found none that reuse a
single instance across rows via `pointTo` while iterating keys/values, so
consumers that iterate
keys/values are neutral (they allocate the same wrappers, only on demand).
### How was this patch tested?
`UnsafeMapSuite` (extended with cases for count-only `numElements`, cached
key/value views, `copy`,
and an empty map), `UnsafeRowConverterSuite` (nested maps), and
`CollectionExpressionsSuite` pass, as
do `WholeStageCodegenSuite` and `GeneratorFunctionSuite` (explode exercises
the map path). The
allocation reduction was verified with JFR (`jdk.ObjectAllocationSample`) on
a `size(map_col)` query
under whole-stage codegen: `UnsafeRow.getMap` wrapper allocation fell ~4022
-> ~94 samples with the
change, with identical results.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
This pull request and its description were written by Isaac.
--
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]
