On 5 nov 2013, at 10:51, Peter Levart <peter.lev...@gmail.com> 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