codelipenghui commented on code in PR #21107:
URL: https://github.com/apache/pulsar/pull/21107#discussion_r1315906178
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentOpenHashSet.java:
##########
@@ -598,8 +598,14 @@ static final <K> long hash(K key) {
return hash;
}
- static final int signSafeMod(long n, int max) {
- return (int) n & (max - 1);
+ static final int signSafeMod(long dividend, int divisor) {
+ int mod = (int) (dividend % divisor);
+
+ if (mod < 0) {
+ mod += divisor;
+ }
+
+ return mod;
Review Comment:
Can we move to a Utility Class for computing the sign safe mod?
We have a similar [Unitity
Class](https://github.com/apache/pulsar/blob/c675a3dd6b4a3a6009d2a10c38ee79d8219b1920/pulsar-client/src/main/java/org/apache/pulsar/client/util/MathUtils.java#L24)
in pulsar-client, not pulsar-client-api.
Is it better to move it to pulsar-common to reduce the duplicated code?
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/collections/ConcurrentOpenHashSet.java:
##########
@@ -598,8 +598,14 @@ static final <K> long hash(K key) {
return hash;
}
- static final int signSafeMod(long n, int max) {
- return (int) n & (max - 1);
+ static final int signSafeMod(long dividend, int divisor) {
+ int mod = (int) (dividend % divisor);
+
+ if (mod < 0) {
+ mod += divisor;
+ }
+
+ return mod;
Review Comment:
@lhotari And, could you please provide more context about
> the impact of the bug has been that the implementations have been rather
unoptimal because of of frequent collisions in storing the item to the
underlying array. Essentially the hash bucket algorithm hasn't been properly
used.
The HashMap in JDK is also used the same way to calculate the table index.
Why `%` is better than `&` in this case to avoid the frequent collisions?
Here is the code snippet in JDK 17:
```java
if ((p = tab[i = (n - 1) & hash]) == null)
tab[i] = newNode(hash, key, value, null);
```
--
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]
