[1.19.2] Performance improvement and thread-safety review on NetworkRegistry

[BloCamLimb]

Forge Version: 1.19.2-43.1.3

Introduction: Currently, net.minecraftforge.network.NetworkRegistry uses the following declaration:

private static Map<ResourceLocation, NetworkInstance> instances = Collections.synchronizedMap(new HashMap());

I think we'd better replace it with

private static final Map<ResourceLocation, NetworkInstance> instances = new ConcurrentHashMap<>();

The performance of Java's ConcurrentHashMap is much higher than that of a synchronized map. And this map only needs a lock during registration. After #lock(), the mutex lock is just a burden, although network packets are now processed on the Netty thread.

Besides that, #createInstance() is not thread-safe when checking for registry duplicates. Checking Map#containsKey() and then Map#put() is not an atomic operation, there may be a race here. Benefitting from ConcurrentHashMap, we could use:

NetworkInstance networkInstance = new NetworkInstance(name, networkProtocolVersion, clientAcceptedVersions, serverAcceptedVersions);
NetworkInstance old = instances.put(name, networkInstance);
if (old != null) {
    throw new IllegalArgumentException("NetworkDirection Channel {" + name + "} already registered");
}

Also, the #lock field should be volatile, since #createInstance() may be called from any thread at any time.

[Jonathing]

Assigned myself to this because I think this is still worth investigating, even for 1.19.2. The one thing stopping me from making PRs is that I'm still kind of new to benchmarking. I want to be able to actually benchmark the change in maps, especially since a PR that would solve this issue would be looking into changing something that already kind of works. If @LexManos or @PaintNinja can help me learn how to make consistent benchmarks for Forge in the Discord, I'd appreciate it.