Improve client packet interception by about 37%.

ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ProxyPacketInjector.java

@@ -23,13 +23,13 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-
 import org.bukkit.entity.Player;
 
 import net.sf.cglib.proxy.Callback;
 import net.sf.cglib.proxy.Enhancer;
 import net.sf.cglib.proxy.Factory;
+import net.sf.cglib.proxy.CallbackFilter;
+import net.sf.cglib.proxy.NoOp;
 
 import com.comphenix.protocol.Packets;
 import com.comphenix.protocol.error.ErrorReporter;
@@ -39,6 +39,8 @@ import com.comphenix.protocol.injector.ListenerInvoker;
 import com.comphenix.protocol.injector.player.PlayerInjectionHandler;
 import com.comphenix.protocol.reflect.FieldUtils;
 import com.comphenix.protocol.reflect.FuzzyReflection;
+import com.comphenix.protocol.reflect.MethodInfo;
+import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
 import com.comphenix.protocol.utility.MinecraftReflection;
 
 /**
@@ -47,6 +49,14 @@ import com.comphenix.protocol.utility.MinecraftReflection;
  * @author Kristian
  */
 class ProxyPacketInjector implements PacketInjector {
+	/**
+	 * Matches the readPacketData(DataInputStream) method in Packet.
+	 */
+	private static FuzzyMethodContract readPacket = FuzzyMethodContract.newBuilder().
+			returnTypeVoid().
+			parameterExactType(DataInputStream.class).
+			parameterCount(1).
+			build();
 	
 	// The "put" method that associates a packet ID with a packet class
 	private static Method putMethod;
@@ -61,12 +71,12 @@ class ProxyPacketInjector implements PacketInjector {
 	// Allows us to determine the sender
 	private PlayerInjectionHandler playerInjection;
 	
-	// Allows us to look up read packet injectors
-	private Map<Integer, ReadPacketModifier> readModifier;
-	
 	// Class loader
 	private ClassLoader classLoader;
 	
+	// Share callback filter
+	private CallbackFilter filter;
+			
 	public ProxyPacketInjector(ClassLoader classLoader, ListenerInvoker manager, 
 						  PlayerInjectionHandler playerInjection, ErrorReporter reporter) throws IllegalAccessException {
 		
@@ -74,7 +84,6 @@ class ProxyPacketInjector implements PacketInjector {
 		this.manager = manager;
 		this.playerInjection = playerInjection;
 		this.reporter = reporter;
-		this.readModifier = new ConcurrentHashMap<Integer, ReadPacketModifier>();
 		initialize();
 	}
 	
@@ -85,11 +94,9 @@ class ProxyPacketInjector implements PacketInjector {
 	 */
 	@Override
 	public void undoCancel(Integer id, Object packet) {
-		ReadPacketModifier modifier = readModifier.get(id);
-		
 		// See if this packet has been cancelled before
-		if (modifier != null && modifier.hasCancelled(packet)) {
-			modifier.removeOverride(packet);
+		if (ReadPacketModifier.hasCancelled(packet)) {
+			ReadPacketModifier.removeOverride(packet);
 		}
 	}
 	
@@ -137,18 +144,34 @@ class ProxyPacketInjector implements PacketInjector {
 			throw new IllegalStateException("Packet " + packetID + " has already been injected.");
 		}
 		
+		if (filter == null) {
+			filter = new CallbackFilter() {
+				@Override
+				public int accept(Method method) {
+					// Skip methods defined in Object
+					if (method.getDeclaringClass().equals(Object.class))
+						return 0;
+					else if (readPacket.isMatch(MethodInfo.fromMethod(method), null))
+						return 1;
+					else
+						return 2;
+				}
+			};
+		}
+		
 		// Subclass the specific packet class
 		ex.setSuperclass(old);
-		ex.setCallbackType(ReadPacketModifier.class);
+		ex.setCallbackFilter(filter);
+		ex.setCallbackTypes(new Class<?>[] { NoOp.class, ReadPacketModifier.class, ReadPacketModifier.class });
 		ex.setClassLoader(classLoader);
 		Class proxy = ex.createClass();
 		
-		// Create the proxy handler
-		ReadPacketModifier modifier = new ReadPacketModifier(packetID, this, reporter);
-		readModifier.put(packetID, modifier);
+		// Create the proxy handlers
+		ReadPacketModifier modifierReadPacket = new ReadPacketModifier(packetID, this, reporter, true);
+		ReadPacketModifier modifierRest = new ReadPacketModifier(packetID, this, reporter, false);
 
 		// Add a static reference
-		Enhancer.registerStaticCallbacks(proxy, new Callback[] { modifier });
+		Enhancer.registerStaticCallbacks(proxy, new Callback[] { NoOp.INSTANCE, modifierReadPacket, modifierRest });
 		
 		try {
 			// Override values
@@ -184,7 +207,6 @@ class ProxyPacketInjector implements PacketInjector {
 			
 			putMethod.invoke(intHashMap, packetID, old);
 			previous.remove(packetID);
-			readModifier.remove(packetID);
 			registry.remove(proxy);
 			overwritten.remove(packetID);
 			return true;

ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ReadPacketModifier.java

@@ -19,23 +19,17 @@ package com.comphenix.protocol.injector.packet;
 
 import java.io.DataInputStream;
 import java.lang.reflect.Method;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.Map;
-import java.util.WeakHashMap;
 
 import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.events.PacketContainer;
 import com.comphenix.protocol.events.PacketEvent;
+import com.google.common.collect.MapMaker;
 
 import net.sf.cglib.proxy.MethodInterceptor;
 import net.sf.cglib.proxy.MethodProxy;
 
 class ReadPacketModifier implements MethodInterceptor {
-
-	@SuppressWarnings("rawtypes")
-	private static Class[] parameters = { DataInputStream.class };
-
 	// A cancel marker
 	private static final Object CANCEL_MARKER = new Object();
 	
@@ -46,20 +40,24 @@ class ReadPacketModifier implements MethodInterceptor {
 	// Report errors
 	private ErrorReporter reporter;
 	
-	// Whether or not a packet has been cancelled
-	private static Map<Object, Object> override = Collections.synchronizedMap(new WeakHashMap<Object, Object>());
+	// If this is a read packet data method
+	private boolean isReadPacketDataMethod;
 	
-	public ReadPacketModifier(int packetID, ProxyPacketInjector packetInjector, ErrorReporter reporter) {
+	// Whether or not a packet has been cancelled
+	private static Map<Object, Object> override = new MapMaker().weakKeys().makeMap();
+	
+	public ReadPacketModifier(int packetID, ProxyPacketInjector packetInjector, ErrorReporter reporter, boolean isReadPacketDataMethod) {
 		this.packetID = packetID;
 		this.packetInjector = packetInjector;
 		this.reporter = reporter;
+		this.isReadPacketDataMethod = isReadPacketDataMethod;
 	}
 	
 	/**
 	 * Remove any packet overrides.
 	 * @param packet - the packet to rever
 	 */
-	public void removeOverride(Object packet) {
+	public static void removeOverride(Object packet) {
 		override.remove(packet);
 	}
 	
@@ -68,7 +66,7 @@ class ReadPacketModifier implements MethodInterceptor {
 	 * @param packet - the given packet.
 	 * @return Overriden object.
 	 */
-	public Object getOverride(Object packet) {
+	public static Object getOverride(Object packet) {
 		return override.get(packet);
 	}
 
@@ -77,23 +75,15 @@ class ReadPacketModifier implements MethodInterceptor {
 	 * @param packet - the packet to check.
 	 * @return TRUE if it has been cancelled, FALSE otherwise.
 	 */
-	public boolean hasCancelled(Object packet) {
+	public static boolean hasCancelled(Object packet) {
 		return getOverride(packet) == CANCEL_MARKER;
 	}
 
 	@Override
 	public Object intercept(Object thisObj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
-		
-		Object returnValue = null;
-		String methodName = method.getName();
-
-		// We always pass these down (otherwise, we'll end up with a infinite loop)
-		if (methodName.equals("hashCode") || methodName.equals("equals") || methodName.equals("toString")) {
-			return proxy.invokeSuper(thisObj, args);
-		}
-		
 		// Atomic retrieval
 		Object overridenObject = override.get(thisObj);
+		Object returnValue = null;
 		
 		if (overridenObject != null) {
 			// This packet has been cancelled
@@ -111,9 +101,7 @@ class ReadPacketModifier implements MethodInterceptor {
 		}
 		
 		// Is this a readPacketData method?
-		if (returnValue == null && 
-				Arrays.equals(method.getParameterTypes(), parameters)) {
-			
+		if (isReadPacketDataMethod) {
 			try {
 				// We need this in order to get the correct player
 				DataInputStream input = (DataInputStream) args[0];
@@ -134,10 +122,9 @@ class ReadPacketModifier implements MethodInterceptor {
 				}
 			} catch (Throwable e) {
 				// Minecraft cannot handle this error
-				reporter.reportDetailed(this, "Cannot handle clienet packet.", e, args[0]);
+				reporter.reportDetailed(this, "Cannot handle client packet.", e, args[0]);
 			}
 		}
-		
 		return returnValue;
 	}