[jbosscache-commits] JBoss Cache SVN: r5333 - in core/trunk/src: main/java/org/jboss/cache/factories and 3 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Fri Feb 8 08:30:51 EST 2008
Author: manik.surtani at jboss.com
Date: 2008-02-08 08:30:51 -0500 (Fri, 08 Feb 2008)
New Revision: 5333
Added:
core/trunk/src/main/java/org/jboss/cache/factories/annotations/CacheInjectionMethods.java
core/trunk/src/main/java/org/jboss/cache/util/reflect/CachedMethod.java
Modified:
core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
core/trunk/src/main/java/org/jboss/cache/factories/ComponentRegistry.java
core/trunk/src/main/java/org/jboss/cache/util/reflect/ReflectionUtil.java
core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java
core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java
Log:
Performance enhancements
Modified: core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -40,6 +40,11 @@
* It is important to node that the direct <b>read</b> methods, such as getDataDirect(), return unmodifiable collections.
* In addition to being unmodifiable, they are also defensively copied from the underlying data map to ensure view consistency.
* <p/>
+ * <b>Note:</b> the above paragraph was true for JBoss Cache 2.0.0, but for 2.1.0, this has changed to now offer direct view of
+ * the data structures, since the defensive copying was seriously affecting scalability. Please use the *Direct() methods
+ * with care - even though you may be able to directly manipulate the data structures returned, the cache isn't designed
+ * for you to do so. Use with care.
+ * <p/>
*
* @author <a href="mailto:manik at jboss.org">Manik Surtani (manik at jboss.org)</a>
* @see Node
Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -8,6 +8,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.jboss.cache.factories.annotations.CacheInjectionMethods;
import org.jboss.cache.factories.annotations.Inject;
import org.jboss.cache.lock.IdentityLock;
import org.jboss.cache.marshall.MarshalledValue;
@@ -32,6 +33,7 @@
* @since 2.0.0
*/
@SuppressWarnings("unchecked")
+ at CacheInjectionMethods
public class UnversionedNode<K, V> extends AbstractNode<K, V>
{
/**
@@ -221,7 +223,8 @@
public Map getDataDirect()
{
if (data == null) return Collections.emptyMap();
- return Collections.unmodifiableMap(data);
+ //return Collections.unmodifiableMap(data);
+ return data; // wrapping in an unmodifiable map is too expensive!
}
public Object put(Object key, Object value)
@@ -484,7 +487,8 @@
public Set<Object> getChildrenNamesDirect()
{
- return children == null ? Collections.emptySet() : new HashSet<Object>(children.keySet());
+ //return children == null ? Collections.emptySet() : new HashSet<Object>(children.keySet());
+ return children == null ? Collections.emptySet() : children.keySet();
}
public Set<Object> getKeysDirect()
@@ -493,7 +497,8 @@
{
return Collections.emptySet();
}
- return Collections.unmodifiableSet(new HashSet<Object>(data.keySet()));
+// return Collections.unmodifiableSet(new HashSet<Object>(data.keySet()));
+ return data.keySet();
}
public boolean removeChildDirect(Object childName)
@@ -632,7 +637,8 @@
NodeSPI spi = (NodeSPI) n;
if (!spi.isDeleted()) exclDeleted.add(spi);
}
- return Collections.unmodifiableSet(exclDeleted);
+// return Collections.unmodifiableSet(exclDeleted);
+ return exclDeleted;
}
public boolean hasChildrenDirect()
@@ -646,7 +652,8 @@
{
if (children != null && !children.isEmpty())
{
- return Collections.unmodifiableSet(new HashSet(children.values()));
+// return Collections.unmodifiableSet(new HashSet(children.values()));
+ return new HashSet(children.values());
}
else
{
Modified: core/trunk/src/main/java/org/jboss/cache/factories/ComponentRegistry.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/factories/ComponentRegistry.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/main/java/org/jboss/cache/factories/ComponentRegistry.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -9,6 +9,7 @@
import org.jboss.cache.config.ConfigurationException;
import org.jboss.cache.config.RuntimeConfig;
import static org.jboss.cache.factories.ComponentRegistry.State.*;
+import org.jboss.cache.factories.annotations.CacheInjectionMethods;
import org.jboss.cache.factories.annotations.ComponentName;
import org.jboss.cache.factories.annotations.DefaultFactoryFor;
import org.jboss.cache.factories.annotations.Inject;
@@ -16,6 +17,7 @@
import org.jboss.cache.factories.annotations.Stop;
import org.jboss.cache.invocation.RemoteCacheInvocationDelegate;
import org.jboss.cache.util.BeanUtils;
+import org.jboss.cache.util.reflect.CachedMethod;
import org.jboss.cache.util.reflect.ReflectionUtil;
import java.lang.annotation.Annotation;
@@ -76,6 +78,11 @@
private static Log log = LogFactory.getLog(ComponentRegistry.class);
private Bootstrap bootstrap;
+ // cache of reflection methods to call during injections. These will be emptied when start() is called.
+ Map<Class, List<CachedMethod>> shortTermMethodCache = null;
+ // these will hang around longer - for components that are frequently created during normal operation.
+ Map<Class, List<CachedMethod>> longTermMethodCache = null;
+
/**
* Creates an instance of the component registry. The configuration passed in is automatically registered.
*
@@ -425,11 +432,12 @@
//if (log.isTraceEnabled()) log.trace("Inspecting class " + target.getClass());
try
{
- List<Method> methods = ReflectionUtil.getAllMethods(target.getClass(), Inject.class);
+ // look in caches first
+ List<CachedMethod> methods = lookupInjectionMethods(target.getClass());
//if (log.isTraceEnabled()) log.trace("Found method set containing " + methods.size() + " methods that need injection: " + methods);
// search for anything we need to inject
- for (Method method : methods)
+ for (CachedMethod method : methods)
{
//if (log.isTraceEnabled()) log.trace("Method " + method + " needs some other components injected!");
performInjection(method, target);
@@ -441,6 +449,26 @@
}
}
+ private List<CachedMethod> lookupInjectionMethods(Class type)
+ {
+ if (longTermMethodCache != null && longTermMethodCache.containsKey(type)) return longTermMethodCache.get(type);
+ if (shortTermMethodCache != null && shortTermMethodCache.containsKey(type)) return shortTermMethodCache.get(type);
+
+ List<CachedMethod> methods = ReflectionUtil.getAllCachedMethods(type, Inject.class);
+ if (type.isAnnotationPresent(CacheInjectionMethods.class))
+ {
+ if (longTermMethodCache == null) longTermMethodCache = new HashMap<Class, List<CachedMethod>>();
+ longTermMethodCache.put(type, methods);
+ }
+ else
+ {
+ if (shortTermMethodCache == null) shortTermMethodCache = new HashMap<Class, List<CachedMethod>>();
+ shortTermMethodCache.put(type, methods);
+ }
+
+ return methods;
+ }
+
/**
* Looks through the parameter list of the given method, attempts to locate parameters that fit the types that may
* exist in the {@link ComponentRegistry}, and then calls the method on the target object with the necessary parameters.
@@ -452,7 +480,7 @@
* if the method cannot be called
*/
@SuppressWarnings("unchecked")
- private <T> void performInjection(Method method, T target) throws IllegalAccessException, InvocationTargetException
+ private <T> void performInjection(CachedMethod method, T target) throws IllegalAccessException, InvocationTargetException
{
Class[] parameterTypes = method.getParameterTypes();
List<Component> componentsToInject = getDeclaredDependencies(method);
@@ -464,12 +492,13 @@
parameters[i] = getComponent(componentsToInject.get(i).name, parameterTypes[i]);
}
+ Method reflectMethod = method.getMethod();
// make sure we set this method to be accessible, so we can call private, package and protected
// methods rather than just public ones.
- method.setAccessible(true);
+ reflectMethod.setAccessible(true);
// invoke the method with the parameters we've worked out.
- method.invoke(target, parameters);
+ reflectMethod.invoke(target, parameters);
}
private String extractComponentName(Annotation[] annotationsOnParameter)
@@ -485,7 +514,7 @@
return null;
}
- private List<Component> getDeclaredDependencies(Method method)
+ private List<Component> getDeclaredDependencies(CachedMethod method)
{
List<Component> dependencies = new LinkedList<Component>();
Class[] parameterTypes = method.getParameterTypes();
@@ -606,6 +635,7 @@
public void start()
{
moveComponentsToState(STARTED);
+ shortTermMethodCache = null;
}
/**
@@ -710,10 +740,10 @@
this.instance = instance;
// now scan the instance for all dependencies.
- List<Method> injectionMethods = ReflectionUtil.getAllMethods(instance.getClass(), Inject.class);
+ List<CachedMethod> injectionMethods = lookupInjectionMethods(instance.getClass());
// now for each injection method, get dependencies
- for (Method m : injectionMethods) dependencies.addAll(getDeclaredDependencies(m));
+ for (CachedMethod m : injectionMethods) dependencies.addAll(getDeclaredDependencies(m));
}
/**
@@ -845,10 +875,10 @@
{
try
{
- List<Method> methods = ReflectionUtil.getAllMethods(instance.getClass(), Inject.class);
+ List<CachedMethod> methods = lookupInjectionMethods(instance.getClass());
// search for anything we need to inject
- for (Method method : methods) performInjection(method, instance);
+ for (CachedMethod method : methods) performInjection(method, instance);
}
catch (Exception e)
{
Copied: core/trunk/src/main/java/org/jboss/cache/factories/annotations/CacheInjectionMethods.java (from rev 5322, core/trunk/src/main/java/org/jboss/cache/factories/annotations/Inject.java)
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/factories/annotations/CacheInjectionMethods.java (rev 0)
+++ core/trunk/src/main/java/org/jboss/cache/factories/annotations/CacheInjectionMethods.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -0,0 +1,23 @@
+package org.jboss.cache.factories.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * If this is set on a class, then all methods on that class (and superclasses) annotated with @Inject are cached in the component registry.
+ * Useful for components that are frequently constructed using the component registry during the lifespan of the cache.
+ *
+ * @author Manik Surtani
+ * @see org.jboss.cache.factories.annotations.ComponentName
+ * @since 2.1.0
+ */
+// ensure this annotation is available at runtime.
+ at Retention(RetentionPolicy.RUNTIME)
+
+// only applies to fields.
+ at Target(ElementType.TYPE)
+public @interface CacheInjectionMethods
+{
+}
\ No newline at end of file
Added: core/trunk/src/main/java/org/jboss/cache/util/reflect/CachedMethod.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/util/reflect/CachedMethod.java (rev 0)
+++ core/trunk/src/main/java/org/jboss/cache/util/reflect/CachedMethod.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -0,0 +1,39 @@
+package org.jboss.cache.util.reflect;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+
+/**
+ * A cached Method object, so that calls to getParameterTypes, getAnnotations, etc are cached.
+ *
+ * @author Manik Surtani (<a href="mailto:manik at jboss.org">manik at jboss.org</a>)
+ * @since 2.1.0
+ */
+public class CachedMethod
+{
+ Method method;
+ Class[] parameterTypes;
+ Annotation[][] parameterAnnotations;
+
+ public CachedMethod(Method method)
+ {
+ this.method = method;
+ this.parameterTypes = method.getParameterTypes();
+ this.parameterAnnotations = method.getParameterAnnotations();
+ }
+
+ public Method getMethod()
+ {
+ return method;
+ }
+
+ public Class[] getParameterTypes()
+ {
+ return parameterTypes;
+ }
+
+ public Annotation[][] getParameterAnnotations()
+ {
+ return parameterAnnotations;
+ }
+}
Modified: core/trunk/src/main/java/org/jboss/cache/util/reflect/ReflectionUtil.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/util/reflect/ReflectionUtil.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/main/java/org/jboss/cache/util/reflect/ReflectionUtil.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -37,6 +37,21 @@
}
/**
+ * Returns a set of Methods that contain the given method annotation. This includes all public, protected, package and private
+ * methods, as well as those of superclasses. Note that this does *not* include overridden methods.
+ *
+ * @param c class to inspect
+ * @param annotationType the type of annotation to look for
+ * @return List of Method objects that require injection.
+ */
+ public static List<CachedMethod> getAllCachedMethods(Class c, Class<? extends Annotation> annotationType)
+ {
+ List<CachedMethod> annotated = new LinkedList<CachedMethod>();
+ inspectRecursivelyCached(c, annotated, annotationType);
+ return annotated;
+ }
+
+ /**
* Inspects a class and it's superclasses (all the way to {@link Object} for method instances that contain a given annotation.
* This even identifies private, package and protected methods, not just public ones.
*
@@ -60,6 +75,29 @@
}
/**
+ * Inspects a class and it's superclasses (all the way to {@link Object} for method instances that contain a given annotation.
+ * This even identifies private, package and protected methods, not just public ones.
+ *
+ * @param c
+ * @param s
+ * @param annotationType
+ */
+ private static void inspectRecursivelyCached(Class c, List<CachedMethod> s, Class<? extends Annotation> annotationType)
+ {
+ // Superclass first
+ if (!c.equals(Object.class)) inspectRecursivelyCached(c.getSuperclass(), s, annotationType);
+
+ for (Method m : c.getDeclaredMethods())
+ {
+ // don't bother if this method has already been overridden by a subclass
+ if (!alreadyFoundCached(m, s) && m.isAnnotationPresent(annotationType))
+ {
+ s.add(new CachedMethod(m));
+ }
+ }
+ }
+
+ /**
* Tests whether a method has already been found, i.e., overridden.
*
* @param m method to inspect
@@ -77,6 +115,24 @@
return false;
}
+ /**
+ * Tests whether a method has already been found, i.e., overridden.
+ *
+ * @param m method to inspect
+ * @param s collection of methods found
+ * @return true a method with the same signature already exists.
+ */
+ private static boolean alreadyFoundCached(Method m, Collection<CachedMethod> s)
+ {
+ for (CachedMethod found : s)
+ {
+ if (m.getName().equals(found.getMethod().getName()) &&
+ Arrays.equals(m.getParameterTypes(), found.getParameterTypes()))
+ return true;
+ }
+ return false;
+ }
+
public static void setValue(Object instance, String fieldName, Object value)
{
try
Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -178,31 +178,6 @@
tm.commit();
}
- public void testImmutabilityOfData()
- {
- rootNode.put("key", "value");
- Map<Object, Object> m = rootNode.getData();
- try
- {
- m.put("x", "y");
- fail("Map should be immutable!!");
- }
- catch (Exception e)
- {
- // expected
- }
-
- try
- {
- rootNode.getKeys().add(new Object());
- fail("Key set should be immutable");
- }
- catch (Exception e)
- {
- // expected
- }
- }
-
public void testImmutabilityOfChildren()
{
rootNode.addChild(A);
Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java 2008-02-08 09:32:51 UTC (rev 5332)
+++ core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java 2008-02-08 13:30:51 UTC (rev 5333)
@@ -83,26 +83,26 @@
Map dataDirect = root.getDataDirect();
Set keysDirect = root.getKeysDirect();
- try
- {
- dataDirect.remove("k");
- fail("getDataDirect() should return an unmodifiable collection object");
- }
- catch (UnsupportedOperationException uoe)
- {
- // good; should be immutable
- }
+// try
+// {
+// dataDirect.remove("k");
+// fail("getDataDirect() should return an unmodifiable collection object");
+// }
+// catch (UnsupportedOperationException uoe)
+// {
+// // good; should be immutable
+// }
+//
+// try
+// {
+// keysDirect.clear();
+// fail("getKeysDirect() should return an unmodifiable collection object");
+// }
+// catch (UnsupportedOperationException uoe)
+// {
+// // good; should be immutable
+// }
- try
- {
- keysDirect.clear();
- fail("getKeysDirect() should return an unmodifiable collection object");
- }
- catch (UnsupportedOperationException uoe)
- {
- // good; should be immutable
- }
-
// now test defensive copy
root.put("k2", "v2");
@@ -111,7 +111,7 @@
assertTrue("getKeysDirect() should have made a defensive copy of the data collection object", !keysDirect.contains("k2"));
}
- public void testChildrenImmutabilityAndDefensiveCopy()
+ public void testChildrenDefensiveCopy()
{
// put some stuff in the root node
String childName = "childName";
@@ -119,17 +119,7 @@
root.addChild(new Fqn<String>(childName));
Set childrenDirect = root.getChildrenDirect();
- try
- {
- childrenDirect.clear();
- fail("getChildrenDirect() should return an unmodifiable collection object");
- }
- catch (UnsupportedOperationException uoe)
- {
- // good; should be immutable
- }
-
- // now test defensive copy
+ // test defensive copy
root.addChild(new Fqn<String>(newChild));
assertTrue("root.addChild() should have succeeded", root.getChildrenNamesDirect().contains(newChild));
More information about the jbosscache-commits
mailing list