[jboss-user] [JBoss Microcontainer Development] New message: "AnnotatedElementMetaDataLoader component metadata optimization"

Kabir Khan do-not-reply at jboss.com
Tue Feb 16 14:58:07 EST 2010


User development,

A new message was posted in the thread "AnnotatedElementMetaDataLoader component metadata optimization":

http://community.jboss.org/message/526629#526629

Author  : Kabir Khan
Profile : http://community.jboss.org/people/kabir.khan@jboss.com

Message:
--------------------------------------------------------------
Inspired by http://community.jboss.org/thread/96937?tstart=0 I did some standalone benchmarks of beans with a lot of methods, and found a bottleneck in CommonAnnotationAdapter which needs to obtain the component metadata for every single constructor/method/property.
 
The problem is that as far as I can tell:
* MemoryMetaDataLoader delegates to AbstractMutableComponentMetaDataLoader.getComponentMetaDataRetrieval(), which returns null if there is no component metadata:
*    public MetaDataRetrieval getComponentMetaDataRetrieval(Signature signature)
   {
      if (components == null)
         return null;
 
      return components.get(signature);
   }

* AnnotatedElementMetaDataLoader on the other hand will always return a loader as long as the passed in signature can be found
*    public MetaDataRetrieval getComponentMetaDataRetrieval(Signature signature)
   {
      if (signature == null)
         return null;
 
      if (annotated instanceof Class)
      {
         Class<?> clazz = Class.class.cast(annotated);
         if (signature instanceof ConstructorSignature)
         {
            ConstructorSignature constructorSignature = (ConstructorSignature) signature;
            Constructor<?> constructor = constructorSignature.getConstructor();
            if (constructor == null)
               constructor = SecurityActions.findConstructor(clazz, signature.getParametersTypes(clazz));
            if (constructor == null)
            {
               if (log.isTraceEnabled())
                  log.trace("Constructor with signature " + signature + " does not exist on class " + clazz.getName());
               return null;
            }
 
            return new AnnotatedElementMetaDataLoader(constructor);
         }
         //Same for fields, methods, parameters

 
The problem with what AnnotatedElementMetaDataLoader does is that even if the constructor in question has no annotations, it still needs to instantiate the "empty" AnnotatedElementMetaDataLoader, which is quite costly. The bulk of that cost goes into creating the ScopeKey, and instantiating ScopeKey's ConcurrentSkipListMap and adding to it take up most of that time.
 
In an attempt to cut down on this cost I tried the following fix which cuts ~650ms off the AS boot time:
   public MetaDataRetrieval getComponentMetaDataRetrieval(Signature signature)
   {
      if (signature == null)
         return null;
 
      if (annotated instanceof Class)
      {
         Class<?> clazz = Class.class.cast(annotated);
         if (signature instanceof ConstructorSignature)
         {
            ConstructorSignature constructorSignature = (ConstructorSignature) signature;
            Constructor<?> constructor = constructorSignature.getConstructor();
            if (constructor == null)
               constructor = SecurityActions.findConstructor(clazz, signature.getParametersTypes(clazz));
            if (constructor == null)
            {
               if (log.isTraceEnabled())
                  log.trace("Constructor with signature " + signature + " does not exist on class " + clazz.getName());
               return null;
            }
 
            //Return null to avoid overhead of initializing AnnotatedElementMetaDataLoader's ScopeKey
            //which shows up to be a big bottleneck in AS startup time          
            if (constructor.getAnnotations().length == 0)
               return NullAnnotatedElementMetaDataLoader.INSTANCE;
 
            return new AnnotatedElementMetaDataLoader(constructor);
         }
         //Same for fields, methods, parameters

 
The problem with this is that it makes some tests fail in MDR, since they assume that there will always be a ComponentMetaData. Those tests are:
 
MetaDataAllTestSuite
MetaData Tests
MetaDataLoader Tests
Reflection MetaDataLoader Tests
org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase
testFieldEmpty(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testEmpty(ComponentBasicAnnotationsTest.java:102)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testFieldEmpty(ComponentBasicAnnotationsTest.java:69)
 
testConstructorEmpty(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testEmpty(ComponentBasicAnnotationsTest.java:102)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testConstructorEmpty(ComponentBasicAnnotationsTest.java:76)
 
testMethodEmpty(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testEmpty(ComponentBasicAnnotationsTest.java:102)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testMethodEmpty(ComponentBasicAnnotationsTest.java:83)
 
testMethodParamsEmpty(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testEmpty(ComponentBasicAnnotationsTest.java:102)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testMethodParamsEmpty(ComponentBasicAnnotationsTest.java:90)
 
testConstructorParamsEmpty(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testEmpty(ComponentBasicAnnotationsTest.java:102)
 at org.jboss.test.metadata.shared.ComponentBasicAnnotationsTest.testConstructorParamsEmpty(ComponentBasicAnnotationsTest.java:97)
 
org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderNotPublicUnitTestCase
testSameName(org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderNotPublicUnitTestCase)
junit.framework.AssertionFailedError: null
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.assertTrue(Assert.java:20)
 at junit.framework.Assert.assertNotNull(Assert.java:214)
 at junit.framework.Assert.assertNotNull(Assert.java:207)
 at org.jboss.test.metadata.AbstractMetaDataTest.assertNoAnnotation(AbstractMetaDataTest.java:226)
 at org.jboss.test.metadata.loader.reflection.test.AnnotatedElementLoaderNotPublicUnitTestCase.testSameName(AnnotatedElementLoaderNotPublicUnitTestCase.java:94)
 
-------------------
The question is do I "fix" those tests or not? ComponentBasicAnnotationsTest comes in two varieties, both of which run this test:
 
   public void testFieldEmpty() throws Exception
   {
      MetaData metaData = setupField();
      metaData = metaData.getComponentMetaData(new FieldSignature("empty"));
      testEmpty(metaData);
   }
 

 
* AnnotatedElementLoaderComponentBasicAnnotationsUnitTestCase which sets up the field "empty" as follows (FieldBean.empty has no annotations)
   protected MetaData setupField()
   {
      AnnotatedElementMetaDataLoader loader = new AnnotatedElementMetaDataLoader(FieldBean.class);
      return new MetaDataRetrievalToMetaDataBridge(loader);
   }

*  
MemoryLoaderComponentBasicAnnotationsUnitTestCase which sets up the "empty" field like this, which makes sure that there is a componentmetadata for it
   protected MetaData setupField()
   {
      MemoryMetaDataLoader loader = new MemoryMetaDataLoader();
      MemoryMetaDataLoader component = new MemoryMetaDataLoader();
      loader.addComponentMetaDataRetrieval(new FieldSignature("empty"), component);
 
      //Some more component MetaDataLoaders set up to test other "fields"
      ...
 
      return new MetaDataRetrievalToMetaDataBridge(loader);
 
   }
 

 
 
We're clearly not counting on there always being a metadata. In practice if there is no component metadata for a particular signature, MetaDataLoader will return null while AnnotatedElementLoader will not, meaning we have two different behaviours for the implemented interface. On the other hand, in the real world (read AS kernel) with things as they have been the component metadata returned to the user will never be null since there would always at least be a created component AnnotatedElementMetaDataLoader there regardless of if all the component MemoryMetaDataLoaders were null or not.

--------------------------------------------------------------

To reply to this message visit the message page: http://community.jboss.org/message/526629#526629




More information about the jboss-user mailing list