Peter, > On 2017-01-17 15:10, Peter Levart wrote: >> Hi, >> >> This is a preview of a patch that addresses the following issue: >> >> https://bugs.openjdk.java.net/browse/JDK-8029019
This very good work. I have not done a complete review, but what jumps out at me is that you have removed some of the validation code and checks from AnnotationInvocationHandler ( in one place there is an assert and a comment that “good” data is expected. AnnotationInvocationHandler has shown up in many interesting places, I wonder if it is wise to remove these. -Chris. >> 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 >> >> >> Allocation rates are also reduced: >> >> Original: >> >> AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt 10 >> 16.000 ± 0.001 B/op >> AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt 10 16.000 >> ± 0.001 B/op >> AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt >> 10 64.000 ± 0.001 B/op >> AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt 10 >> 48.000 ± 0.001 B/op >> AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt 10 >> 16.000 ± 0.001 B/op >> AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt 10 128.000 ± >> 0.001 B/op >> AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt 10 120.001 ± >> 0.001 B/op >> AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt 10 5200.002 ± >> 0.001 B/op >> >> Patched: >> >> AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt 10 ≈ >> 10⁻⁵ B/op >> AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt 10 ≈ >> 10⁻⁵ B/op >> AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt >> 10 48.000 ± 0.001 B/op >> AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt 10 >> 32.000 ± 0.001 B/op >> AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt 10 ≈ >> 10⁻⁵ B/op >> AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt 10 64.000 ± >> 0.001 B/op >> AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt 10 24.000 ± >> 0.001 B/op >> AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt 10 5136.002 ± >> 0.001 B/op >> >> >> 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. >> >> Regards, Peter >>