An alternative to changing the tests (although I think we should agree once and for all what the getComponentMetadataRetrieval() should return) is to return a singleton:

   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;
            }
            if (constructor.getAnnotations().length == 0)
               return NullAnnotatedElementMetaDataLoader.INSTANCE;
            return new AnnotatedElementMetaDataLoader(constructor);
         }
         //Same for fields, methods, parameters
   }
 
   private static class NullAnnotatedElementMetaDataLoader implements MetaDataRetrieval
   {
      final static NullAnnotatedElementMetaDataLoader INSTANCE = new NullAnnotatedElementMetaDataLoader();
      final static ScopeKey NULL_SCOPE_KEY = new ScopeKey(new Scope(CommonLevels.JOINPOINT, INSTANCE));
      final static BasicAnnotationsItem NO_ANNOTATIONS_ITEM = new BasicAnnotationsItem(null, new AnnotationItem[0]);
      final static BasicMetaDatasItem NO_METADATAS_ITEM = new BasicMetaDatasItem(null, new MetaDataItem[0]);
      
      public MetaDataRetrieval getComponentMetaDataRetrieval(Signature signature)
      {
         return INSTANCE;
      }

      public ScopeKey getScope()
      {
         return NULL_SCOPE_KEY;
      }

      public MetaDataRetrieval getScopedRetrieval(ScopeLevel level)
      {
         return INSTANCE;
      }

      public ValidTime getValidTime()
      {
         return null;
      }

      public boolean isEmpty()
      {
         return true;
      }

      public <T extends Annotation> AnnotationItem<T> retrieveAnnotation(Class<T> annotationType)
      {
         return null;
      }

      public AnnotationsItem retrieveAnnotations()
      {
         return NO_ANNOTATIONS_ITEM;
      }

      public AnnotationsItem retrieveAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
      {
         return NO_ANNOTATIONS_ITEM;
      }

      public AnnotationsItem retrieveLocalAnnotations()
      {
         return NO_ANNOTATIONS_ITEM;
      }

      public MetaDatasItem retrieveLocalMetaData()
      {
         return NO_METADATAS_ITEM;
      }

      public MetaDatasItem retrieveMetaData()
      {
         return NO_METADATAS_ITEM;
      }

      public <T> MetaDataItem<T> retrieveMetaData(Class<T> type)
      {
         return null;
      }

      public MetaDataItem<?> retrieveMetaData(String name)
      {
         return null;
      }
   }

 

I am not sure about the ValidTime, and the ScopeKey seems to not be used, but all tests in mdr pass.

Both of these fixes give problems when running the kernel tests.

Returning null gives a few errors similar to the ones in MDR's tests, i.e. always expecting component metadata to be non-null in AnnotatedMethodTestCase and AnnotatedConstructorTestCase. This one line fix in AnnotatedMethodTestCase (and similar in AnnotatedConstructorTestCase) solves the problem and all the tests in kernel modules pass.

   private Annotation[] getComponentMetaData(KernelControllerContext context, Signature signature)
   {
      KernelMetaDataRepository repository = context.getKernel().getMetaDataRepository();
      MetaData metaData = repository.getMetaData(context);
      MetaData component = metaData.getComponentMetaData(signature);
      if (component == null)         //Added
         return new Annotation[0];   //Added
      return component.getAnnotations();
   }

Returning NullAnnotatedElementMetaDataLoader.INSTANCE gives loads of errors in the jboss-kernel project, but they might be due to my NullAEMDL making some wrong assumptions, so I'll have a look at that.

Ok, I'll go with the singleton. The problems I was seeing with returning the singleton was that its getValidTime() method returned null, adding a validTime field makes all tests in mdr and kernel pass. I'll add a simple test for that and commit what I have so far against https://jira.jboss.org/jira/browse/JBMDR-66

So far we avoid creating AnnotatedElementLoaders and ScopeKeys for the cases where a member does not have any annotations.

I think this could be taken a bit further, when a member has annotations, we currently create an AnnotatedElementLoader and thus a ScopeKey. As mentioned ScopeKeys are expensive to create due to creating their contained ConcurrentSkipListMap and this add call in the constructor which adds to the ConcurrentSkipListMap:

   public ScopeKey(Scope scope)
   {
      addScope(scope);
   }

   private static final ScopeKey getScopeKey(AnnotatedElement annotated)
   {
      Scope scope;
      if (annotated instanceof Class)
      {
         Class<?> clazz = Class.class.cast(annotated);
         scope = new Scope(CommonLevels.CLASS, clazz);
      }
      else if (annotated instanceof Member)
      {
         Member member = (Member) annotated;
         scope = new Scope(CommonLevels.JOINPOINT, member);
      }
      else
      {
         return ScopeKey.DEFAULT_SCOPE;
      }
      return new ScopeKey(scope);
   }

to

   private static final ScopeKey getScopeKey(AnnotatedElement annotated)
   {
      Scope scope;
      if (annotated instanceof Class)
      {
         Class<?> clazz = Class.class.cast(annotated);
         scope = new Scope(CommonLevels.CLASS, clazz);
         return new ScopeKey(scope);
      }
      else if (annotated instanceof Member)
      {
         Member member = (Member) annotated;
         scope = new Scope(CommonLevels.JOINPOINT, member);
         return new SingleLevelScopeKey(scope);
      }
      else
      {
         return ScopeKey.DEFAULT_SCOPE;
      }
   }

 

where SingelLevelScopeKey will be a lightweight implementation of ScopeKey that does not have the internal ConcurrentSkipListMap. I might even be able to use the existing UnmodifiableScopeKey, so I will look into that first.

I think this could this also be used for the CLASS scope?

I need to check, but will see if this could this also be used for the other metadata retrievals.

I changed it to use UnmodifiableScopeKey for both CLASS and JOINPOINT levels.

When creating UnmodifiableScopeKey, there was still some overhead in unnecessarily creating the parent ScopeKey.scopes map which will always be ignored, so I have made that lazily initialized in ScopeKey.addScope(Scope).

I did some AS starts with my initial implementation (AnnotatedElementMetaDataLoader.getComponentMetaDataRetrieval() returns null if no annotations) from this thread yesterday, and when rerunning them today and comparing with another AS instance with the alternative solution and rest of the fixes from this thread I found that JBoss AS with today's jboss-mdr snapshot starts up slower than yesterday's jboss-mdr snapshot by almost a second. I'll look into why.

I have done some benchmarks for the different solutions for AnnotatedElementMetaDataLoader.getComponentMetaDataRetrieval().

1. The worst performing is the original form, creating a new AEMDL when there are no annotations
2. The best performing is returning null when there are no annotations, taking half the time of 1
3. What we have now performs somewhere in between, about 1.5 times that of 2

In all cases the use of CachingMetaDataContext (which I create outside the benchmark, so this is for reads only) performs worse than the plain MetaDataContext, but I think it is needed for getAnnotationsAnnotatedWith()?

I have gone back to making AEMDL.getCMDR() return null if there are no annotations. It is the fastest way, and is the least affected by caching. To preserve backwards compatibility and to not have to change any tests, I have added this fix to make MetaDataRetrievalToMetaDataBridge always return something even if there is no component metadata:

   public MetaData getComponentMetaData(Signature signature)
   {
      MetaDataRetrieval component = retrieval.getComponentMetaDataRetrieval(signature);
      if (component == null)
         //return null;
         return NullComponentMetaData.INSTANCE;
      return new MetaDataRetrievalToMetaDataBridge(component);
   }
...
   private static class NullComponentMetaData implements MetaData
   {
      final static NullComponentMetaData INSTANCE = new NullComponentMetaData();

      public <T extends Annotation> T getAnnotation(Class<T> annotationType)
      {
         return null;
      }

      public Annotation[] getAnnotations()
      {
         return MetaData.NO_ANNOTATIONS;
      }

      public Annotation[] getAnnotationsAnnotatedWith(Class<? extends Annotation> meta)
      {
         return MetaData.NO_ANNOTATIONS;
      }

      public MetaData getComponentMetaData(Signature signature)
      {
         return null;
      }

      public Annotation[] getLocalAnnotations()
      {
         return MetaData.NO_ANNOTATIONS;
      }

      public Object[] getLocalMetaData()
      {
         return MetaData.NO_METADATA;
      }

      public <T> T getMetaData(Class<T> type)
      {
         return null;
      }

      public Object[] getMetaData()
      {
         return MetaData.NO_METADATA;
      }

      public Object getMetaData(String name)
      {
         return null;
      }

      public <T> T getMetaData(String name, Class<T> type)
      {
         return null;
      }

      public MetaData getScopeMetaData(ScopeLevel level)
      {
         return null;
      }

      public long getValidTime()
      {
         return 0;
      }

      public boolean isAnnotationPresent(Class<? extends Annotation> annotationType)
      {
         return false;
      }

      public boolean isEmpty()
      {
         return true;
      }

      public boolean isMetaDataPresent(Class<?> type)
      {
         return false;
      }

      public boolean isMetaDataPresent(String name)
      {
         return false;
      }

      public boolean isMetaDataPresent(String name, Class<?> type)
      {
         return false;
      }  
   }