david-mollitor-db opened a new pull request, #58726:
URL: https://github.com/apache/spark/pull/58726
### What changes were proposed in this pull request?
`CollectFrequentItems` (the `collect_frequent_items` aggregate behind
`DataFrame.stat.freqItems`)
maintains an item → count buffer (`mutable.Map[Any, Long]`) and updates it
once per input value in
the private `add` method (from both `update` and `merge`). Its "already
tracked" path was:
```scala
if (map.contains(key)) {
map(key) += count
}
```
On a hit this does **three** hash lookups on the same key — `contains`, then
`apply` and `update`
(the desugaring of `map(key) += count`). This PR looks the key up once and
branches on the result,
dropping the hit path to **two** lookups (`get` + `update`):
```scala
map.get(key) match {
case Some(existing) => map(key) = existing + count
case None => // unchanged: insert-if-room, else the bounded-counter
eviction
...
}
```
The miss path is unchanged (one `get` in place of one `contains`), and the
eviction logic in the
`None` branch is untouched.
### Why are the changes needed?
Scala's `HashMap` re-hashes the key on every operation, so the old hit path
recomputed
`key.hashCode()` three times per increment. For a `UTF8String` key — the
common `freqItems` case of
high-cardinality string columns — `hashCode` is an uncached Murmur3 pass
over the whole value (and
`equals` on a collision is a full byte compare), so the redundant lookup is
O(key length). `add`
runs per input value, and once the counter map saturates (its steady state)
the hit path dominates.
Two lookups is the floor for incrementing an existing key whose value is an
immutable `Long` (a read
plus a write), so this removes the one avoidable lookup without changing the
algorithm.
### Does this PR introduce _any_ user-facing change?
No. `map(key) = existing + count` is exactly the previous `map(key) +=
count`; results are identical
for all inputs.
### How was this patch tested?
Existing `DataFrameStatSuite` tests pass (including "Frequent Items" and
"Frequent Items 2");
scalastyle is clean. This is a behavior-preserving internal refactor, so no
new test was added.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8
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]
