[PR] [AMORO-4274] Remove Roaring64Bitmap.isEmpty() check in CombinedDelete… [amoro]

["byungnam (via GitHub)" <gi...@apache.org>]

byungnam opened a new pull request, #4275:
URL: https://github.com/apache/amoro/pull/4275

   ## Why are the changes needed?
   
   `CombinedDeleteFilter` builds a per-file position-delete predicate that runs once per scanned record. For each record it looks up the file path in `positionMap` and checks `posSet.isEmpty()` before `posSet.contains(...)`:
   
   ```java
   Roaring64Bitmap posSet = positionMap.get(structLikeForDelete.filePath());
   if (posSet == null || posSet.isEmpty()) {
     return false;
   }
   return posSet.contains(structLikeForDelete.getPosition());
   ```
   
   The `isEmpty()` check is unnecessary and wastes CPU on the hot path:
   
   1. `positionMap` is populated only via `computeIfAbsent(...).add(...)` while reading position-delete files, so every value stored in the map is a **non-empty** `Roaring64Bitmap`. `positionMap.get(...)` therefore returns either `null` (key absent) or a non-empty bitmap — never an empty one, so the `isEmpty()` branch is unreachable.
   2. `Roaring64Bitmap.isEmpty()` is backed by `getLongCardinality()`, which traverses all high-to-low containers. It is not an O(1) check, so paying it once per record is measurable overhead on tables with large position-delete sets.
   
   Close #4274
   
   ## Brief change log
   
   - `amoro-format-iceberg` — `CombinedDeleteFilter`: in the position-delete predicate, drop the unreachable `posSet.isEmpty()` branch, keeping only the `posSet == null` guard.
   
   ## How was this patch tested?
   
   - [x] Existing tests: covered by the existing delete-filter / reader tests in `amoro-format-iceberg`; behavior is unchanged because the removed branch was unreachable.
   - [ ] Add screenshots for manual tests if appropriate
   - [x] Run test locally before making a pull request
   
   ## Documentation
   
   - Does this pull request introduce a new feature? no
   - If yes, how is the feature documented? (not applicable)
   


-- 
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: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [AMORO-4274] Remove Roaring64Bitmap.isEmpty() check in CombinedDelete… [amoro]

["zhoujinsong (via GitHub)" <gi...@apache.org>]

zhoujinsong merged PR #4275:
URL: https://github.com/apache/amoro/pull/4275


-- 
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: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org