bottleneck by java.lang.Class.getAnnotations() - a better patch

Peter Levart peter.levart at gmail.com
Tue Nov 6 13:12:24 UTC 2012


On 11/05/2012 06:23 AM, David Holmes wrote:
> Hi Peter,
>
> Moving the annotations fields into a helper object would tie in with 
> the Class-instance size reduction effort that was investigated as part 
> of "JEP 149: Reduce Core-Library Memory Usage":
>
> http://openjdk.java.net/jeps/149
>
> The investigations there to date only looked at relocating the 
> reflection related fields, though the JEP mentions the annotations as 
> well.
>
> Any such effort requires extensive benchmarking and performance 
> analysis before being accepted though.
>
> David
> -----
>

On 11/05/2012 10:25 AM, Alan Bateman wrote:
> Here's a good starting place on the interaction of runtime visible 
> attributes and RedefineClasses/RetransformClasses:
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5002251
>
> -Alan.

Hi all,

Presented here is a patch mainly against java.lang.Class and also 
against java.lang.reflect.[Field,Method,Constructor,Executable] classes.

Currently java.lang.Class uses the following fields to maintain caches 
of reflection data that are invalidated as a result of class or 
superclass redefinition/re-transformation:

     private volatile transient SoftReference<Field[]> declaredFields;
     private volatile transient SoftReference<Field[]> publicFields;
     private volatile transient SoftReference<Method[]> declaredMethods;
     private volatile transient SoftReference<Method[]> publicMethods;
     private volatile transient SoftReference<Constructor<T>[]> 
declaredConstructors;
     private volatile transient SoftReference<Constructor<T>[]> 
publicConstructors;
     private volatile transient SoftReference<Field[]> declaredPublicFields;
     private volatile transient SoftReference<Method[]> 
declaredPublicMethods;

     // Value of classRedefinedCount when we last cleared the cached values
     // that are sensitive to class redefinition.
     private volatile transient int lastRedefinedCount = 0;

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

If I understand Alan's references correctly, current VM can redefine the 
class in a way that changes method bodies. Also new methods can be 
added. And the set of annotations can also be altered. And future 
improvements could allow even more.

Because annotations are cached on Field/Method/Constructor instances, 
all the above fields must be invalidated when the class or superclass is 
redefined.

It can also be observed that Field/Method/Constructor caches are 
maintained using SoftReferences but annotations are hard references. I 
don't know if this is intentional. I believe that annotations could also 
be SoftReferenced, so that in the event of memory pressure they get 
cleared. Many applications retrieve annotations only in the early stages 
of their life-cycle and then either cache them themselves or forget 
about them.

So I designed the patch to equalize this. If this is undesirable, the 
patch could be modified to make a distinction again.

The patch replaces the above-mentioned java.lang.Class fields with a 
single field:

     private volatile transient SoftReference<VolatileData<T>> volatileData;

...which is a SoftReference to the following structure:

     // volatile data that might get invalid when JVM TI 
RedefineClasses() is called
     static class VolatileData<T> {
         volatile Field[] declaredFields;
         volatile Field[] publicFields;
         volatile Method[] declaredMethods;
         volatile Method[] publicMethods;
         volatile Constructor<T>[] declaredConstructors;
         volatile Constructor<T>[] publicConstructors;
         // Intermediate results for getFields and getMethods
         volatile Field[] declaredPublicFields;
         volatile Method[] declaredPublicMethods;
         // Annotations
         volatile Map<Class<? extends Annotation>, Annotation> annotations;
         volatile Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
         // Value of classRedefinedCount when we created this 
VolatileData instance
         final int redefinedCount;

So let's look at static overhead differences using 64 bit addressing 
(useful load - arrays, Maps not counted since the patched code uses the 
same amount of same types of each).

* Fresh java.lang.Class instance:

current JDK8 code:

10 OOPs + 1 int = 10*8+4 = 84 bytes in 1 instance

vs. patched code :

1 OOP = 8 bytes in 1 instance

* Fully loaded java.lang.Class (Fields, Methods, Constructors, annotations):

current JDK8 code:

10 OOPs + 1 int = 84 bytes
8 SoftReference instances = 8*(header + 4 OOPs + 1 long) = 8*(24+32+8) = 
8*64 = 512 bytes
total: 84+512 = 596 bytes in 9 instances

vs. patched code :

1 OOP = 8 bytes
1 SoftReference = 64 bytes
1 VolatileData = header + 10 OOPs + 1 int = 24+84 = 108 bytes
total: 8+64+108 = 180 bytes in 3 instances

So we have 84 vs. 8 (empty) or 596 vs. 180 (fully loaded) byte overheads and
1 vs. 1 (empty) or 9 vs. 3 (fully loaded) instance overheads

Other than that, the patch also removes synchronized blocks for lazy 
initialization of annotations in Class, Field, Method and Constructor 
and replaces them with volatile fields. In case of Class.volatileData, 
this field is initialized using a CAS so there is no race which could 
install an already stale instance over more recent. Although such race 
would quickly be corrected at next call to any retrieval method, because 
redefinedCount is now an integral part of the cached structure not an 
individual volatile field.

There is also a change in how annotations are cached in Field, Method 
and Constructor. Originally they are cached in each copy of the 
Field/Method/Constructor that is returned to the outside world at each 
invocation of Class.getFields() etc. Such caching is not very effective 
if the annotations are only retrieved once per instance. The patch 
changes this and delegates caching to the "root" instance which is held 
inside Class so caching becomes more effective in certain usage 
patterns. There's now a possible hot-spot on the "root" instance but 
that seems not to be a bottleneck since the fast-path does not involve 
blocking synchronization (just volatile read). The effects of this 
change are clearly visible in one of the benchmarks.

I have tried to create 3 micro benchmarks which exercise concurrent load 
on 3 Class instances.

Here's the benchmark code:

https://raw.github.com/plevart/jdk8-hacks/master/src/test/ReflectionTest.java

And here are the results when run on an Intel i7 CPU (4 cores, 2 
threads/core) Linux machine using -Xmx4G VM option:

https://raw.github.com/plevart/jdk8-hacks/master/benchmark_results.txt


The huge difference of Test1 results is a direct consequence of patched 
code delegating caching of annotations in Field/Method/Constructor to 
the "root" instance.

Test2 results show no noticeable difference between original and patched 
code. This, I believe, is the most common usage of the API, so another 
level of indirection does not appear to present any noticeable 
performance overhead.

The Test3 on the other hand shows the synchronization overhead of 
current jdk8 code in comparison with non-blocking synchronization in 
patched code.

JEP 149 also mentions testing with SPECjbb2005 and SPECjvm98, but that 
exceeds my possibilities.

The patch against jdk8/jdk8/jdk hg repository is here:

https://raw.github.com/plevart/jdk8-hacks/master/volatile_class_data_caching.patch

You can also browse the changed sources:

https://github.com/plevart/jdk8-hacks

For completeness I also paste the patch below.

Regards, Peter


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    Tue Nov 06 14:11:43 2012 
+0100
@@ -2212,39 +2212,72 @@

      // Caches for certain reflective results
      private static boolean useCaches = true;
-    private volatile transient SoftReference<Field[]> declaredFields;
-    private volatile transient SoftReference<Field[]> publicFields;
-    private volatile transient SoftReference<Method[]> declaredMethods;
-    private volatile transient SoftReference<Method[]> publicMethods;
-    private volatile transient SoftReference<Constructor<T>[]> 
declaredConstructors;
-    private volatile transient SoftReference<Constructor<T>[]> 
publicConstructors;
-    // Intermediate results for getFields and getMethods
-    private volatile transient SoftReference<Field[]> declaredPublicFields;
-    private volatile transient SoftReference<Method[]> 
declaredPublicMethods;
+
+    // volatile data that might get invalid when JVM TI 
RedefineClasses() is called
+    static class VolatileData<T> {
+        volatile Field[] declaredFields;
+        volatile Field[] publicFields;
+        volatile Method[] declaredMethods;
+        volatile Method[] publicMethods;
+        volatile Constructor<T>[] declaredConstructors;
+        volatile Constructor<T>[] publicConstructors;
+        // Intermediate results for getFields and getMethods
+        volatile Field[] declaredPublicFields;
+        volatile Method[] declaredPublicMethods;
+        // Annotations
+        volatile Map<Class<? extends Annotation>, Annotation> annotations;
+        volatile Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
+        // Value of classRedefinedCount when we created this 
VolatileData instance
+        final int redefinedCount;
+
+        VolatileData(int redefinedCount) {
+            this.redefinedCount = redefinedCount;
+        }
+
+        // initialize Unsafe machinery here, since we need to call 
Class.class instance method and would like to avoid
+        // calling it in the static initializer of the Class class...
+        private static final Unsafe unsafe;
+        // offset of Class.volatileData instance field
+        private static final long volatileDataOffset;
+
+        static {
+            unsafe = Unsafe.getUnsafe();
+            // bypass caches
+            Field volatileDataField = 
searchFields(Class.class.getDeclaredFields0(false), "volatileData");
+            if (volatileDataField == null) throw new Error("No 
volatileData field found in java.lang.Class");
+            volatileDataOffset = 
unsafe.objectFieldOffset(volatileDataField);
+        }
+
+        static <T> boolean compareAndSwap(Class<?> clazz, 
SoftReference<VolatileData<T>> oldData, SoftReference<VolatileData<T>> 
newData) {
+            return unsafe.compareAndSwapObject(clazz, 
volatileDataOffset, oldData, newData);
+        }
+    }
+
+    private volatile transient SoftReference<VolatileData<T>> volatileData;

      // Incremented by the VM on each call to JVM TI RedefineClasses()
      // that redefines this class or a superclass.
      private volatile transient int classRedefinedCount = 0;

-    // Value of classRedefinedCount when we last cleared the cached values
-    // that are sensitive to class redefinition.
-    private volatile transient int lastRedefinedCount = 0;
+    // Lazily create and cache VolatileData
+    private VolatileData<T> volatileData() {
+        if (!useCaches) return null;

-    // Clears cached values that might possibly have been obsoleted by
-    // a class redefinition.
-    private void clearCachesOnClassRedefinition() {
-        if (lastRedefinedCount != classRedefinedCount) {
-            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
-            // 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.
-            lastRedefinedCount = classRedefinedCount;
+        while (true)
+        {
+            SoftReference<VolatileData<T>> volatileData = 
this.volatileData;
+            int classRedefinedCount = this.classRedefinedCount;
+            VolatileData<T> vd;
+            if (volatileData != null && (vd = volatileData.get()) != 
null && vd.redefinedCount == classRedefinedCount) {
+                return vd;
+            }
+            // no SoftReference or cleared SoftReference or stale 
VolatileData
+            vd = new VolatileData<T>(classRedefinedCount);
+            // try to CAS it...
+            if (VolatileData.compareAndSwap(this, volatileData, new 
SoftReference<VolatileData<T>>(vd))) {
+                return vd;
+            }
+            // else retry
          }
      }

@@ -2288,26 +2321,18 @@
      private Field[] privateGetDeclaredFields(boolean publicOnly) {
          checkInitted();
          Field[] res = null;
-        if (useCaches) {
-            clearCachesOnClassRedefinition();
-            if (publicOnly) {
-                if (declaredPublicFields != null) {
-                    res = declaredPublicFields.get();
-                }
-            } else {
-                if (declaredFields != null) {
-                    res = declaredFields.get();
-                }
-            }
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = publicOnly ? vd.declaredPublicFields : vd.declaredFields;
              if (res != null) return res;
          }
          // No cached value available; request value from VM
          res = Reflection.filterFields(this, 
getDeclaredFields0(publicOnly));
-        if (useCaches) {
+        if (vd != null) {
              if (publicOnly) {
-                declaredPublicFields = new SoftReference<>(res);
+                vd.declaredPublicFields = res;
              } else {
-                declaredFields = new SoftReference<>(res);
+                vd.declaredFields = res;
              }
          }
          return res;
@@ -2319,11 +2344,9 @@
      private Field[] privateGetPublicFields(Set<Class<?>> 
traversedInterfaces) {
          checkInitted();
          Field[] res = null;
-        if (useCaches) {
-            clearCachesOnClassRedefinition();
-            if (publicFields != null) {
-                res = publicFields.get();
-            }
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = vd.publicFields;
              if (res != null) return res;
          }

@@ -2356,8 +2379,8 @@

          res = new Field[fields.size()];
          fields.toArray(res);
-        if (useCaches) {
-            publicFields = new SoftReference<>(res);
+        if (vd != null) {
+            vd.publicFields = res;
          }
          return res;
      }
@@ -2381,17 +2404,9 @@
      private Constructor<T>[] privateGetDeclaredConstructors(boolean 
publicOnly) {
          checkInitted();
          Constructor<T>[] res = null;
-        if (useCaches) {
-            clearCachesOnClassRedefinition();
-            if (publicOnly) {
-                if (publicConstructors != null) {
-                    res = publicConstructors.get();
-                }
-            } else {
-                if (declaredConstructors != null) {
-                    res = declaredConstructors.get();
-                }
-            }
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = publicOnly ? vd.publicConstructors : 
vd.declaredConstructors;
              if (res != null) return res;
          }
          // No cached value available; request value from VM
@@ -2402,11 +2417,11 @@
          } else {
              res = getDeclaredConstructors0(publicOnly);
          }
-        if (useCaches) {
+        if (vd != null) {
              if (publicOnly) {
-                publicConstructors = new SoftReference<>(res);
+                vd.publicConstructors = res;
              } else {
-                declaredConstructors = new SoftReference<>(res);
+                vd.declaredConstructors = res;
              }
          }
          return res;
@@ -2424,26 +2439,18 @@
      private Method[] privateGetDeclaredMethods(boolean publicOnly) {
          checkInitted();
          Method[] res = null;
-        if (useCaches) {
-            clearCachesOnClassRedefinition();
-            if (publicOnly) {
-                if (declaredPublicMethods != null) {
-                    res = declaredPublicMethods.get();
-                }
-            } else {
-                if (declaredMethods != null) {
-                    res = declaredMethods.get();
-                }
-            }
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = publicOnly ? vd.declaredPublicMethods : 
vd.declaredMethods;
              if (res != null) return res;
          }
          // No cached value available; request value from VM
          res = Reflection.filterMethods(this, 
getDeclaredMethods0(publicOnly));
          if (useCaches) {
              if (publicOnly) {
-                declaredPublicMethods = new SoftReference<>(res);
+                vd.declaredPublicMethods = res;
              } else {
-                declaredMethods = new SoftReference<>(res);
+                vd.declaredMethods = res;
              }
          }
          return res;
@@ -2546,11 +2553,9 @@
      private Method[] privateGetPublicMethods() {
          checkInitted();
          Method[] res = null;
-        if (useCaches) {
-            clearCachesOnClassRedefinition();
-            if (publicMethods != null) {
-                res = publicMethods.get();
-            }
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = vd.publicMethods;
              if (res != null) return res;
          }

@@ -2558,7 +2563,7 @@
          // Start by fetching public declared methods
          MethodArray methods = new MethodArray();
          {
-                Method[] tmp = privateGetDeclaredMethods(true);
+            Method[] tmp = privateGetDeclaredMethods(true);
              methods.addAll(tmp);
          }
          // Now recur over superclass and direct superinterfaces.
@@ -2598,8 +2603,8 @@
          methods.addAllIfNotPresent(inheritedMethods);
          methods.compactAndTrim();
          res = methods.getArray();
-        if (useCaches) {
-            publicMethods = new SoftReference<>(res);
+        if (vd != null) {
+            vd.publicMethods = res;
          }
          return res;
      }
@@ -2609,7 +2614,7 @@
      // Helpers for fetchers of one field, method, or constructor
      //

-    private Field searchFields(Field[] fields, String name) {
+    private static Field searchFields(Field[] fields, String name) {
          String internedName = name.intern();
          for (int i = 0; i < fields.length; i++) {
              if (fields[i].getName() == internedName) {
@@ -3049,8 +3054,7 @@
          if (annotationClass == null)
              throw new NullPointerException();

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

      /**
@@ -3070,41 +3074,45 @@
       * @since 1.5
       */
      public Annotation[] getAnnotations() {
-        initAnnotationsIfNecessary();
-        return AnnotationParser.toArray(annotations);
+        return AnnotationParser.toArray(privateGetAnnotations(false));
      }

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

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

-    private synchronized void initAnnotationsIfNecessary() {
-        clearCachesOnClassRedefinition();
-        if (annotations != null)
-            return;
-        declaredAnnotations = AnnotationParser.parseAnnotations(
+    private Map<Class<? extends Annotation>, Annotation> 
privateGetAnnotations(boolean declaredOnly) {
+        Map<Class<? extends Annotation>, Annotation> res;
+        VolatileData<T> vd = volatileData();
+        if (vd != null) {
+            res = declaredOnly ? vd.declaredAnnotations : vd.annotations;
+            if (res != null) return res;
+        }
+
+        Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations = AnnotationParser.parseAnnotations(
              getRawAnnotations(), getConstantPool(), this);
+        Map<Class<? extends Annotation>, Annotation> annotations;
          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()) {
+            for (Map.Entry<Class<? extends Annotation>, Annotation> e : 
superClass.privateGetAnnotations(false).entrySet()) {
                  Class<? extends Annotation> annotationClass = e.getKey();
                  if 
(AnnotationType.getInstance(annotationClass).isInherited())
                      annotations.put(annotationClass, e.getValue());
              }
              annotations.putAll(declaredAnnotations);
          }
+
+        vd.annotations = annotations;
+        vd.declaredAnnotations = declaredAnnotations;
+
+        return declaredOnly ? declaredAnnotations : annotations;
      }

      // Annotation types cache their internal (AnnotationType) form
diff -r 7ac292e57b5a src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.java      Thu Nov 
01 14:12:21 2012 -0700
+++ b/src/share/classes/java/lang/reflect/Constructor.java      Tue Nov 
06 14:11:43 2012 +0100
@@ -482,7 +482,10 @@
       * @since 1.5
       */
      public Annotation[] getDeclaredAnnotations()  {
-        return super.getDeclaredAnnotations();
+        if (root != null)
+            return root.getDeclaredAnnotations();
+        else
+            return super.getDeclaredAnnotations();
      }

      /**
diff -r 7ac292e57b5a src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.java       Thu Nov 
01 14:12:21 2012 -0700
+++ b/src/share/classes/java/lang/reflect/Executable.java       Tue Nov 
06 14:11:43 2012 +0100
@@ -378,11 +378,12 @@
          return AnnotationParser.toArray(declaredAnnotations());
      }

-    private transient Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
+    private volatile transient Map<Class<? extends Annotation>, 
Annotation> declaredAnnotations;

-    private synchronized  Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations() {
+    private Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations() {
+        Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations = this.declaredAnnotations;
          if (declaredAnnotations == null) {
-            declaredAnnotations = AnnotationParser.parseAnnotations(
+            this.declaredAnnotations = declaredAnnotations = 
AnnotationParser.parseAnnotations(
                  getAnnotationBytes(),
                  sun.misc.SharedSecrets.getJavaLangAccess().
                  getConstantPool(getDeclaringClass()),
diff -r 7ac292e57b5a src/share/classes/java/lang/reflect/Field.java
--- a/src/share/classes/java/lang/reflect/Field.java    Thu Nov 01 
14:12:21 2012 -0700
+++ b/src/share/classes/java/lang/reflect/Field.java    Tue Nov 06 
14:11:43 2012 +0100
@@ -1027,11 +1027,15 @@
          return AnnotationParser.toArray(declaredAnnotations());
      }

-    private transient Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations;
+    private volatile transient Map<Class<? extends Annotation>, 
Annotation> declaredAnnotations;

-    private synchronized  Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations() {
+    private Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations() {
+        if (root != null)
+            return root.declaredAnnotations();
+
+        Map<Class<? extends Annotation>, Annotation> 
declaredAnnotations = this.declaredAnnotations;
          if (declaredAnnotations == null) {
-            declaredAnnotations = AnnotationParser.parseAnnotations(
+            this.declaredAnnotations = declaredAnnotations = 
AnnotationParser.parseAnnotations(
                  annotations, sun.misc.SharedSecrets.getJavaLangAccess().
                  getConstantPool(getDeclaringClass()),
                  getDeclaringClass());
diff -r 7ac292e57b5a src/share/classes/java/lang/reflect/Method.java
--- a/src/share/classes/java/lang/reflect/Method.java   Thu Nov 01 
14:12:21 2012 -0700
+++ b/src/share/classes/java/lang/reflect/Method.java   Tue Nov 06 
14:11:43 2012 +0100
@@ -583,7 +583,10 @@
       * @since 1.5
       */
      public Annotation[] getDeclaredAnnotations()  {
-        return super.getDeclaredAnnotations();
+        if (root != null)
+            return root.getDeclaredAnnotations();
+        else
+            return super.getDeclaredAnnotations();
      }

      /**





More information about the core-libs-dev mailing list