RFR: 8245024: Simplify and eagerly initialize StringConcatFactory

[Claes Redestad]

Hi Mandy,

I haven't looked at JDK-8155659 in years - and lambda bootstrap has been
simplified a lot since. Maybe there's a way to pre-initializate a few
classes to avoid the issues similarly now. It's not been a priority
to fix since it isn't a regression in behavior like JDK-8155090 was -
"just" a behavioral gotcha. Doesn't mean it shouldn't be fixed, if
possible.

Regardless, SCF doesn't use the InnerClassLambdaMetafactory, so the
bootstrapping circularity issue at hand here is unrelated.

If you remove the initialization in System.java and run the test, you'll
get a stacktrace like this:

java.lang.BootstrapMethodError: bootstrap method initialization exception
	at WithSecurityManager$1.checkPermission(WithSecurityManager.java:59)
	at java.base/java.lang.SecurityManager.checkRead(SecurityManager.java:747)
	at java.base/java.io.File.exists(File.java:818)
	at 
java.base/jdk.internal.loader.URLClassPath$FileLoader.getResource(URLClassPath.java:1239)
	at 
java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:317)
	at 
java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:733)
	at 
java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:731)
	at 
java.base/java.security.AccessController.doPrivileged(AccessController.java:312)
	at 
java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:744)
	at 
java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:646)
	at 
java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:604)
	at 
java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:168)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at 
com.sun.javatest.regtest.agent.MainWrapper.handleTestException(MainWrapper.java:94)
	at 
com.sun.javatest.regtest.agent.MainWrapper.access$000(MainWrapper.java:38)
	at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:136)
	at java.base/java.lang.Thread.run(Thread.java:832)

/Claes

On 2020-05-14 22:39, Mandy Chung wrote:
> Hi Claes,
> 
> Is the bootstrapping issue contributed by Class::getDeclaredConstructors 
> call in InnerClassLambdaMetafactory::buildCallSite?
> 
> I wonder how much performance gain compared to using 
> Lookup::findConstructor for non-capturing lambda case.   If LMF uses 
> Lookup::findConstructor, it does not perform any security permission 
> check since it's a full privileged lookup and address JDK-8155659 lambda 
> bootstrapping issue with security manager.   Is it easy to measure the 
> performance difference?
> 
> Mandy
> 
> On 5/14/20 7:39 AM, Claes Redestad wrote:
>> Hi,
>>
>> JDK-8155090 added some measures to StringConcatFactory to prevent
>> certain bootstrapping issues. I think we should consider eagerly
>> initializing the StringConcatFactory base class during bootstrap
>> instead.
>>
>> Doing so will add a tiny bit of class loading overhead to bootstrap
>> (which of course will be amortized on first use of SCF) but still this
>> would also be a good opportunity to clean up a little.
>>
>> One straightforward means to do so is to move the CACHE, DUMPER and
>> CACHE_ENABLE fields to BytecodeStringBuilderStrategy, since it's the
>> only place where they are used (the MethodHandle based strategies don't
>> manually spin classes and rely on LF sharing to de-duplicate shared
>> components).
>>
>> Webrev: http://cr.openjdk.java.net/~redestad/8245024/open.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8245024
>>
>> This reduces initialization overhead and simplifices bootstraps for the
>> default case, while still passing relevant test cases such as test/jdk
>> /java/lang/String/concat/WithSecurityManager.java for all strategies.
>>
>> Testing: tier1-2
>>
>> Thanks!
>>
>> /Claes
>