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

[Joel Borggrén-Franck]

This looks fine to me. None of the gotchas I could remember that may or may
not apply to Peters bigger change  applies to this one.

You should get a second opinon from Joe or Peter though.

cheers
/Joel

On Tue, 10 Oct 2017 at 20:02 Christoph Dreis <christoph.dreis at freenet.de>
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) {
>
>