This is an automated email from the ASF dual-hosted git repository. yangjie01 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push: new ab9ca96ca0ae [SPARK-46401][CORE] Use `!isEmpty()` on `RoaringBitmap` instead of `getCardinality() > 0` in `RemoteBlockPushResolver` ab9ca96ca0ae is described below commit ab9ca96ca0aeee0e775687ced4da87a0ed05903e Author: yangjie01 <yangji...@baidu.com> AuthorDate: Thu Dec 14 17:17:21 2023 +0800 [SPARK-46401][CORE] Use `!isEmpty()` on `RoaringBitmap` instead of `getCardinality() > 0` in `RemoteBlockPushResolver` ### What changes were proposed in this pull request? This pr use `!mapTracker.isEmpty()` instead of `mapTracker.getCardinality() > 0` in `RemoteBlockPushResolver ` to reduce some computational costs, as the `getCardinality()` method triggers a recount every time it is called. - [getCardinality](https://github.com/RoaringBitmap/RoaringBitmap/blame/578a5162fa7c80be3988bfdbf9abe06ae124722c/RoaringBitmap/src/main/java/org/roaringbitmap/RoaringBitmap.java#L1884-L1901) ```java /** * Returns the number of distinct integers added to the bitmap (e.g., number of bits set). * * return the cardinality */ Override public long getLongCardinality() { long size = 0; for (int i = 0; i < this.highLowContainer.size(); i++) { size += this.highLowContainer.getContainerAtIndex(i).getCardinality(); } return size; } Override public int getCardinality() { return (int) getLongCardinality(); } ``` - [isEmpty](https://github.com/RoaringBitmap/RoaringBitmap/blob/578a5162fa7c80be3988bfdbf9abe06ae124722c/RoaringBitmap/src/main/java/org/roaringbitmap/RoaringBitmap.java#L2218-L2226) ```java /** * Checks whether the bitmap is empty. * * return true if this bitmap contains no set bit */ Override public boolean isEmpty() { return highLowContainer.size() == 0; } ``` This change is refer to https://github.com/RoaringBitmap/RoaringBitmap/issues/239 | https://github.com/RoaringBitmap/RoaringBitmap/pull/684 ### Why are the changes needed? Use a more appropriate API to reduce the computational cost of empty-checking for RoaringBitmap. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions ### Was this patch authored or co-authored using generative AI tooling? No Closes #44347 from LuciferYang/bitmap-isEmpty. Authored-by: yangjie01 <yangji...@baidu.com> Signed-off-by: yangjie01 <yangji...@baidu.com> --- .../java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java index 2f8b5b99746b..5f9576843b47 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java @@ -809,7 +809,7 @@ public class RemoteBlockPushResolver implements MergedShuffleFileManager { msg.shuffleMergeId, partition.reduceId); // This can throw IOException which will marks this shuffle partition as not merged. partition.finalizePartition(); - if (partition.mapTracker.getCardinality() > 0) { + if (!partition.mapTracker.isEmpty()) { bitmaps.add(partition.mapTracker); reduceIds.add(partition.reduceId); sizes.add(partition.getLastChunkOffset()); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org