On 5 nov 2013, at 10:51, Peter Levart <[email protected]> wrote:
> On 11/05/2013 10:33 AM, Joel Borggrén-Franck wrote:
>>> >I would also restructure the Method/Constructor accessor logic
>>> >differently. The check for ReflectUtil.isVMAnonymousClass() can be
>>> >performed just once (in the newMethodAccessor/newConstructorAccessor
>>> >methods) and based on this check, create accessor:
>>> >
>>> >- for classic declaring class - as is / unchanged
>>> >- for anonymous declaring class - just create and return
>>> >NativeMethodAccessorImpl without a parent
>>> >
>>> >Then in NativeMethodAccessorImpl (and same for constructor), modify the
>>> >inflation checking logic:
>>> >
>>> > if (parent != null && ++numInvocations >
>>> > ReflectionFactory.inflationThreshold()) {
>>> > MethodAccessorImpl acc = (MethodAccessorImpl)
>>> > new MethodAccessorGenerator().
>>> > generateMethod(method.getDeclaringClass(),
>>> > method.getName(),
>>> > method.getParameterTypes(),
>>> > method.getReturnType(),
>>> > method.getExceptionTypes(),
>>> > method.getModifiers());
>>> > parent.setDelegate(acc);
>>> > }
>> I don't like adding even more special cases to this check. IMHO a better way
>> that we discussed and rejected, opting for a smaller change, is to create a
>> NonInflatingMethodAccessor and just drop in one of those without a delegate
>> for when creating the accessor for methods/ctors on anonymous classes.
>
> Even better. I would name the new class NativeMethodAccessorImpl and the one
> doing inflation InflatingNativeMethodAccessorImpl which would extend
> NativeMethodAccessorImpl, override invoke() and call super.invoke()... This
> way, no native methods duplication is needed. invoke() is already an
> interface method with 2 implementations. Now it will have 3. Does this
> present any difference for dispatch optimization?
>
FWIW i think the magic number is 4, but I don't know where I got that. Anyhow,
this might be slightly controversial, but for all code I care about (reflective
invocation of methods/ctors on non-VM-anonymous classes) the check happens
exactly once as is.
I think this should be good enough (but we seem to have other issues with
noInflation=true) because the test crashes:
diff -r 51b002381b35 src/share/classes/sun/reflect/ReflectionFactory.java
--- a/src/share/classes/sun/reflect/ReflectionFactory.java Mon Nov 04
10:12:18 2013 -0800
+++ b/src/share/classes/sun/reflect/ReflectionFactory.java Tue Nov 05
11:07:53 2013 +0100
@@ -33,6 +33,7 @@
import java.security.AccessController;
import java.security.Permission;
import java.security.PrivilegedAction;
+import sun.reflect.misc.ReflectUtil;
/** <P> The master factory for all reflective objects, both those in
java.lang.reflect (Fields, Methods, Constructors) as well as their
@@ -144,7 +145,7 @@
public MethodAccessor newMethodAccessor(Method method) {
checkInitted();
- if (noInflation) {
+ if (noInflation &&
!ReflectUtil.isVMAnonymousClass(method.getDeclaringClass())) {
return new MethodAccessorGenerator().
generateMethod(method.getDeclaringClass(),
method.getName(),
@@ -181,7 +182,7 @@
return new BootstrapConstructorAccessorImpl(c);
}
- if (noInflation) {
+ if (noInflation &&
!ReflectUtil.isVMAnonymousClass(c.getDeclaringClass())) {
return new MethodAccessorGenerator().
generateConstructor(c.getDeclaringClass(),
c.getParameterTypes(),
diff -r 51b002381b35
test/java/lang/invoke/lambda/RepetitiveLambdaSerialization.java
--- a/test/java/lang/invoke/lambda/RepetitiveLambdaSerialization.java Mon Nov
04 10:12:18 2013 -0800
+++ b/test/java/lang/invoke/lambda/RepetitiveLambdaSerialization.java Tue Nov
05 11:07:53 2013 +0100
@@ -27,6 +27,7 @@
* @summary Lambda serialization fails once reflection proxy generation kicks
in
* @author Robert Field
* @run main/othervm RepetitiveLambdaSerialization
+ * @run main/othervm -Dsun.reflect.noInflation=true
RepetitiveLambdaSerialization
*/
import java.io.*;
diff -r 51b002381b35
test/sun/reflect/AnonymousNewInstance/ManyNewInstanceAnonTest.java
--- a/test/sun/reflect/AnonymousNewInstance/ManyNewInstanceAnonTest.java
Mon Nov 04 10:12:18 2013 -0800
+++ b/test/sun/reflect/AnonymousNewInstance/ManyNewInstanceAnonTest.java
Tue Nov 05 11:07:53 2013 +0100
@@ -30,6 +30,7 @@
* @compile -XDignore.symbol.file ManyNewInstanceAnonTest.java
* @run main ClassFileInstaller ManyNewInstanceAnonTest
* @run main/othervm -Xbootclasspath/a:. -Xverify:all ManyNewInstanceAnonTest
+ * @run main/othervm -Xbootclasspath/a:. -Xverify:all
-Dsun.reflection.noInflation=true ManyNewInstanceAnonTest
*/
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
cheers
/Joel
