Various performance issues in core projects
While working on EJBTHREE-1800, we found that most of the issues were either related to AOP or MC. So i created a simple plain MC app (no EJBs in picture) with 101 MC beans and each MC bean having 100 methods each. Like this:
package org.myapp.mc;
import org.jboss.beans.metadata.api.annotations.*;
public class MCBean2
{
@Inject
private SomeOtherInterface otherBean;
public SomeOtherInterface getOtherBean()
{
return this.otherBean;
}
public void method1(){}
public void method2(){}
public void method3(){}
public void method4(){}
public void method5(){}
public void method6(){}
public void method7(){}
public void method8(){}
public void method9(){}
Each MC bean has just one plain @Inject and 100 non-annotated methods. When i deployed this on my system, it took close to 2 seconds for this to be deployed. There's nothing fancy in this application, so i would have expected this to be faster. But this timing aligns with what we are seeing with the EJB3 deployment (we do not deploy the EJB impl class as MC beans but we do deploy the EJB containers as MC beans).
Based on what i could see for this plain MC application in JProfiler, here are the issues (so far):
1) As mentioned in the other thread, MC looks for annotations on every method on every MC bean for annotation plugins (org.jboss.kernel.plugins.annotations.CommonAnnotationAdapter). So 100 methods * 101 beans. During this process, the call leads to org.jboss.metadata.plugins.loader.reflection.AnnotatedElementMetaDataLoader.getComponentMetaDataRetrieval which has this code:
SecurityActions.findMethod(clazz, signature.getName(), signature.getParametersTypes(clazz));
similarly for constructors and fields:
constructor = SecurityActions.findConstructor(clazz, signature.getParametersTypes(clazz)); field = SecurityActions.findField(clazz, signature.getName());
This call ultimately leads to org.jboss.reflect.plugins.introspection.ReflectionUtils.findMethod/findField/findConstructor, which have code like this:
public static Method findMethod(Class<?> clazz, String name, Class<?>... parameterTypes)
{
if (clazz == null)
return null;
Method[] methods = clazz.getDeclaredMethods();
if (methods.length != 0)
{
for (Method method : methods)
{
if (method.getName().equals(name))
{
Class<?>[] methodParams = method.getParameterTypes();
if (methodParams.length != 0)
{
if (Arrays.equals(methodParams, parameterTypes))
return method;
}
else if (parameterTypes == null || parameterTypes.length == 0)
return method;
}
}
}
return findMethod(clazz.getSuperclass(), name, parameterTypes);
}
Same applies for the findConstructor and findField. As can be seen, this method calls the expensive Class.getDeclaredMethods() and then iterates over the method to figure out the correct method. Remember, this method gets called for each method on each MC bean (100 * 101 for this application alone) which ultimately leads to performance issues. Profiler shows me there are 33232 calls to the Class.getDeclaredMethods() each taking an average of 297 micro sec. I changed the implementation of these methods (findMethod, findConstructor and findField) and see good enough performance improvements:
public static Method findMethod(Class<?> clazz, String name, Class<?>... parameterTypes)
{
if (clazz == null)
return null;
try
{
Method method = clazz.getDeclaredMethod(name, parameterTypes);
return method;
}
catch (SecurityException se)
{
throw se;
}
catch (NoSuchMethodException nsme)
{
// check on superclass if present
if (clazz.getSuperclass()!=null)
{
return findMethod(clazz.getSuperclass(), name, parameterTypes);
}
else
{
return null;
}
}
}
The calls to the less expensive Class.getDeclaredMethod and Class.getDeclaredField (and so on..) show that the average time of the calls has drastically dropped down to 9 micro sec.
This issue is similar to the one i reported here (in a different class) http://www.jboss.org/index.html?module=bb&op=viewtopic&t=154875
2) The org.jboss.kernel.plugins.annotations.CommonAnnotationAdapter iterates over each method to apply the annotation plugins for those methods:
...//code trimmed
Set<MethodInfo> methods = info.getMethods();
if (methods != null && methods.isEmpty() == false)
{
for(MethodInfo mi : methods)
{
ClassInfo declaringCI = mi.getDeclaringClass();
// direct == check is OK
if (declaringCI != objectTI && visitedMethods.contains(mi) == false)
{
Signature mis = new MethodSignature(mi);
MetaData cmdr = retrieval.getComponentMetaData(mis);
if (cmdr != null)
{
methodPlugins = getPlugins(ElementType.METHOD, METHOD_FILTER, annotationClasses);
for(T plugin : methodPlugins)
{
if (isApplyPhase)
applyPlugin(plugin, mi, cmdr, handle);
else
cleanPlugin(plugin, mi, cmdr, handle);
}
//code trimmed
As can be seen, during this process the getPlugins(...) is called for each method even though the getPlugins(...) is not per method specific. The getPlugins internally iterates over the available plugins (which are a lot) and then applies filtering for each annotation class and does other stuff. The call to this method can be moved out of the loop to improve the performance:
Set<MethodInfo> methods = info.getMethods();
if (methods != null && methods.isEmpty() == false)
{
//moved out of loop
methodPlugins = getPlugins(ElementType.METHOD, METHOD_FILTER, annotationClasses);
for(MethodInfo mi : methods)
{
ClassInfo declaringCI = mi.getDeclaringClass();
// direct == check is OK
if (declaringCI != objectTI && visitedMethods.contains(mi) == false)
{
Signature mis = new MethodSignature(mi);
MetaData cmdr = retrieval.getComponentMetaData(mis);
This too shows an improvement in the performance.
3) The org.jboss.kernel.plugins.annotations.CommonAnnotationAdapter also has a piece of code which looks for static methods:
// static methods
MethodInfo[] staticMethods = getStaticMethods(classInfo);
if (staticMethods != null && staticMethods.length != 0)
{
for(MethodInfo smi : staticMethods)
{
if (smi.isStatic() && smi.isPublic())
{
Signature mis = new MethodSignature(smi);
MetaData cmdr = retrieval.getComponentMetaData(mis);
if (cmdr != null)
{
if (methodPlugins == null)
methodPlugins = getPlugins(ElementType.METHOD, METHOD_FILTER, annotationClasses);
for(T plugin : methodPlugins)
{
if (isApplyPhase)
applyPlugin(plugin, smi, cmdr, handle);
else
cleanPlugin(plugin, smi, cmdr, handle);
}
}
/**
* Get the static methods of class info.
*
* @param classInfo the class info
* @return the static methods
*/
protected MethodInfo[] getStaticMethods(ClassInfo classInfo)
{
return classInfo.getDeclaredMethods();
}
I think the call to classInfo.getDeclaredMethods() can again be avoided, since we already have the methods of this class (a few lines before this call). So this change/fix:
// static methods
if (methodPlugins == null)
methodPlugins = getPlugins(ElementType.METHOD, METHOD_FILTER, annotationClasses);
for (MethodInfo method : methods)
{
// if the method was already visited, then no point processing it again
if (!visitedMethods.contains(method))
{
// ensure that the method is static and is declared in the class being processed
if(method.isStatic() && method.getDeclaringClass().equals(classInfo))
{
Signature mis = new MethodSignature(method);
MetaData cmdr = retrieval.getComponentMetaData(mis);
if (cmdr != null)
{
for(T plugin : methodPlugins)
{
if (isApplyPhase)
applyPlugin(plugin, method, cmdr, handle);
else
cleanPlugin(plugin, method, cmdr, handle);
}
}
else if (trace)
log.trace("No annotations for " + method);
}
}
}
4) Deployers - I see that every MC bean that gets deployed, also gets registered as a MBean. Is this required? During the MBean registration, i saw this piece of code (which Jprofiler shows as expensive, given the number of MC beans being registered as MBean) in org/jboss/deployers/structure/spi/helpers/ComponentDeploymentContext.java:
public ObjectName getObjectName()
{
if (objectName == null)
{
String name = getName();
name = name.replace("\"", """);
String temp = "jboss.deployment:id=\"" + name + "\",type=Component";
The String.replace is marked as a costly operation. I changed this to:
Index: src/main/java/org/jboss/deployers/structure/spi/helpers/ComponentDeploymentContext.java
===================================================================
--- src/main/java/org/jboss/deployers/structure/spi/helpers/ComponentDeploymentContext.java (revision 87763)
+++ src/main/java/org/jboss/deployers/structure/spi/helpers/ComponentDeploymentContext.java (working copy)
@@ -139,8 +139,9 @@
if (objectName == null)
{
String name = getName();
- name = name.replace("\"", """);
- String temp = "jboss.deployment:id=\"" + name + "\",type=Component";
+ //name = name.replace("\"", """);
+ name = ObjectName.quote(name);
+ String temp = "jboss.deployment:id=" + name + ",type=Component";
try
{
objectName = new ObjectName(temp);
The question however still remains, do we really need to register each MC bean as a MBean?
With all these changes (across various projects), i have been able to deploy that sample application now within around 1.1 seconds(which i think is still on the higher side). I am going to continue looking into other issues and see how much improvement it brings in to the EJB3 deployments.
P.S: I have all the patches ready if anyone wants to take a look.
