bottleneck by java.lang.Class.getAnnotations() - proposed patch

[Peter Levart]

Hi,

I propose the following patch to java.lang.Class in order to overcome 
the synchronization bottleneck when accessing annotations from multiple 
threads. The patch packs the 'annotations' and 'declaredAnnotations' 
Maps together with an int redefinedCount into a structure:

     static class Annotations {
         final Map<Class<? extends Annotation>, Annotation> annotations;
         final Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
         final int redefinedCount;

which is referenced by a single volatile Class.annotations field. 
Instead of  initAnnotationsIfNecessary() there's a 
getAnnotationsPrivate() method that uses simple double-checked locking 
to lazily initialize and return this structure. The slow-path part is 
still synchronized to ensure that 
Class.setAnnotationType/getAnnotationType call backs from AnnotationType 
are only done while holding a lock.

Regards, Peter

Here's the patch to jdk8 source:

diff -r 7ac292e57b5a src/share/classes/java/lang/Class.java
--- a/src/share/classes/java/lang/Class.java    Thu Nov 01 14:12:21 2012 
-0700
+++ b/src/share/classes/java/lang/Class.java    Sun Nov 04 23:53:04 2012 
+0100
@@ -2237,10 +2237,8 @@
              declaredFields = publicFields = declaredPublicFields = null;
              declaredMethods = publicMethods = declaredPublicMethods = 
null;
              declaredConstructors = publicConstructors = null;
-            annotations = declaredAnnotations = null;

-            // Use of "volatile" (and synchronization by caller in the case
-            // of annotations) ensures that no thread sees the update to
+            // Use of "volatile" ensures that no thread sees the update to
              // lastRedefinedCount before seeing the caches cleared.
              // We do not guard against brief windows during which multiple
              // threads might redundantly work to fill an empty cache.
@@ -3049,8 +3047,7 @@
          if (annotationClass == null)
              throw new NullPointerException();

-        initAnnotationsIfNecessary();
-        return (A) annotations.get(annotationClass);
+        return (A) 
getAnnotationsPrivate().annotations.get(annotationClass);
      }

      /**
@@ -3070,40 +3067,52 @@
       * @since 1.5
       */
      public Annotation[] getAnnotations() {
-        initAnnotationsIfNecessary();
-        return AnnotationParser.toArray(annotations);
+        return 
AnnotationParser.toArray(getAnnotationsPrivate().annotations);
      }

      /**
       * @since 1.5
       */
      public Annotation[] getDeclaredAnnotations()  {
- initAnnotationsIfNecessary();
-        return AnnotationParser.toArray(declaredAnnotations);
+        return 
AnnotationParser.toArray(getAnnotationsPrivate().declaredAnnotations);
}

      // Annotations cache
-    private transient Map<Class<? extends Annotation>, Annotation> 
annotations;
-    private transient Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
+    private transient volatile Annotations annotations;

-    private synchronized void initAnnotationsIfNecessary() {
-        clearCachesOnClassRedefinition();
-        if (annotations != null)
-            return;
-        declaredAnnotations = AnnotationParser.parseAnnotations(
-            getRawAnnotations(), getConstantPool(), this);
-        Class<?> superClass = getSuperclass();
-        if (superClass == null) {
-            annotations = declaredAnnotations;
-        } else {
-            annotations = new HashMap<>();
-            superClass.initAnnotationsIfNecessary();
-            for (Map.Entry<Class<? extends Annotation>, Annotation> e : 
superClass.annotations.entrySet()) {
-                Class<? extends Annotation> annotationClass = e.getKey();
-                if 
(AnnotationType.getInstance(annotationClass).isInherited())
-                    annotations.put(annotationClass, e.getValue());
+    private Annotations getAnnotationsPrivate() {
+        // double checked locking
+        int redefinedCount = classRedefinedCount;
+        Annotations anns = annotations;
+        if (anns != null && anns.redefinedCount == redefinedCount) {
+            return anns;
+        }
+
+        synchronized (this) {
+            redefinedCount = classRedefinedCount;
+            anns = annotations;
+            if (anns != null && anns.redefinedCount == redefinedCount) {
+                return anns;
              }
-            annotations.putAll(declaredAnnotations);
+
+            Map<Class<? extends Annotation>, Annotation> declAnnMap = 
AnnotationParser.parseAnnotations(
+                getRawAnnotations(), getConstantPool(), this);
+            Map<Class<? extends Annotation>, Annotation> annMap;
+            Class<?> superClass = getSuperclass();
+            if (superClass == null) {
+                annMap = declAnnMap;
+            } else {
+                annMap = new HashMap<>();
+                for (Map.Entry<Class<? extends Annotation>, Annotation> 
e : superClass.getAnnotationsPrivate().annotations.entrySet()) {
+                    Class<? extends Annotation> annotationClass = 
e.getKey();
+                    if 
(AnnotationType.getInstance(annotationClass).isInherited())
+                        annMap.put(annotationClass, e.getValue());
+                }
+                annMap.putAll(declAnnMap);
+            }
+
+            annotations = anns = new Annotations(annMap, declAnnMap, 
redefinedCount);
+            return anns;
          }
      }

@@ -3119,6 +3128,18 @@
          return annotationType;
      }

+    static final class Annotations {
+        final Map<Class<? extends Annotation>, Annotation> annotations;
+        final Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
+        final int redefinedCount;
+
+        Annotations(Map<Class<? extends Annotation>, Annotation> 
annotations, Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations, int redefinedCount) {
+            this.annotations = annotations;
+            this.declaredAnnotations = declaredAnnotations;
+            this.redefinedCount = redefinedCount;
+        }
+    }
+
      /* Backing store of user-defined values pertaining to this class.
       * Maintained by the ClassValue class.
       */