BewareMyPower commented on issue #23215:
URL: https://github.com/apache/pulsar/issues/23215#issuecomment-2360658773
> It might be a big changes. Is it possible to remain ConcurrentOpenHashMap
interface, and use ConcurrentHashMap as ConcurrentOpenHashMap interface
realization? So that we may only need to modify ConcurrentOpenHashMap file.
It will produce much more garbage code in Pulsar.
The idea to reduce code changes seems good, but the `ConcurrentOpenHashMap`
is not fully compatible with `ConcurrentHashMap`. Let me count one by one.
1. Constructor changes.
```java
ConcurrentOpenHashMap.<TopicName,
PersistentOfflineTopicStats>newBuilder()./* ...
*/build();
```
You still need to keep the builder with all meaningless parameters like
`concurrencyLevel`, which is confusing.
2. Non-standard `keys()` and `values()` APIs
```java
public List<K> keys() {
List<K> keys = new ArrayList<>((int) size());
forEach((key, value) -> keys.add(key));
return keys;
}
public List<V> values() {
List<V> values = new ArrayList<>((int) size());
forEach((key, value) -> values.add(value));
return values;
}
```
`ConcurrentHashMap` does not provide these methods because of the overhead
of copying keys or values into a new collection.
3. The `removeNullValue` method, which is really wrong and should not exist.
So just let me split the huge PR into multiple relatively small PRs.
--
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]
