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

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