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