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

[David Holmes]

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 5/11/2012 9:09 AM, Peter Levart wrote:
> 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.
> */
>