(incubator-uniffle) branch master updated: [MINOR] refactor: avoid unnecessary bitmap clone and AND (#1442)

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xianjingfeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 345048fc4 [MINOR] refactor: avoid unnecessary bitmap clone and AND (#1442)
345048fc4 is described below

commit 345048fc4da3c96e2f860ac360f2d610112a52a5
Author: Enrico Minack <gi...@enrico.minack.dev>
AuthorDate: Fri Jan 12 03:29:14 2024 +0100

    [MINOR] refactor: avoid unnecessary bitmap clone and AND (#1442)
    
    ### What changes were proposed in this pull request?
    Add note that processedBlockIds can be a superset of blockIdBitmap. Add quick check if processedBlockIds is equal to blockIdBitmap. In that case, there is no need to clone and AND the bitmaps.
    
    ### Why are the changes needed?
    The logic is equivalent but more performant when processedBlockIds is not a superset of blockIdBitmap (when no other blocks than the blockIdBitmap are retrieved).
    
    ### Does this PR introduce any user-facing change?
    No.
    
    ### How was this patch tested?
    Existing tests in ShuffleReadClientImplTest cover this.
---
 .../org/apache/uniffle/common/util/RssUtils.java   | 26 +++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java b/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
index 601f809e3..ad2b4c4fb 100644
--- a/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
+++ b/common/src/main/java/org/apache/uniffle/common/util/RssUtils.java
@@ -364,16 +364,22 @@ public class RssUtils {
 
   public static void checkProcessedBlockIds(
       Roaring64NavigableMap blockIdBitmap, Roaring64NavigableMap processedBlockIds) {
-    Roaring64NavigableMap cloneBitmap;
-    cloneBitmap = RssUtils.cloneBitMap(blockIdBitmap);
-    cloneBitmap.and(processedBlockIds);
-    if (!blockIdBitmap.equals(cloneBitmap)) {
-      throw new RssException(
-          "Blocks read inconsistent: expected "
-              + blockIdBitmap.getLongCardinality()
-              + " blocks, actual "
-              + cloneBitmap.getLongCardinality()
-              + " blocks");
+    // processedBlockIds can be a superset of blockIdBitmap,
+    // here we check that processedBlockIds has all bits of blockIdBitmap set
+    // first a quick check:
+    //   we only need to do the bitwise AND when blockIdBitmap is not equal to processedBlockIds
+    if (!blockIdBitmap.equals(processedBlockIds)) {
+      Roaring64NavigableMap cloneBitmap;
+      cloneBitmap = RssUtils.cloneBitMap(blockIdBitmap);
+      cloneBitmap.and(processedBlockIds);
+      if (!blockIdBitmap.equals(cloneBitmap)) {
+        throw new RssException(
+            "Blocks read inconsistent: expected "
+                + blockIdBitmap.getLongCardinality()
+                + " blocks, actual "
+                + cloneBitmap.getLongCardinality()
+                + " blocks");
+      }
     }
   }