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.dr...@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.dr...@freenet.de] > > Sent: Friday, February 10, 2017 11:22 AM > > To: 'Claes Redestad' <claes.redes...@oracle.com>; 'Peter Levart' > > <peter.lev...@gmail.com>; 'Core-Libs-Dev' <core-libs- > > d...@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) { > >
