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
>> 

Reply via email to