lhotari commented on code in PR #25076:
URL: https://github.com/apache/pulsar/pull/25076#discussion_r2622241779
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -126,10 +126,13 @@ public boolean addMessage(long ledgerId, long entryId,
long deliverAt) {
}
long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt);
- delayedMessageMap.computeIfAbsent(timestamp, k -> new
Long2ObjectRBTreeMap<>())
- .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap())
- .add(entryId);
- delayedMessagesCount.incrementAndGet();
+ Roaring64Bitmap roaring64Bitmap = delayedMessageMap
+ .computeIfAbsent(timestamp, k -> new Long2ObjectRBTreeMap<>())
+ .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap());
+ if (!roaring64Bitmap.contains(entryId)) {
+ roaring64Bitmap.add(entryId);
Review Comment:
It looks like `.addLong` should be used in the Roaring64Bitmap API.
```suggestion
roaring64Bitmap.addLong(entryId);
```
The `.add` method works too, but the method signature takes a long array
(long...). Perhaps the compiler is able to optimize that, so it might not make
a difference.
It's unfortunate that `Roaring64Bitmap` doesn't have the `checkedAdd` method
as there is in `RoaringBitmap`. That would eliminate the need for the
`.contains` check.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
