From a2b67f8ddb91bbac246dc3d5161acf4f1aa19526 Mon Sep 17 00:00:00 2001 From: Kenzie Togami Date: Fri, 10 May 2019 05:01:01 -0700 Subject: [PATCH] Re-write EventBus to be faster --- .../annotation/RequiresNewerGuava.java | 33 ----- .../worldedit/util/eventbus/EventBus.java | 115 +++++++----------- .../util/eventbus/HierarchyCache.java | 21 ++-- .../worldedit/util/eventbus/EventBusTest.java | 53 ++++++++ 4 files changed, 103 insertions(+), 119 deletions(-) delete mode 100644 worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/RequiresNewerGuava.java create mode 100644 worldedit-core/src/test/java/com/sk89q/worldedit/util/eventbus/EventBusTest.java diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/RequiresNewerGuava.java b/worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/RequiresNewerGuava.java deleted file mode 100644 index 749849ce8..000000000 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/internal/annotation/RequiresNewerGuava.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * WorldEdit, a Minecraft world manipulation toolkit - * Copyright (C) sk89q - * Copyright (C) WorldEdit team and contributors - * - * This program is free software: you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as published by the - * Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License - * for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program. If not, see . - */ - -package com.sk89q.worldedit.internal.annotation; - -import java.lang.annotation.Documented; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -/** - * Marks features that should be replaced with Google Guava but cannot - * yet because Bukkit uses such an old version of Guava. - */ -@Retention(RetentionPolicy.SOURCE) -@Documented -public @interface RequiresNewerGuava { -} diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/EventBus.java b/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/EventBus.java index d97c090ea..1121b6193 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/EventBus.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/EventBus.java @@ -19,11 +19,9 @@ package com.sk89q.worldedit.util.eventbus; +import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; -import com.google.common.collect.Multimaps; import com.google.common.collect.SetMultimap; -import com.google.common.eventbus.DeadEvent; -import com.sk89q.worldedit.internal.annotation.RequiresNewerGuava; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,11 +29,11 @@ import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collection; 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.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import static com.google.common.base.Preconditions.checkNotNull; @@ -46,17 +44,15 @@ import static com.google.common.base.Preconditions.checkNotNull; *

This class is based on Guava's {@link EventBus} but priority is supported * and events are dispatched at the time of call, rather than being queued up. * This does allow dispatching during an in-progress dispatch.

- * - *

This implementation utilizes naive synchronization on all getter and - * setter methods. Dispatch does not occur when a lock has been acquired, - * however.

*/ -public class EventBus { +public final class EventBus { private final Logger logger = LoggerFactory.getLogger(EventBus.class); + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private final SetMultimap, EventHandler> handlersByType = - Multimaps.newSetMultimap(new HashMap<>(), this::newHandlerSet); + HashMultimap.create(); /** * Strategy for finding handler methods in registered objects. Currently, @@ -65,7 +61,6 @@ public class EventBus { */ private final SubscriberFindingStrategy finder = new AnnotatedSubscriberFinder(); - @RequiresNewerGuava private HierarchyCache flattenHierarchyCache = new HierarchyCache(); /** @@ -74,10 +69,15 @@ public class EventBus { * @param clazz the event class to register * @param handler the handler to register */ - public synchronized void subscribe(Class clazz, EventHandler handler) { + public void subscribe(Class clazz, EventHandler handler) { checkNotNull(clazz); checkNotNull(handler); - handlersByType.put(clazz, handler); + lock.writeLock().lock(); + try { + handlersByType.put(clazz, handler); + } finally { + lock.writeLock().unlock(); + } } /** @@ -85,9 +85,14 @@ public class EventBus { * * @param handlers a map of handlers */ - public synchronized void subscribeAll(Multimap, EventHandler> handlers) { + public void subscribeAll(Multimap, EventHandler> handlers) { checkNotNull(handlers); - handlersByType.putAll(handlers); + lock.writeLock().lock(); + try { + handlersByType.putAll(handlers); + } finally { + lock.writeLock().unlock(); + } } /** @@ -96,10 +101,15 @@ public class EventBus { * @param clazz the class * @param handler the handler */ - public synchronized void unsubscribe(Class clazz, EventHandler handler) { + public void unsubscribe(Class clazz, EventHandler handler) { checkNotNull(clazz); checkNotNull(handler); - handlersByType.remove(clazz, handler); + lock.writeLock().lock(); + try { + handlersByType.remove(clazz, handler); + } finally { + lock.writeLock().unlock(); + } } /** @@ -107,15 +117,15 @@ public class EventBus { * * @param handlers a map of handlers */ - public synchronized void unsubscribeAll(Multimap, EventHandler> handlers) { + public void unsubscribeAll(Multimap, EventHandler> handlers) { checkNotNull(handlers); - for (Map.Entry, Collection> entry : handlers.asMap().entrySet()) { - Set currentHandlers = getHandlersForEventType(entry.getKey()); - Collection eventMethodsInListener = entry.getValue(); - - if (currentHandlers != null &&!currentHandlers.containsAll(entry.getValue())) { - currentHandlers.removeAll(eventMethodsInListener); + lock.writeLock().lock(); + try { + for (Map.Entry, Collection> entry : handlers.asMap().entrySet()) { + handlersByType.get(entry.getKey()).removeAll(entry.getValue()); } + } finally { + lock.writeLock().unlock(); } } @@ -146,25 +156,23 @@ public class EventBus { * successfully after the event has been posted to all handlers, and * regardless of any exceptions thrown by handlers. * - *

If no handlers have been subscribed for {@code event}'s class, and - * {@code event} is not already a {@link DeadEvent}, it will be wrapped in a - * DeadEvent and reposted. - * * @param event event to post. */ public void post(Object event) { List dispatching = new ArrayList<>(); - synchronized (this) { - Set> dispatchTypes = flattenHierarchy(event.getClass()); - + Set> dispatchTypes = flattenHierarchyCache.get(event.getClass()); + lock.readLock().lock(); + try { for (Class eventType : dispatchTypes) { - Set wrappers = getHandlersForEventType(eventType); + Set wrappers = handlersByType.get(eventType); if (wrappers != null && !wrappers.isEmpty()) { dispatching.addAll(wrappers); } } + } finally { + lock.readLock().unlock(); } Collections.sort(dispatching); @@ -175,14 +183,12 @@ public class EventBus { } /** - * Dispatches {@code event} to the handler in {@code handler}. This method - * is an appropriate override point for subclasses that wish to make - * event delivery asynchronous. + * Dispatches {@code event} to the handler in {@code handler}. * * @param event event to dispatch. * @param handler handler that will call the handler. */ - protected void dispatch(Object event, EventHandler handler) { + private void dispatch(Object event, EventHandler handler) { try { handler.handleEvent(event); } catch (InvocationTargetException e) { @@ -190,39 +196,4 @@ public class EventBus { } } - /** - * Retrieves a mutable set of the currently registered handlers for - * {@code type}. If no handlers are currently registered for {@code type}, - * this method may either return {@code null} or an empty set. - * - * @param type type of handlers to retrieve. - * @return currently registered handlers, or {@code null}. - */ - synchronized Set getHandlersForEventType(Class type) { - return handlersByType.get(type); - } - - /** - * Creates a new Set for insertion into the handler map. This is provided - * as an override point for subclasses. The returned set should support - * concurrent access. - * - * @return a new, mutable set for handlers. - */ - protected synchronized Set newHandlerSet() { - return new HashSet<>(); - } - - /** - * Flattens a class's type hierarchy into a set of Class objects. The set - * will include all superclasses (transitively), and all interfaces - * implemented by these superclasses. - * - * @param concreteClass class whose type hierarchy will be retrieved. - * @return {@code clazz}'s complete type hierarchy, flattened and uniqued. - */ - Set> flattenHierarchy(Class concreteClass) { - return flattenHierarchyCache.get(concreteClass); - } - } diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/HierarchyCache.java b/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/HierarchyCache.java index 92f1cb56b..9e0a97813 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/HierarchyCache.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/util/eventbus/HierarchyCache.java @@ -19,34 +19,27 @@ package com.sk89q.worldedit.util.eventbus; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import com.sk89q.worldedit.internal.annotation.RequiresNewerGuava; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.WeakHashMap; /** * Holds a cache of class hierarchy. - * - *

This exists because Bukkit has an ancient version of Guava and the cache - * library in Guava has since changed. */ -@RequiresNewerGuava class HierarchyCache { - private final Map, Set>> cache = new WeakHashMap<>(); + private final LoadingCache, Set>> cache = CacheBuilder.newBuilder() + .weakKeys() + .build(CacheLoader.from(this::build)); public Set> get(Class concreteClass) { - Set> ret = cache.get(concreteClass); - if (ret == null) { - ret = build(concreteClass); - cache.put(concreteClass, ret); - } - return ret; + return cache.getUnchecked(concreteClass); } protected Set> build(Class concreteClass) { diff --git a/worldedit-core/src/test/java/com/sk89q/worldedit/util/eventbus/EventBusTest.java b/worldedit-core/src/test/java/com/sk89q/worldedit/util/eventbus/EventBusTest.java new file mode 100644 index 000000000..f3e7e2b6f --- /dev/null +++ b/worldedit-core/src/test/java/com/sk89q/worldedit/util/eventbus/EventBusTest.java @@ -0,0 +1,53 @@ +package com.sk89q.worldedit.util.eventbus; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.Assert.assertEquals; + +public class EventBusTest { + + private static final class Event { + + } + + private static final class Subscriber { + + private final List events = new ArrayList<>(); + + @Subscribe + public void onEvent(Event event) { + events.add(event); + } + + } + + private EventBus eventBus = new EventBus(); + + @Test + public void testRegister() { + Subscriber subscriber = new Subscriber(); + eventBus.register(subscriber); + Event e1 = new Event(); + eventBus.post(e1); + Event e2 = new Event(); + eventBus.post(e2); + assertEquals(asList(e1, e2), subscriber.events); + } + + @Test + public void testUnregister() { + Subscriber subscriber = new Subscriber(); + eventBus.register(subscriber); + Event e1 = new Event(); + eventBus.post(e1); + eventBus.unregister(subscriber); + Event e2 = new Event(); + eventBus.post(e2); + assertEquals(singletonList(e1), subscriber.events); + } +}