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

[Martin Buchholz]

On Wed, Nov 7, 2012 at 11:02 AM, Peter Levart <peter.levart at gmail.com>wrote:

>  On 11/07/2012 06:59 PM, Martin Buchholz wrote:
>
>  We've also seen deadlocks in accessing annotations in the wild.
> So, many thanks for working on this (both performance improvements and
> deadlock removal).
> We don't have a test case to contribute, but here's a stacktrace:
>
> That's one thread. What about the other(s)?
>
>        sun.reflect.annotation.AnnotationType.getInstance(Annotation
Type.java:80)
       sun.reflect.annotation.AnnotationParser.parseAnnotation
(AnnotationParser.java:220)
       sun.reflect.annotation.AnnotationParser.parseAnnotations2
(AnnotationParser.java:87)
       sun.reflect.annotation.AnnotationParser.parseAnnotations
(AnnotationParser.java:70)
       java.lang.Class.initAnnotationsIfNecessary(Class.java:3093)
       java.lang.Class.getAnnotation(Class.java:3050)
       sun.reflect.annotation.AnnotationType.<init>(AnnotationType.java:130)
       sun.reflect.annotation.AnnotationType.getInstance(Annotation
Type.java:83)
       sun.reflect.annotation.AnnotationParser.parseAnnotation
(AnnotationParser.java:220)
       sun.reflect.annotation.AnnotationParser.parseAnnotations2
(AnnotationParser.java:87)
       sun.reflect.annotation.AnnotationParser.parseAnnotations
(AnnotationParser.java:70)
       java.lang.Class.initAnnotationsIfNecessary(Class.java:3093)
       java.lang.Class.getAnnotation(Class.java:3050)
       sun.reflect.annotation.AnnotationType.<init>(AnnotationType.java:130)
       sun.reflect.annotation.AnnotationType.getInstance(Annotation
Type.java:83)
       sun.reflect.annotation.AnnotationParser.parseAnnotation
(AnnotationParser.java:220)
       sun.reflect.annotation.AnnotationParser.parseAnnotations2
(AnnotationParser.java:87)
       sun.reflect.annotation.AnnotationParser.parseAnnotations
(AnnotationParser.java:70)
       java.lang.reflect.Field.declaredAnnotations(Field.java:1034)
       java.lang.reflect.Field.getAnnotation(Field.java:1018)


> I guess this is similar to http://bugs.sun.com/view_bug.do?bug_id=7122142
>
> Various locks are involved:
>
> - AnnotationType.getInstance is a static synchronized method (locks on
> AnnotationType.class)
>
> - Class.initAnnotationsIfNecessary is an instance synchronized method
> (locks on various .class instances)
>
> lazy initialization sequence can take various ordering combinations among
> threads (recursing on loading annotations on annotations - the meta
> annotations - when requested by the AnnotationType init).
>
> The proposed patch removes blocking synchronization on
> Class.initAnnotationsIfNecessary (replacing this method with another one
> that has no blocking synchronization). Therefore just one lock remains in
> such scenarios (AnnotationType.getInstance) and there's no dead-lock with
> just one lock ;-)
>

I haven't actually tried to fix it, but I've thought about it, also
thinking about replacing locks with lazy lock-free initialization, probably
using Unsafe to cas class metadata.