[NEW BUG]: Reduce allocations in sun.reflect.annotation.AnnotationInvocationHandler.invoke()

[Claes Redestad]

Hi,

for general guidelines on contributing: 
http://openjdk.java.net/contribute/ [1]

As for the patch I think it looks good, but out of curiosity I wonder if
you have any numbers on how much allocations you see in these methods and
how much this patch helps? A non-escaping clone like this should typically
be possible for the JIT to escape entirely, but if that's not happening I
don't see why we should be opposed to a simple optimization like this.

Once changes suggested by Peter Levart is pushed[2] we could improve this
further by using LangReflectAccess.getExecutableSharedParameterTypes, which
will get direct access to the array and thus help the slow case too (if that
matters).

Is it perhaps worth changing ordering around while we're at it?

   if (parameterCount == 1 && member.equals("equals") && ...

Thanks!

/Claes

[1] Someone correct me if I'm wrong, but I've heard somewhere that for a
trivial patch contribution like this signing an OCA is not strictly 
necessary.
Regardless, providing the patch attached or inline is enough to allow us to
sponsor it for you.

[2]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-October/043932.html
http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.07/

On 2016-11-28 13:57, Christoph Dreis wrote:
> Hey,
>
>   
>
> I'm new to the OpenJDK and not sure (yet) how the procedure especially for
> new bugs/enhancement is. So I apologise upfront if I made any mistakes.
>
>   
>
> I'm working mostly with the Spring-Framework/Spring-Boot in my current
> projects. In these frameworks a lot of dynamic proxying can happen.
> Recently, I noticed that the JDK in versions 8 up to the current snapshots
> produces some allocations coming from
> sun.reflect.annotation.AnnotationInvocationHandler.invoke() we could avoid
> in the majority of cases. Only the check for equality needs the actual
> parameter types - all other cases only need the parameter count which JDK 8
> luckily provides with Method.getParameterCount().
>
>   
>
> What about changing the current behaviour to something like my attached
> proposal? I'd be happy if someone is willing to sponsor this change. Again -
> if I made any mistakes here, please let me know for the next time.
>
>   
>
> Cheers,
>
> Christoph Dreis
>
>   
>
> ================================
>
> Reduce allocations in
> sun.reflect.annotation.AnnotationInvocationHandler.invoke()
>
>   
>
> diff -r ba70dcd8de76 -r 86bbc5442c1d
> src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandl
> er.java
>
> ---
> a/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHan
> dler.java     Fri Nov 11 13:11:27 2016 +0000
>
> +++
> b/src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHan
> dler.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 (member.equals("equals") && parameterCount == 1 &&
>
> +            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) {
>
>   
>
>   
>