RFR: 8029019: (ann) Optimize annotation handling in core reflection

Peter Levart peter.levart at gmail.com
Wed Oct 11 20:55:10 UTC 2017


Hi Christoph,

I agree to bring-in optimizations in small chunks. So your proposed 
handler invoke() method is as follows:

     public Object invoke(Object proxy, Method method, Object[] args) {
         String member = method.getName();
         int parameterCount = method.getParameterCount();

         // Handle Object and Annotation methods
         if (parameterCount == 1 && member.equals("equals") &&
             method.getParameterTypes()[0] == Object.class)
             return equalsImpl(proxy, args[0]);
         if (parameterCount != 0)
             throw new AssertionError("Too many parameters for an 
annotation method");

         switch(member) {
         case "toString":
             return toStringImpl();
         case "hashCode":
             return hashCodeImpl();
         case "annotationType":
             return type;
         }

         // Handle annotation member accessors
         Object result = memberValues.get(member);

         if (result == null)
             throw new IncompleteAnnotationException(type, member);

         if (result instanceof ExceptionProxy)
             throw ((ExceptionProxy) result).generateException();

         if (result.getClass().isArray() && Array.getLength(result) != 0)
             result = cloneArray(result);

         return result;
     }


This is good, because it only retrieves/clones the parameterTypes array 
for equals() method invocation. But we might do even slightly better if 
we consider the following:

- Method.getName() returns an interned String and String literals are 
interned strings. Reference comparison is therefore possible
- The pair (declaringClass, methodName) uniquely identifies the method 
for a bunch of interesting methods when declaringClass is either 
java.lang.Object or java.lang.annotation.Annotation, so we don't need to 
obtain method's parameterTypes even for equals().


For example:

     public Object invoke(Object proxy, Method method, Object[] args) {

         String memberName = method.getName(); // guaranteed interned String
         Class<?> dtype = method.getDeclaringClass();

         if (dtype == Object.class ||  // equals/hashCode/toString
             dtype == Annotation.class // 
annotationType/equals/hashCode/toString
             ) {
                 assert memberName == "equals" && 
method.getParameterCount() == 1 ||
                        memberName != "equals" && 
method.getParameterCount() == 0;

             if (memberName == "equals") return equalsImpl(proxy, args[0]);
             if (memberName == "hashCode") return hashCodeImpl();
             if (memberName == "annotationType") return type;
             if (memberName == "toString") return toStringImpl();

             // unreachable statement
             throw new AssertionError("Invalid method: " + method);
         }

         assert dtype == type;
         assert method.getParameterCount() == 0;

         // Handle annotation member accessors
         Object result = memberValues.get(memberName);

         if (result == null)
             throw new IncompleteAnnotationException(type, memberName);

         if (result instanceof ExceptionProxy)
             throw ((ExceptionProxy) result).generateException();

         if (result.getClass().isArray() && Array.getLength(result) != 0)
             result = cloneArray(result);

         return result;
     }


What do you think?

Regards, Peter

On 10/10/17 19:59, Christoph Dreis wrote:
> Hi Claes,
>
> with JDK 9 being out - congrats to everyone - I'll try my chance to raise issue https://bugs.openjdk.java.net/browse/JDK-8029019 again.
>
> A little recap: In November 2016 I reported the unnecessary allocation of Method.getParameterTypes() in AnnotationInvocationHandler.invoke() and a patch that would fix this. As it turned out, this and other improvements were already suggested by Peter in above mentioned ticket. Because it was reasonably late in February 2017 to cover the complete story for 9, I suggested to only include the low-risk patch, which was understandably also too late.
>
> So I'm trying again to bring this into the JDK - I hope you don't mind me being so persistent on this one.
>
> For completeness find the initial patch attached below for the initial problem I was trying to solve.
>
> Cheers,
> Christoph
>
> ================================
> Reduce allocations in sun.reflect.annotation.AnnotationInvocationHandler.invoke()
>
> diff -r ba70dcd8de76 -r 86bbc5442c1d src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
> --- a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java     Fri Nov 11 13:11:27 2016 +0000
> +++ b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java  Mon Nov 14 19:04:33 2016 +0100
> @@ -57,13 +57,13 @@
>
>       public Object invoke(Object proxy, Method method, Object[] args) {
>           String member = method.getName();
> -        Class<?>[] paramTypes = method.getParameterTypes();
> +        int parameterCount = method.getParameterCount();
>
>           // Handle Object and Annotation methods
> -        if (member.equals("equals") && paramTypes.length == 1 &&
> -            paramTypes[0] == Object.class)
> +        if (parameterCount == 1 && member.equals("equals") &&
> +            method.getParameterTypes()[0] == Object.class)
>               return equalsImpl(proxy, args[0]);
> -        if (paramTypes.length != 0)
> +        if (parameterCount != 0)
>               throw new AssertionError("Too many parameters for an annotation method");
>
>           switch(member) {
>
>> -----Original Message-----
>> From: Christoph Dreis [mailto:christoph.dreis at freenet.de]
>> Sent: Friday, February 10, 2017 11:22 AM
>> To: 'Claes Redestad' <claes.redestad at oracle.com>; 'Peter Levart'
>> <peter.levart at gmail.com>; 'Core-Libs-Dev' <core-libs-
>> dev at openjdk.java.net>
>> Subject: RE: RFR: 8029019: (ann) Optimize annotation handling in core
>> reflection
>>
>> Hi Claes,
>>
>>> I think this all seems reasonable, but subtle behavior changes like
>>> this needs more scrutiny than I can provide.  I've discussed this
>>> offline with Joe and sadly concluded it's probably too much, too late
>>> for 9 at this point.
>>>
>>> Hope you don't mind re-targetting this to JDK 10.
>> Do you mean the error handling change or the ticket in general? In case of
>> the latter, may I start a proposal?
>>
>> As the original issue for me was to reduce the allocations coming from
>> AnnotationInvocationHandler.invoke() caused by
>> Method.getParameterTypes(), what about creating a sub-ticket of 8029019
>> and just take care of the low-risk change below? That would eventually allow
>> a backbort to JDK 8 as well, although I'm not quite sure yet how the backport
>> process is working.
>>
>> What do you think?
>>
>> Cheers,
>> Christoph
>>
>> ========= PATCH ===========
>> diff --git
>> a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocation
>> Handler.java
>> b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocation
>> Handler.java
>> ---
>> a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocation
>> Handler.java
>> +++ b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvoc
>> +++ ationHandler.java
>> @@ -57,13 +57,13 @@
>>
>>       public Object invoke(Object proxy, Method method, Object[] args) {
>>           String member = method.getName();
>> -        Class<?>[] paramTypes = method.getParameterTypes();
>> +        int parameterCount = method.getParameterCount();
>>
>>           // Handle Object and Annotation methods
>> -        if (member.equals("equals") && paramTypes.length == 1 &&
>> -            paramTypes[0] == Object.class)
>> +        if (parameterCount == 1 && member.equals("equals") &&
>> +            method.getParameterTypes()[0] == Object.class)
>>               return equalsImpl(proxy, args[0]);
>> -        if (paramTypes.length != 0)
>> +        if (parameterCount != 0)
>>               throw new AssertionError("Too many parameters for an annotation
>> method");
>>
>>           switch(member) {



More information about the core-libs-dev mailing list