Replace cardinality checks with isEmpty calls
#239
Labels
Comments
|
Will club it as part of #240 |
2 tasks
LuciferYang
added a commit
to apache/spark
that referenced
this issue
Dec 14, 2023
…etCardinality() > 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 RoaringBitmap/RoaringBitmap#239 | RoaringBitmap/RoaringBitmap#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 <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To check that a container is not empty, we routinely do a cardinality check
c.getCardinality() > 0. We should replace those with! c.isEmpty()for clarity and performance.The text was updated successfully, but these errors were encountered: