You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "david-mollitor-db (via GitHub)" <gi...@apache.org> on 2026/09/11 02:58:00 UTC
[PR] [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add [spark]
david-mollitor-db opened a new pull request, #58727:
URL: https://github.com/apache/spark/pull/58727
### What changes were proposed in this pull request?
In `CollectFrequentItems.add` (the `collect_frequent_items` aggregate behind
`DataFrame.stat.freqItems`), the eviction branch computed:
```scala
val minCount = if (map.values.isEmpty) 0 else map.values.min
```
The `map.values.isEmpty` guard is dead: this line runs only inside the branch where the buffer is
full (`map.size >= size`), and the class enforces `require(size > 0)`, so the map always has at least
one entry there. This PR drops the guard:
```scala
// Non-empty here: this branch runs only when map.size >= size, and size > 0.
val minCount = map.values.min
```
### Why are the changes needed?
The guard is unreachable-true and not free. `scalac` does not eliminate it (it cannot prove the map
is non-empty — that is a runtime invariant), so at runtime `map.values` is evaluated and allocates a
fresh `Iterable` view wrapper on every call. This runs on the eviction path (buffer full + key miss),
the steady state for a high-cardinality column. Removing it drops the dead branch and one of the two
`map.values` allocations on that path. (HotSpot escape analysis may already scalar-replace the
short-lived wrapper in hot compiled code, but that is not guaranteed; removing it at the source level
does not rely on the JIT.)
### Does this PR introduce _any_ user-facing change?
No. The map is provably non-empty at this point, so the removed branch never executed; results are
identical.
### How was this patch tested?
Existing `DataFrameStatSuite` tests pass (including "Frequent Items" and "Frequent Items 2");
scalastyle is clean. Behavior-preserving cleanup, 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b closed pull request #58727: [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add
URL: https://github.com/apache/spark/pull/58727
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58727:
URL: https://github.com/apache/spark/pull/58727#issuecomment-5783072985
Thank you @david-mollitor-db!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add [spark]
Posted by "david-mollitor-db (via GitHub)" <gi...@apache.org>.
david-mollitor-db commented on PR #58727:
URL: https://github.com/apache/spark/pull/58727#issuecomment-5780541594
@uros-b Please consider for merge when you get a moment. Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59428][SQL] Remove a dead emptiness guard in CollectFrequentItems.add [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58727:
URL: https://github.com/apache/spark/pull/58727#issuecomment-5783069009
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/269fa37d5cbc1071c3b95a8395972c3588afca26
- merged into branch-4.x https://github.com/apache/spark/commit/d08eeb1d6d7b67ee136a2267987f13590724dcce
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org