From bb8bec907c8c29ad3fae8521e7943bf639e38389 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 14 Sep 2012 12:41:57 +0200 Subject: [PATCH] Added a priorty to every listener. Improved performance. --- .../protocol/events/ListenerPriority.java | 51 +++++ .../protocol/events/PacketAdapter.java | 18 ++ .../protocol/events/PacketListener.java | 6 + .../injector/ConcurrentListenerMultimap.java | 209 ++++++++++++++++++ .../injector/PacketFilterManager.java | 102 ++++----- .../protocol/injector/PlayerInjector.java | 8 +- .../protocol/injector/StructureCache.java | 2 +- 7 files changed, 340 insertions(+), 56 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/events/ListenerPriority.java create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/ConcurrentListenerMultimap.java diff --git a/ProtocolLib/src/com/comphenix/protocol/events/ListenerPriority.java b/ProtocolLib/src/com/comphenix/protocol/events/ListenerPriority.java new file mode 100644 index 00000000..f3d27dfd --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/events/ListenerPriority.java @@ -0,0 +1,51 @@ +package com.comphenix.protocol.events; + +/** + * Represents a packet event priority, similar to the Bukkit EventPriority. + * + * @author Kristian + */ +public enum ListenerPriority { + /** + * Event call is of very low importance and should be ran first, to allow + * other plugins to further customise the outcome. + */ + LOWEST(0), + /** + * Event call is of low importance. + */ + LOW(1), + /** + * Event call is neither important or unimportant, and may be ran normally. + */ + NORMAL(2), + /** + * Event call is of high importance. + */ + HIGH(3), + /** + * Event call is critical and must have the final say in what happens to the + * event. + */ + HIGHEST(4), + /** + * Event is listened to purely for monitoring the outcome of an event. + *

+ * No modifications to the event should be made under this priority. + */ + MONITOR(5); + + private final int slot; + + private ListenerPriority(int slot) { + this.slot = slot; + } + + /** + * A low slot represents a low priority. + * @return Integer representation of this priorty. + */ + public int getSlot() { + return slot; + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/events/PacketAdapter.java b/ProtocolLib/src/com/comphenix/protocol/events/PacketAdapter.java index 4e3ff3ca..d5aea595 100644 --- a/ProtocolLib/src/com/comphenix/protocol/events/PacketAdapter.java +++ b/ProtocolLib/src/com/comphenix/protocol/events/PacketAdapter.java @@ -34,6 +34,7 @@ public abstract class PacketAdapter implements PacketListener { protected Plugin plugin; protected Set packetsID; protected ConnectionSide connectionSide; + protected ListenerPriority listenerPriority; /** * Initialize a packet listener. @@ -42,9 +43,21 @@ public abstract class PacketAdapter implements PacketListener { * @param packets - the packet IDs the listener is looking for. */ public PacketAdapter(Plugin plugin, ConnectionSide connectionSide, Integer... packets) { + this(plugin, connectionSide, ListenerPriority.NORMAL, packets); + } + + /** + * Initialize a packet listener. + * @param plugin - the plugin that spawned this listener. + * @param connectionSide - the packet type the listener is looking for. + * @param listenerPriority - the event priority. + * @param packets - the packet IDs the listener is looking for. + */ + public PacketAdapter(Plugin plugin, ConnectionSide connectionSide, ListenerPriority listenerPriority, Integer... packets) { this.plugin = plugin; this.connectionSide = connectionSide; this.packetsID = Sets.newHashSet(packets); + this.listenerPriority = listenerPriority; } @Override @@ -72,6 +85,11 @@ public abstract class PacketAdapter implements PacketListener { return plugin; } + @Override + public ListenerPriority getListenerPriority() { + return listenerPriority; + } + /** * Retrieves the name of the plugin that has been associated with the listener. * @return Name of the associated plugin. diff --git a/ProtocolLib/src/com/comphenix/protocol/events/PacketListener.java b/ProtocolLib/src/com/comphenix/protocol/events/PacketListener.java index e263d15b..682b3780 100644 --- a/ProtocolLib/src/com/comphenix/protocol/events/PacketListener.java +++ b/ProtocolLib/src/com/comphenix/protocol/events/PacketListener.java @@ -48,6 +48,12 @@ public interface PacketListener { */ public ConnectionSide getConnectionSide(); + /** + * Retrieve the priority in the execution order of this packet listener. Highest priority will be executed last. + * @return Execution order in terms of priority. + */ + public ListenerPriority getListenerPriority(); + /** * Set of packet ids we expect to recieve. * @return Packets IDs. diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ConcurrentListenerMultimap.java b/ProtocolLib/src/com/comphenix/protocol/injector/ConcurrentListenerMultimap.java new file mode 100644 index 00000000..9fac1868 --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ConcurrentListenerMultimap.java @@ -0,0 +1,209 @@ +package com.comphenix.protocol.injector; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.Iterator; +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.logging.Level; +import java.util.logging.Logger; + +import com.comphenix.protocol.events.PacketAdapter; +import com.comphenix.protocol.events.PacketEvent; +import com.comphenix.protocol.events.PacketListener; +import com.google.common.base.Objects; +import com.google.common.primitives.Ints; + +/** + * A thread-safe implementation of a listener multimap. + * + * @author Kristian + */ +public class ConcurrentListenerMultimap { + + // The core of our map + protected ConcurrentMap> listeners = + new ConcurrentHashMap>(); + + /** + * Adds a listener to its requested list of packet recievers. + * @param listener - listener with a list of packets to recieve notifcations for. + */ + public void addListener(PacketListener listener) { + for (Integer packetID : listener.getPacketsID()) { + addListener(packetID, listener); + } + } + + // Add the listener to a specific packet notifcation list + private void addListener(Integer packetID, PacketListener listener) { + + SortedArrayList list = listeners.get(packetID); + + // We don't want to create this for every lookup + if (list == null) { + // It would be nice if we could use a PriorityBlockingQueue, but it doesn't preseve iterator order, + // which is a essential feature for our purposes. + final SortedArrayList value = new SortedArrayList(new Comparator() { + @Override + public int compare(PacketListener o1, PacketListener o2) { + // This ensures that lower priority listeners are executed first + return Ints.compare(o1.getListenerPriority().getSlot(), + o2.getListenerPriority().getSlot()); + } + }); + + list = listeners.putIfAbsent(packetID, value); + + // We may end up creating multiple multisets, but we'll agree + // on the one to use. + if (list == null) { + list = value; + } + } + + // Careful when modifying the set + synchronized(list) { + list.insertSorted(listener); + } + } + + /** + * Removes the given listener from the packet event list. + * @param listener - listener to remove. + * @return Every packet ID that was removed due to no listeners. + */ + public List removeListener(PacketListener listener) { + + List removedPackets = new ArrayList(); + + // Again, not terribly efficient. But adding or removing listeners should be a rare event. + for (Integer packetID : listener.getPacketsID()) { + + SortedArrayList list = listeners.get(packetID); + + // Remove any listeners + if (list != null) { + synchronized(list) { + // Don't remove from newly created lists + if (list.size() > 0) { + list.removeAll(listener); + + if (list.size() == 0) { + listeners.remove(packetID); + removedPackets.add(packetID); + } + } + } + } + + // Move on to the next + } + + return removedPackets; + } + + /** + * Invokes the given packet event for every registered listener. + * @param logger - the logger that will be used to inform about listener exceptions. + * @param event - the packet event to invoke. + */ + public void invokePacketRecieving(Logger logger, PacketEvent event) { + SortedArrayList list = listeners.get(event.getPacketID()); + + if (list == null) + return; + + // We have to be careful. Cannot modify the underlying list when sending notifications. + synchronized (list) { + for (PacketListener listener : list) { + try { + listener.onPacketReceiving(event); + } catch (Exception e) { + // Minecraft doesn't want your Exception. + logger.log(Level.SEVERE, + "Exception occured in onPacketReceiving() for " + PacketAdapter.getPluginName(listener), e); + } + } + } + } + + /** + * Invokes the given packet event for every registered listener. + * @param logger - the logger that will be used to inform about listener exceptions. + * @param event - the packet event to invoke. + */ + public void invokePacketSending(Logger logger, PacketEvent event) { + SortedArrayList list = listeners.get(event.getPacketID()); + + if (list == null) + return; + + synchronized (list) { + for (PacketListener listener : list) { + try { + listener.onPacketSending(event); + } catch (Exception e) { + // Minecraft doesn't want your Exception. + logger.log(Level.SEVERE, + "Exception occured in onPacketReceiving() for " + PacketAdapter.getPluginName(listener), e); + } + } + } + } + + /** + * An implicitly sorted array list that preserves insertion order and maintains duplicates. + * + * Note that only the {@link insertSorted} method will update the list correctly, + * @param - type of the sorted list. + */ + private class SortedArrayList implements Iterable { + + private Comparator comparator; + private List list = new ArrayList(); + + public SortedArrayList(Comparator comparator) { + this.comparator = comparator; + } + + /** + * Inserts the given element in the proper location. + * @param value - element to insert. + */ + public void insertSorted(T value) { + list.add(value); + for (int i = list.size() - 1; i > 0 && comparator.compare(value, list.get(i-1)) < 0; i--) { + T tmp = list.get(i); + list.set(i, list.get(i-1)); + list.set(i-1, tmp); + } + } + + /** + * Removes every instance of the given element. + * @param element - element to remove. + */ + public void removeAll(T element) { + for (Iterator it = list.iterator(); it.hasNext(); ) { + if (Objects.equal(it.next(), element)) { + it.remove(); + } + } + } + + /** + * Retrieve the size of the list. + * @return Size of the list. + */ + public int size() { + return list.size(); + } + + @Override + public Iterator iterator() { + return list.iterator(); + } + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 8647d982..e9099f81 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -20,10 +20,12 @@ package com.comphenix.protocol.injector; import java.io.DataInputStream; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; import java.util.logging.Level; import java.util.logging.Logger; @@ -43,10 +45,8 @@ import org.bukkit.plugin.Plugin; import org.bukkit.plugin.PluginManager; import com.comphenix.protocol.ProtocolManager; -import com.comphenix.protocol.events.ConnectionSide; -import com.comphenix.protocol.events.PacketContainer; -import com.comphenix.protocol.events.PacketEvent; -import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.events.*; +import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; import com.google.common.base.Objects; import com.google.common.collect.ImmutableSet; @@ -57,14 +57,18 @@ public final class PacketFilterManager implements ProtocolManager { private Set packetListeners = new CopyOnWriteArraySet(); // Player injection - private Map connectionLookup = new HashMap(); + private Map connectionLookup = new ConcurrentHashMap(); private Map playerInjection = new HashMap(); // Packet injection private PacketInjector packetInjector; // Enabled packet filters - private Set packetFilters = new HashSet(); + private Set sendingFilters = Collections.newSetFromMap(new ConcurrentHashMap()); + + // The two listener containers + private ConcurrentListenerMultimap recievedListeners = new ConcurrentListenerMultimap(); + private ConcurrentListenerMultimap sendingListeners = new ConcurrentListenerMultimap(); // Whether or not this class has been closed private boolean hasClosed; @@ -107,10 +111,18 @@ public final class PacketFilterManager implements ProtocolManager { public void addPacketListener(PacketListener listener) { if (listener == null) throw new IllegalArgumentException("listener cannot be NULL."); - + + ConnectionSide side = listener.getConnectionSide(); packetListeners.add(listener); - enablePacketFilters(listener.getConnectionSide(), - listener.getPacketsID()); + + // Add listeners + if (side.isForServer()) + sendingListeners.addListener(listener); + if (side.isForClient()) + recievedListeners.addListener(listener); + + // Inform our injected hooks + enablePacketFilters(side, listener.getPacketsID()); } @Override @@ -118,9 +130,24 @@ public final class PacketFilterManager implements ProtocolManager { if (listener == null) throw new IllegalArgumentException("listener cannot be NULL"); + ConnectionSide side = listener.getConnectionSide(); + List sendingRemoved = null; + List receivingRemoved = null; + + // Remove from the overal list of listeners packetListeners.remove(listener); - disablePacketFilters(listener.getConnectionSide(), - listener.getPacketsID()); + + // Add listeners + if (side.isForServer()) + sendingRemoved = sendingListeners.removeListener(listener); + if (side.isForClient()) + receivingRemoved = recievedListeners.removeListener(listener); + + // Remove hooks, if needed + if (sendingRemoved != null && sendingRemoved.size() > 0) + disablePacketFilters(ConnectionSide.SERVER_SIDE, sendingRemoved); + if (receivingRemoved != null && receivingRemoved.size() > 0) + disablePacketFilters(ConnectionSide.CLIENT_SIDE, receivingRemoved); } @Override @@ -132,7 +159,7 @@ public final class PacketFilterManager implements ProtocolManager { // Remove the listener if (Objects.equal(listener.getPlugin(), plugin)) { - packetListeners.remove(listener); + removePacketListener(listener); } } } @@ -142,15 +169,7 @@ public final class PacketFilterManager implements ProtocolManager { * @param event - the packet event to invoke. */ public void invokePacketRecieving(PacketEvent event) { - for (PacketListener listener : packetListeners) { - try { - if (canHandlePacket(listener, event)) - listener.onPacketReceiving(event); - } catch (Exception e) { - // Minecraft doesn't want your Exception. - logger.log(Level.SEVERE, "Exception occured in onPacketReceiving() for " + listener.toString(), e); - } - } + recievedListeners.invokePacketRecieving(logger, event); } /** @@ -158,26 +177,7 @@ public final class PacketFilterManager implements ProtocolManager { * @param event - the packet event to invoke. */ public void invokePacketSending(PacketEvent event) { - for (PacketListener listener : packetListeners) { - try { - if (canHandlePacket(listener, event)) - listener.onPacketSending(event); - } catch (Exception e) { - logger.log(Level.SEVERE, "Exception occured in onPacketReceiving() for " + listener.toString(), e); - } - } - } - - private boolean canHandlePacket(PacketListener listener, PacketEvent event) { - // Make sure the listener is looking for this packet - if (!listener.getPacketsID().contains(event.getPacket().getID())) - return false; - - // And this type of packet - if (event.isServerPacket()) - return listener.getConnectionSide().isForServer(); - else - return listener.getConnectionSide().isForClient(); + sendingListeners.invokePacketSending(logger, event); } /** @@ -188,13 +188,13 @@ public final class PacketFilterManager implements ProtocolManager { * @param side - which side the event will arrive from. * @param packets - the packet id(s). */ - private void enablePacketFilters(ConnectionSide side, Set packets) { + private void enablePacketFilters(ConnectionSide side, Iterable packets) { if (side == null) throw new IllegalArgumentException("side cannot be NULL."); for (int packetID : packets) { - if (side.isForServer()) - packetFilters.add(packetID); + if (side.isForServer()) + sendingFilters.add(packetID); if (side.isForClient() && packetInjector != null) packetInjector.addPacketHandler(packetID); } @@ -205,13 +205,13 @@ public final class PacketFilterManager implements ProtocolManager { * @param packets - the packet id(s). * @param side - which side the event no longer should arrive from. */ - private void disablePacketFilters(ConnectionSide side, Set packets) { + private void disablePacketFilters(ConnectionSide side, Iterable packets) { if (side == null) throw new IllegalArgumentException("side cannot be NULL."); for (int packetID : packets) { if (side.isForServer()) - packetFilters.remove(packetID); + sendingFilters.remove(packetID); if (side.isForClient() && packetInjector != null) packetInjector.removePacketHandler(packetID); } @@ -268,7 +268,7 @@ public final class PacketFilterManager implements ProtocolManager { if (!skipDefaults) { try { packet.getModifier().writeDefaults(); - } catch (IllegalAccessException e) { + } catch (FieldAccessException e) { throw new RuntimeException("Security exception.", e); } } @@ -279,9 +279,9 @@ public final class PacketFilterManager implements ProtocolManager { @Override public Set getPacketFilters() { if (packetInjector != null) - return Sets.union(packetFilters, packetInjector.getPacketHandlers()); + return Sets.union(sendingFilters, packetInjector.getPacketHandlers()); else - return packetFilters; + return sendingFilters; } /** @@ -297,7 +297,7 @@ public final class PacketFilterManager implements ProtocolManager { // Don't inject if the class has closed if (!hasClosed && player != null && !playerInjection.containsKey(player)) { try { - PlayerInjector injector = new PlayerInjector(player, this, packetFilters); + PlayerInjector injector = new PlayerInjector(player, this, sendingFilters); injector.injectManager(); playerInjection.put(player, injector); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java index 88407308..6cd5244a 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java @@ -84,15 +84,15 @@ class PlayerInjector { // The packet manager and filters private PacketFilterManager manager; - private Set packetFilters; + private Set sendingFilters; // Previous data input private DataInputStream cachedInput; - public PlayerInjector(Player player, PacketFilterManager manager, Set packetFilters) throws IllegalAccessException { + public PlayerInjector(Player player, PacketFilterManager manager, Set sendingFilters) throws IllegalAccessException { this.player = player; this.manager = manager; - this.packetFilters = packetFilters; + this.sendingFilters = sendingFilters; initialize(); } @@ -342,7 +342,7 @@ class PlayerInjector { Integer id = MinecraftRegistry.getPacketToID().get(packet.getClass()); // Make sure we're listening - if (packetFilters.contains(id)) { + if (sendingFilters.contains(id)) { // A packet has been sent guys! PacketContainer container = new PacketContainer(id, packet); PacketEvent event = PacketEvent.fromServer(manager, container, player); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java b/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java index 6b8eab6f..bc80d438 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java @@ -28,7 +28,7 @@ import com.comphenix.protocol.reflect.StructureModifier; * Caches structure modifiers. * @author Kristian */ -class StructureCache { +public class StructureCache { // Structure modifiers private static Map> structureModifiers = new HashMap>();