RFR: 8245024: Simplify and eagerly initialize StringConcatFactory

[Mandy Chung]

Ah, it's unrelated to JDK-8155659.

The change looks okay.  Minor: it might worth adding the getClassName 
method in BSBS class.

I also agree with Paul that we should examine if it's time to remove the 
non-default strategies if they are mostly unused.

Mandy

On 5/14/20 2:27 PM, Claes Redestad wrote:
> 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
>>