hg: jdk8/tl/jdk: 7194897: JSR 292: Cannot create more than 16 instances of an anonymous class; ...

[Joel Borggrén-Franck]

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