8029019: (ann) Optimize annotation handling in core reflection

[Christoph Dreis]

Hi Peter,

I was very happy when I saw this on the mailing list today as I'm the one who brought this up again end of November. Thank you for taking the time! I hope you don't mind my comments on this one.

> This is a preview of a patch that addresses the following issue:
> 
>      https://bugs.openjdk.java.net/browse/JDK-8029019
> 
> But it is also an attempt to clean-up failure reporting for invalid
> annotation types. Here's the 1st webrev:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-
> dev/8029019_Annotations.optimization/webrev.01/
> 
> With this patch applied, running the following benchmark:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-
> dev/8029019_Annotations.optimization/AnnotationsBench.java
> 
> Produces the following results:
> 
> Original:
> 
> Benchmark                                 Mode  Cnt Score     Error  Units
> AnnotationsBench.a1_PrimitiveMember       avgt   10 20.790 ±   1.317  ns/op
> AnnotationsBench.a2_ObjectMember          avgt   10 21.550 ±   0.580  ns/op
> AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 28.314 ±   1.477 ns/op
> AnnotationsBench.a4_ObjectArrayMember     avgt   10 27.094 ±   0.574  ns/op
> AnnotationsBench.a5_AnnotationType        avgt   10 13.047 ±   0.983  ns/op
> AnnotationsBench.a6_HashCode              avgt   10 158.001 ±   9.347  ns/op
> AnnotationsBench.a7_Equals                avgt   10 663.921 ±  27.256  ns/op
> AnnotationsBench.a8_ToString              avgt   10 1772.901 ± 107.355 ns/op
> 
> Patched:
> 
> Benchmark                                 Mode  Cnt Score    Error  Units
> AnnotationsBench.a1_PrimitiveMember       avgt   10 8.547 ±  0.100  ns/op
> AnnotationsBench.a2_ObjectMember          avgt   10 8.352 ±  0.340  ns/op
> AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 17.526 ±  0.696  ns/op
> AnnotationsBench.a4_ObjectArrayMember     avgt   10 15.639 ±  0.116  ns/op
> AnnotationsBench.a5_AnnotationType        avgt   10 4.692 ±  0.154  ns/op
> AnnotationsBench.a6_HashCode              avgt   10 142.954 ±  7.460  ns/op
> AnnotationsBench.a7_Equals                avgt   10 184.535 ±  0.917  ns/op
> AnnotationsBench.a8_ToString              avgt   10 1806.685 ± 96.370  ns/op
>

Nice results! As most of the garbage in my applications (and presumably other Spring applications) comes from calling annotationType() and the unnecessary cloning of parameter types, I wonder why the patched version doesn't have a larger impact on toString() (or hashCode()) as well. toString() seems to be even slightly slower. Any ideas here?

Regardless, I found a possible improvement for toString() in case it needs to convert Class members (around L274-283 in the patch). Instead of creating a StringBuilder for the array brackets only we could use the instance to build up the whole string instead of (presumably) creating another one at the end.

==== PATCH  =====
    private static String toSourceString(Class<?> clazz) {
        Class<?> finalComponent = clazz;
        StringBuilder sb = new StringBuilder();

        while(finalComponent.isArray()) {
            finalComponent = finalComponent.getComponentType();
            sb.append("[]");
        }

        sb.insert(0, finalComponent.getName());
        sb.append(".class");

        return sb.toString();
    }
 
> 
> 
> As for the failure reporting: requesting an annotation for invalid
> annotation type now always produces AnnotationFormatError. Previously,
> some invalid annotations were simply skipped, some produced
> AnnotationFormatError and for some of them, AnnotationFormatError was
> raised only when evaluating Annotation's equals() method.
> 
> I would like to discuss this failure behavior more in-depth.

As I'm fairly new to the OpenJDK project, I can't say too much (of value) about possible implications of this change, but for consistency reasons I like this very much. Though my gut feeling tells me that this might be too big of a (behaviour) change that might prevent the other optimizations from coming in. At least that's my current fear as one of the guys that is actually heavily affected by the performance bug since JDK 8.   

Cheers,
Christoph