On Fri, 20 Nov 2020 10:55:21 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> The trick is that if we know that a class like java.lang.invoke.Proxy may
>> exist,
>> it means that instead of distorting the j.l.r.Proxy API to increase of few
>> percents the performance when calling a default method, you can come with a
>> simpler design in term of API that just add an API point to call a default
>> method.
>>
>> Better performance becoming on the the goals of java.lang.invoke.Proxy.
>
> Hi Mandy,
> I re-ran the benchmark on your latest version (the static API) and I get
> similar results as you:
>
> Benchmark Mode Cnt Score Error Units
> ProxyBench.implClass avgt 5 3.745 ± 0.033 ns/op
> ProxyBench.implProxy avgt 5 29.826 ± 0.183 ns/op
> ProxyBench.ppImplClass avgt 5 3.683 ± 0.009 ns/op
> ProxyBench.ppImplProxy avgt 5 29.124 ± 0.535 ns/op
>
> I also tried a variant where the access check in the invokeDefault static
> method is not pre-screened with checking of the interface modifiers and
> package export status but relies on Method.checkAccess cache alone:
>
> // access check
> Class<?> caller = Reflection.getCallerClass();
> int modifiers = method.getModifiers();
> method.checkAccess(caller, intf, proxyClass, modifiers);
>
> ... and the results are not worse, even marginally better:
>
> Benchmark Mode Cnt Score Error Units
> ProxyBench.implClass avgt 5 3.724 ± 0.012 ns/op
> ProxyBench.implProxy avgt 5 29.138 ± 0.271 ns/op
> ProxyBench.ppImplClass avgt 5 3.744 ± 0.009 ns/op
> ProxyBench.ppImplProxy avgt 5 28.961 ± 0.182 ns/op
>
> I think this looks reasonably good.
In my previous attempts when I was modifying the ProxyGenerator I noticed that
generated proxy fields holding Method objects are just "static" but could be
"static final". If they were "static final", JIT could constant-fold the Method
instances. If this was combined with marking some fields in Method as @Stable,
this could improve optimization of code a bit further. I did this with the
following patch:
Index: src/java.base/share/classes/java/lang/reflect/InvocationHandler.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/java.base/share/classes/java/lang/reflect/InvocationHandler.java
(revision 8352a20bf54a085a97d3f703b5dab482d3f9eccc)
+++ src/java.base/share/classes/java/lang/reflect/InvocationHandler.java
(date 1605869089804)
@@ -271,15 +271,10 @@
throw new IllegalArgumentException(""" + method + "" is not a
default method");
}
Class<?> intf = method.getDeclaringClass();
- // access check if it is a non-public proxy interface or not
unconditionally exported
- if (!Modifier.isPublic(intf.getModifiers()) ||
- !intf.getModule().isExported(intf.getPackageName())) {
- // throw IAE if the caller class has no access to the default
method
- // same access check to Method::invoke on the default method
- int modifiers = method.getModifiers();
- Class<?> caller = Reflection.getCallerClass();
- method.checkAccess(caller, intf, proxyClass, modifiers);
- }
+ // access check
+ Class<?> caller = Reflection.getCallerClass();
+ int modifiers = method.getModifiers();
+ method.checkAccess(caller, intf, proxyClass, modifiers);
MethodHandle mh = Proxy.defaultMethodHandle(proxyClass, method);
// invoke the super method
Index: src/java.base/share/classes/java/lang/reflect/Method.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/java.base/share/classes/java/lang/reflect/Method.java (revision
8352a20bf54a085a97d3f703b5dab482d3f9eccc)
+++ src/java.base/share/classes/java/lang/reflect/Method.java (date
1605870445135)
@@ -31,6 +31,7 @@
import jdk.internal.reflect.Reflection;
import jdk.internal.vm.annotation.ForceInline;
import jdk.internal.vm.annotation.IntrinsicCandidate;
+import jdk.internal.vm.annotation.Stable;
import sun.reflect.annotation.ExceptionProxy;
import sun.reflect.annotation.TypeNotPresentExceptionProxy;
import sun.reflect.generics.repository.MethodRepository;
@@ -66,7 +67,7 @@
* @since 1.1
*/
public final class Method extends Executable {
- private Class<?> clazz;
+ @Stable private Class<?> clazz;
private int slot;
// This is guaranteed to be interned by the VM in the 1.4
// reflection implementation
@@ -74,7 +75,7 @@
private Class<?> returnType;
private Class<?>[] parameterTypes;
private Class<?>[] exceptionTypes;
- private int modifiers;
+ @Stable private int modifiers;
// Generics and annotations support
private transient String signature;
// generic info repository; lazily initialized
Index: src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
(revision 8352a20bf54a085a97d3f703b5dab482d3f9eccc)
+++ src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java (date
1605870027579)
@@ -490,7 +490,7 @@
for (List<ProxyMethod> sigmethods : proxyMethods.values()) {
for (ProxyMethod pm : sigmethods) {
// add static field for the Method object
- visitField(Modifier.PRIVATE | Modifier.STATIC,
pm.methodFieldName,
+ visitField(Modifier.PRIVATE | Modifier.STATIC |
Modifier.FINAL, pm.methodFieldName,
LJLR_METHOD, null, null);
// Generate code for proxy method
...and this makes the following results:
Benchmark Mode Cnt Score Error Units
ProxyBench.implClass avgt 5 3.766 ± 0.040 ns/op
ProxyBench.implProxy avgt 5 26.847 ± 0.626 ns/op
ProxyBench.ppImplClass avgt 5 3.700 ± 0.017 ns/op
ProxyBench.ppImplProxy avgt 5 26.322 ± 0.048 ns/op
But this can be changed as a follow-up patch that also takes care of
Constructor and Field.
-------------
PR: https://git.openjdk.java.net/jdk/pull/313
