ConcurrentHashSet is redundant with ConcurrentHashMap#newKeySet()

[magicprinc]

It seems that io.vertx.core.impl.ConcurrentHashSet is a copy of JDK's Collections.newSetFromMap. The Best Code is No Code At All. It is probably a good idea to review and replace this class with two utility methods:

public static <E> Set<E> concurrentHashSet (){
  return Collections.newSetFromMap(new ConcurrentHashMap<>());
}

public static <E> Set<E> concurrentHashSet (int initialSize){
  return Collections.newSetFromMap(new ConcurrentHashMap<>(initialSize));
}

PR: https://github.com/eclipse-vertx/vert.x/pull/4852

[He-Pin]

Better to use ConcurrentHashMap.newKeySet

[tsegismont]

You're welcome to contribute a PR for this, as well as in the impacted projects:

• https://github.com/search?q=org%3Avert-x3+ConcurrentHashSet&type=code
• https://github.com/search?q=org%3Aeclipse-vertx+ConcurrentHashSet&type=code

[magicprinc]

I would like to, but I still have failed tests when I build vert.x-core locally on my Windows10/JDK17. So I am not ready yet... :-(

BTW, I would enjoy if you find my other PR useful: https://github.com/eclipse-vertx/vert.x/pull/4844 https://github.com/eclipse-vertx/vert.x/pull/4847 https://github.com/eclipse-vertx/vert.x/pull/4850

They have more benefits than ConcurrentHashSet removial.

[He-Pin]

If you don't mind, i can do this too:), @magicprinc ,a nice find!

[magicprinc]

@tsegismont Anyway. Without tests :-) https://github.com/eclipse-vertx/vert.x/pull/4852

@He-Pin Could you test it? And add test for concurrentHashSet implementations too?

[magicprinc]

1) I have replaced method with its body. 2) Fix in vertx-service-discovery: https://github.com/vert-x3/vertx-service-discovery/pull/172 3) Fix in vertx-mqtt: https://github.com/vert-x3/vertx-mqtt/pull/243

@tsegismont All your tasks are done!

[tsegismont]

@magicprinc there's one more usage in vertx-mqtt, can you please take care of it before we merge this PR?

[magicprinc]

@tsegismont I am sorry, but I can't find it. Could you name a class with it? /

[magicprinc]

GitHub search shows 2 files, and both are fixed in my PR https://github.com/vert-x3/vertx-mqtt/pull/243

[tsegismont]

Here are the references in MQTT:

https://github.com/vert-x3/vertx-mqtt/blob/master/src/test/java/io/vertx/mqtt/test/server/MqttServerTest.java#L84 https://github.com/vert-x3/vertx-mqtt/blob/master/src/test/java/io/vertx/mqtt/test/server/MqttServerWebSocketTest.java#L78

[magicprinc]

@tsegismont They are both fixed in PR https://github.com/vert-x3/vertx-mqtt/pull/243 Have you seen this PR?

[tsegismont]

For some reason, I wasn't subscribed to all notifications on vertx-mqtt

This is why I missed the PR, sorry.

[tsegismont]

Closed by #4852