MemoryConnection has an extremly poor collection choice

I'm sure these variables have other names internal to minecraft, but i'll refer to them by their MCP name.

readPacketCache is a synchronized array list. Items are removed from it in a read function by readPacketCache.remove(0). This a) blocks all access to the object and b) causes every element to be copied into a lower index, making this an order N operation.
This makes reading N items from the "queue" an O(N^2) operation, holding a lock for each O(N) operation.
This means that under high load, insert to the queue is an O(N) operation, with a high overhead, while it waits for the lock.

the java concurrentLinkedQueue is an unbounded queue designed for exactly this situation with constant time, lock-free push/pull concurrently (several threads can push and pull at the same time without causing any of them to block).

Changing this code does not cause any other code to need to be changed, as it is a private variable, and INetworkManager's are ALWAYS accessed via their interface, because it could be a Memory or a TCP connection, and the code doesn't know which.

this is cross-posted as a patch to fml - https://github.com/MinecraftForge/FML/issues/189

I have tested this in my MCP install, with both buildcraft and other mods active, with no problems.

package net.minecraft.network;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;

import net.minecraft.network.packet.NetHandler;
import net.minecraft.network.packet.Packet;

public class MemoryConnection implements INetworkManager
{
    private static final SocketAddress mySocketAddress = new InetSocketAddress("127.0.0.1", 0);
    private final Queue<Packet> readPacketCache = new ConcurrentLinkedQueue<Packet>();
    private MemoryConnection pairedConnection;
    private NetHandler myNetHandler;

    /** set to true by {server,network}Shutdown */
    private boolean shuttingDown = false;
    private String shutdownReason = "";
    private Object[] field_74439_g;
    private boolean gamePaused = false;

<snip - unchanged functions removed>
    /**
     * Checks timeouts and processes all pending read packets.
     */
    public void processReadPackets()
    {
        int var1 = 2500;

        while (var1-- >= 0 && !this.readPacketCache.isEmpty())
        {
            Packet var2 = this.readPacketCache.poll();
            var2.processPacket(this.myNetHandler);
        }

        if (this.readPacketCache.size() > var1)
        {
            System.out.println("Memory connection overburdened; after processing 2500 packets, we still have " + this.readPacketCache.size() + " to go!");
        }

        if (this.shuttingDown && this.readPacketCache.isEmpty())
        {
            this.myNetHandler.handleErrorMessage(this.shutdownReason, this.field_74439_g);
        }
    }
<snip - more unchanged functions removed>
}

--

@@ -7,7 +7,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
 import net.minecraft.network.packet.NetHandler;
 import net.minecraft.network.packet.Packet;
@@ -16,5 +16,5 @@
 {
     private static final SocketAddress mySocketAddress = new InetSocketAddress("127.0.0.1", 0);
-    private final List readPacketCache = Collections.synchronizedList(new ArrayList());
+    private final Queue<Packet> readPacketCache = new ConcurrentLinkedQueue<Packet>();
     private MemoryConnection pairedConnection;
     private NetHandler myNetHandler;
@@ -77,5 +77,5 @@
         while (var1-- >= 0 && !this.readPacketCache.isEmpty())
         {
-            Packet var2 = (Packet)this.readPacketCache.remove(0);
+            Packet var2 = this.readPacketCache.poll();
             var2.processPacket(this.myNetHandler);
         }