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

Author: BloCamLimbCreated Aug 30, 2022Updated Apr 2, 2026
LabelsPerformanceCleanupTriage1.19

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.

Source: MinecraftForge/MinecraftForge