Advisor.hasAnnotation handling
brian.stansberry Apr 24, 2009 8:15 AMSome EJB3 perf testing Dominik Pospisil is doing is showing major contention around a synchronized block in BaseClassLoader.loadClass(). I'll open a separate thread on Design of POJO Server re: reducing the cost of that sync block; this thread is about an issue in AOP that's increasing the amount of contention on the monitor guarding the block.
In the perf test, many threads are contending on the monitor via this call stack, which will get executed once per request to the ejb:
"WorkerThread#298[10.16.88.182:54023]" prio=10 tid=0x6534d800 nid=0x56be waiting for monitor entry [0x598ad000..0x598adf30] java.lang.Thread.State: BLOCKED (on object monitor) at org.jboss.classloader.spi.base.BaseClassLoader.loadClass(BaseClassLoader.java:411) - waiting to lock <0x79d9a068> (a org.jboss.classloader.spi.base.BaseClassLoader) at java.lang.ClassLoader.loadClass(ClassLoader.java:252) at org.jboss.ejb3.metadata.annotation.AnnotationRepositoryToMetaData.loadClass(AnnotationRepositoryToMetaData.java:216) at org.jboss.ejb3.metadata.annotation.AnnotationRepositoryToMetaData.hasAnnotation(AnnotationRepositoryToMetaData.java:328) at org.jboss.aop.Advisor.hasAnnotation(Advisor.java:889) at org.jboss.aop.Advisor.hasAnnotation(Advisor.java:865) at org.jboss.ejb3.interceptors.aop.LifecycleCallbacks.createLifecycleCallbackInterceptors(LifecycleCallbacks.java:101) at org.jboss.ejb3.EJBContainer.invokeCallback(EJBContainer.java:1112) at org.jboss.ejb3.stateful.StatefulContainer.invokePrePassivate(StatefulContainer.java:668) at org.jboss.ejb3.stateful.StatefulBeanContext.preReplicate(StatefulBeanContext.java:544) at org.jboss.ejb3.cache.tree.StatefulTreeCache.putInCache(StatefulTreeCache.java:510) at org.jboss.ejb3.cache.tree.StatefulTreeCache.replicate(StatefulTreeCache.java:278) at org.jboss.ejb3.cache.StatefulReplicationInterceptor.invoke(StatefulReplicationInterceptor.java:114)
If the org.jboss.ejb3.metadata.annotation.AnnotationRepositoryToMetaData.loadClass call could be avoided, the sync block would not be hit. Following from an email thread discusses how it's unfortunate that loadClass() is called at all:
"Brian Stansberry" wrote:
The other thing is loading this class at all seems unfortunate. Pseudo call stack:
1) LifecycleCallbacks.createLifecycleCallbackInterceptors --> Advisor.hasAnnotation(.. PrePassivate.class)
here we have the class
2) Advisor.hasAnnotation --> AnnotationRepositoryToMetaData.hasAnnotation(... PrePassivate.class.getName())
Advisor converts to String, probably because the default impl of AnnotationRepository caches things w/ String keys. But special class AnnotationRepositoryToMetaData does not..."Carlo de Wolf" wrote:
No, because Advisor calls the wrong method on AnnotationRepository.
Two lines down all exceptions are being ignored because of EJB3. Why is that?
If the annotation class can't be found by the annotation repository then it probably should return false, not throw a fit.
3) AnnotationRepositoryToMetaData.hasAnnotation() --> ClassLoader.loadClass(String)
to get back the class we have a ref to a couple frames back. :(
So, question is, if Advisor.hasAnnotation(Method m, Class<? extends Annotation> annotation) is called, can the call to AnnotationRepository be to hasAnnotation(Member m, Class annotation) rather than hasAnnotation(Member m, String annotation) ?
Secondary question is Carlo's point about exception handling.