Author: bela(a)jboss.com Date: 2007-12-30 03:13:49 -0500 (Sun, 30 Dec 2007) New Revision: 4926 Modified: core/trunk/src/main/java/org/jboss/cache/notifications/Notifier.java Log: Moved copying of data and other initialization for notification callback into the if block: this only needs to be done if there are listeners available. JProfiler analysis showed that the copying of data used up a lot of CPU cycles, because it is on the critical path. This fix should speed up things by a huge margin Modified: core/trunk/src/main/java/org/jboss/cache/notifications/Notifier.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/notifications/Notifier.java 2007-12-28 16:58:21 UTC (rev 4925) +++ core/trunk/src/main/java/org/jboss/cache/notifications/Notifier.java 2007-12-30 08:13:49 UTC (rev 4926) @@ -102,7 +102,7 @@ log.warn("Attempted to register listener of class " + listener.getClass() + ", but no valid, public methods annotated with method-level event annotations found! Ignoring listener."); } - private void testListenerClassValidity(Class<?> listenerClass) + private static void testListenerClassValidity(Class<?> listenerClass) { if (!listenerClass.isAnnotationPresent(CacheListener.class)) throw new IncorrectCacheListenerException("Cache listener class MUST be annotated with org.jboss.cache.notifications.annotation.CacheListener"); @@ -110,7 +110,7 @@ throw new IncorrectCacheListenerException("Cache listener class MUST be public!"); } - private void testListenerMethodValidity(Method m, Class allowedParameter, String annotationName) + private static void testListenerMethodValidity(Method m, Class allowedParameter, String annotationName) { if (m.getParameterTypes().length != 1 || !m.getParameterTypes()[0].isAssignableFrom(allowedParameter)) throw new IncorrectCacheListenerException("Methods annotated with " + annotationName + " must accept exactly one parameter, of assignable from type " + allowedParameter.getName()); @@ -168,7 +168,7 @@ l.removeAll(markedForRemoval); - if (l.size() == 0) listenerInvocations.remove(annotation); + if (l.isEmpty()) listenerInvocations.remove(annotation); } } @@ -212,13 +212,12 @@ */ public void notifyNodeCreated(Fqn fqn, boolean pre, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeCreated.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -243,14 +242,13 @@ */ public void notifyNodeModified(Fqn fqn, boolean pre, NodeModifiedEvent.ModificationType modificationType, Map data, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Map dataCopy = copy(data); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeModified.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Map dataCopy = copy(data); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -276,14 +274,13 @@ */ public void notifyNodeRemoved(Fqn fqn, boolean pre, Map data, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Map dataCopy = copy(data); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeRemoved.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Map dataCopy = copy(data); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -307,12 +304,11 @@ */ public void notifyNodeVisited(Fqn fqn, boolean pre, InvocationContext ctx) { - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeVisited.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -327,13 +323,12 @@ public void notifyNodeMoved(Fqn originalFqn, Fqn newFqn, boolean pre, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeMoved.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -358,13 +353,12 @@ */ public void notifyNodeEvicted(final Fqn fqn, final boolean pre, InvocationContext ctx) { - final boolean originLocal = ctx.isOriginLocal(); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeEvicted.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + final boolean originLocal = ctx.isOriginLocal(); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -388,14 +382,13 @@ */ public void notifyNodeLoaded(Fqn fqn, boolean pre, Map data, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Map dataCopy = copy(data); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeLoaded.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Map dataCopy = copy(data); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -420,14 +413,13 @@ */ public void notifyNodeActivated(Fqn fqn, boolean pre, Map data, InvocationContext ctx) { - boolean originLocal = ctx.isOriginLocal(); - Map dataCopy = copy(data); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodeActivated.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + boolean originLocal = ctx.isOriginLocal(); + Map dataCopy = copy(data); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -452,13 +444,12 @@ */ public void notifyNodePassivated(Fqn fqn, boolean pre, Map data, InvocationContext ctx) { - Map dataCopy = copy(data); - Transaction tx = ctx.getTransaction(); - List<ListenerInvocation> listeners = listenerInvocations.get(NodePassivated.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + Map dataCopy = copy(data); + Transaction tx = ctx.getTransaction(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -482,7 +473,7 @@ { List<ListenerInvocation> listeners = listenerInvocations.get(CacheStarted.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); @@ -503,7 +494,7 @@ { List<ListenerInvocation> listeners = listenerInvocations.get(CacheStopped.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); @@ -525,7 +516,7 @@ { List<ListenerInvocation> listeners = listenerInvocations.get(ViewChanged.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); @@ -545,12 +536,12 @@ */ public void notifyTransactionCompleted(Transaction transaction, boolean successful, InvocationContext ctx) { - Transaction tx = ctx.getTransaction(); - boolean isOriginLocal = ctx.isOriginLocal(); List<ListenerInvocation> listeners = listenerInvocations.get(TransactionCompleted.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + Transaction tx = ctx.getTransaction(); + boolean isOriginLocal = ctx.isOriginLocal(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -570,12 +561,12 @@ */ public void notifyTransactionRegistered(Transaction transaction, InvocationContext ctx) { - Transaction tx = ctx.getTransaction(); - boolean isOriginLocal = ctx.isOriginLocal(); List<ListenerInvocation> listeners = listenerInvocations.get(TransactionRegistered.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { + Transaction tx = ctx.getTransaction(); + boolean isOriginLocal = ctx.isOriginLocal(); InvocationContext backup = resetInvocationContext(ctx); EventImpl e = new EventImpl(); e.setCache(cache); @@ -591,7 +582,7 @@ { List<ListenerInvocation> listeners = listenerInvocations.get(CacheBlocked.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { EventImpl e = new EventImpl(); e.setCache(cache); @@ -605,7 +596,7 @@ { List<ListenerInvocation> listeners = listenerInvocations.get(CacheUnblocked.class); - if (listeners != null && listeners.size() > 0) + if (listeners != null && !listeners.isEmpty()) { EventImpl e = new EventImpl(); e.setCache(cache); @@ -616,7 +607,7 @@ } - private Map copy(Map data) + private static Map copy(Map data) { if (safe(data)) return data; return new MapCopy(data); @@ -653,7 +644,7 @@ * @param map * @return */ - private boolean safe(Map map) + private static boolean safe(Map map) { return map == null || map instanceof MapCopy || map.getClass().equals(emptyMap) || map.getClass().equals(singletonMap); }