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

Alexander Knöller alexander.knoeller at gmail.com
Mon Nov 5 13:36:19 UTC 2012

Hi David.

What about my proposal for a simple double-checked-locking for the redefinitions fields and the use of local variables for "annotations" in getAnnotations() and initAnnotationsIfNecessary() ? Are additional local Variables similar "bad habit" for memory usage (although they only affect the stack)?

Alex

Am 05.11.2012 um 06:23 schrieb 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. */

More information about the core-libs-dev mailing list