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 <[email protected]>
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 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
---
.../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: [email protected]
For additional commands, e-mail: [email protected]
