LuciferYang commented on PR #57059:
URL: https://github.com/apache/spark/pull/57059#issuecomment-4979887198
Nice extraction. One follow-up that could fold into this new `object
HashAggregateExec`: the tail of `outputFromRegularHashMap` in
`doProduceWithKeys` (currently `HashAggregateExec.scala:611-614`) is also a
type-independent block re-emitted per stage:
```java
$iterTerm.close();
if ($sorterTerm == null) {
$hashMapTerm.free();
}
```
All three terms are fixed-type (`mapIter` = `KVIterator<UnsafeRow,
UnsafeRow>`, `sorter` = `UnsafeKVExternalSorter`, `hashMap` =
`UnsafeFixedWidthAggregationMap`), so it can become a sibling static helper
next to `spillHashMapToSorter`:
```scala
def closeRegularHashMap(
mapIter: KVIterator[UnsafeRow, UnsafeRow],
sorter: UnsafeKVExternalSorter,
hashMap: UnsafeFixedWidthAggregationMap): Unit = {
mapIter.close()
if (sorter == null) hashMap.free()
}
```
The win is smaller than the spill branch (two methodrefs + one null check),
but unlike the spill branch (only on stages that actually fall back) this tail
is emitted on *every* grouped HashAggregate stage, so it shows up more often --
and it lands right in the object you're already adding. Want to fold it into
this PR, or spin a follow-up ticket? Happy to help either way.
--
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]
