gengliangwang opened a new pull request, #58220:
URL: https://github.com/apache/spark/pull/58220
### What changes were proposed in this pull request?
This backports #57059 ([SPARK-57980][SQL]) to `branch-4.3`, cherry-picked
from master commit
c62a58d77ab45ffc5276845d050b5b1f0d7fee8f. The PR is already on `master` and
`branch-4.x`.
When `HashAggregateExec` cannot allocate an aggregation buffer from the
in-memory hash map, its
generated code spills the map to disk: the first spill destructs the map
into a new
`UnsafeKVExternalSorter`, and later spills merge the map into the existing
sorter. This
type-independent branch is re-emitted into every HashAggregateExec
whole-stage-codegen stage.
This PR extracts it into a shared helper
`HashAggregateExec.spillHashMapToSorter(hashMap, sorter):
UnsafeKVExternalSorter` and calls it from
the generated code. Returning the sorter lets the generated code keep it in
a single mutable field.
Before (generated code, per HashAggregateExec stage):
```java
if (hashAgg_unsafeRowAggBuffer_0 == null) {
if (hashAgg_sorter_0 == null) {
hashAgg_sorter_0 = hashAgg_hashMap_0.destructAndCreateExternalSorter();
} else {
hashAgg_sorter_0.merge(hashAgg_hashMap_0.destructAndCreateExternalSorter());
}
...
```
After:
```java
if (hashAgg_unsafeRowAggBuffer_0 == null) {
hashAgg_sorter_0 =
org.apache.spark.sql.execution.aggregate.HashAggregateExec
.spillHashMapToSorter(hashAgg_hashMap_0, hashAgg_sorter_0);
...
```
The helper is a static method (it needs nothing from the plan instance), so
-- unlike routing
through the `references[]` plan object -- it adds no per-stage constant-pool
entry. Only the fully
type-independent spill-and-create block is moved. The surrounding logic that
is interleaved with the
test-only controlled-fallback counter (`resetCounter`) and the retry lookup
(which uses the
schema-specific key/hash variables) is left inline unchanged.
**Conflict resolution.** The cherry-pick conflicted in
`HashAggregateExec.scala`. On `master`,
`findOrInsertRegularHashMap` has since been restructured by the
adaptive-partial-aggregation work,
which split the spill block out into its own `spillMap` fragment reused by
two branches. `branch-4.3`
predates that and still has the single flat interpolated string with the
spill block inline. The
resolution keeps `branch-4.3`'s structure and replaces only the inline
`destructAndCreateExternalSorter`/`merge` block with the helper call; no
part of the
adaptive-partial-aggregation refactor is pulled in. The new helper and the
new test suite are
verbatim from the master commit.
### Why are the changes needed?
This is part of the umbrella SPARK-56908 (reduce the size of code generated
by whole-stage codegen).
The spill branch is real bytecode and constant-pool method-references (the
`destructAndCreateExternalSorter` and `merge` calls) that Janino cannot fold
away; collapsing it to a
single helper call compiles it once per JVM instead of re-emitting it into
every HashAggregateExec
stage. Planning all 135 TPC-DS queries produces 562 HashAggregateExec
whole-stage-codegen stages that
carry this block.
On `master`, `WholeStageCodegenSizeBenchmark` measured this as -0.4% summed
constant pool
(432,881 -> 431,233) and -0.2% source code size (21,846,001 -> 21,794,526
chars), with max method
bytecode, inner classes, and codegen fallbacks unchanged. Those numbers are
not re-measured here: the
benchmark harness (SPARK-57915) is not on `branch-4.3`.
Keeping `branch-4.3` in sync with `master` and `branch-4.x` on this file
also keeps subsequent
SPARK-56908 backports conflict-free.
### Does this PR introduce _any_ user-facing change?
No. The helper performs exactly the same spill/merge operations in the same
order as the previous
inline code; this is a pure code-organization change with no behavioral
difference.
### How was this patch tested?
Existing tests, plus the test suite that came with the original PR, run on
`branch-4.3`:
- `HashAggregateExecSuite` (added by the original PR) -- directly covers
both branches of the
extracted helper, the first-spill destruct and the later-spill merge. 2
tests, passed.
- `HashAggregationQueryWithControlledFallbackSuite`, which forces the hash
map to spill at
controlled points (`fallbackStartsAt` = 1, 2, 3) and therefore exercises
the rewritten generated
code with whole-stage codegen both on and off. 26 tests, passed.
- `sql/compile` + `sql/Test/compile` clean.
No behavior changes, so no new tests are added beyond the ones cherry-picked
with the commit.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
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]
