RFR(S): 8232081: Try to link all classes during dynamic CDS dump

[Ioi Lam]

Hi Calvin,

This looks good to me.

I would suggest adding the something like this to the test case:

class Child extends Parent {
     int get() {return 2;}
     int dummy() {
         // When Child is linked during dynamic dump (when the VM is 
shutting
         // down), it will be verified, which will recursively cause 
Child2/Parent2
         // to be loaded and verified.
         // We want to make sure that this is done at a point in the VM 
lifecycle where
         // it's still safe to do so.
         Parent2 x = new Child2();
         return x.get();
     }
}

Thanks
- Ioi

On 2/27/20 4:48 PM, Calvin Cheung wrote:
> Hi David, Ioi,
>
> Thanks for your review and suggestions.
>
> On 2/26/20 9:46 PM, Ioi Lam wrote:
>>
>>
>> On 2/26/20 7:50 PM, David Holmes wrote:
>>> Hi Calvin,
>>>
>>> Adding core-libs-dev as you are messing with their code :)
>>>
>>> On 27/02/2020 1:19 pm, Calvin Cheung wrote:
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8232081
>>>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.00/
>>>>
>>>> The proposed changeset for this RFE adds a JVM_LinkClassesForCDS() 
>>>> function to be called from java/lang/Shutdown to notify the JVM to 
>>>> link the classes loaded by the builtin class loaders. The 
>>>
>>> This would be much less disruptive if this was handled purely on the 
>>> VM side once we have started shutdown. No need to make any changes 
>>> to the Java side then, nor jvm.cpp.
>>>
>>
>> Hi David,
>>
>> To link the classes, we need to be able to run Java code -- when 
>> linking a class X loaded by the app loader, X will be verified. 
>> During verification of X, we may need to load additional classes from 
>> the app loader, which executes Java code during its class loading 
>> operations.
>>
>> We also need to handle all the exit conditions. As far as I can tell, 
>> an app can exit the JVM in two ways:
>>
>> (1) Explicitly calling System.exit(). This will call 
>> java.lang.Shutdown.exit() which does this:
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/0edc7fd0d7a3/src/java.base/share/classes/java/lang/Shutdown.java#l163 
>>
>>
>>             beforeHalt(); // native
>>             runHooks();
>>             halt(status);
>>
>> (2) When all non-daemon threads have died (e.g., falling out of the 
>> bottom of HelloWorld.main()). There's no explicit call to 
>> System.exit(), but the JVM will proactively call 
>> java.lang.Shutdown.shutdown() inside JavaThread::invoke_shutdown_hooks()
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/0edc7fd0d7a3/src/hotspot/share/runtime/thread.cpp#l4331 
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/0edc7fd0d7a3/src/java.base/share/classes/java/lang/Shutdown.java#l184 
>>
>>
>> If we want to avoid modifying the Java code, I think we can intercept 
>> JVM_BeforeHalt() and JavaThread::invoke_shutdown_hooks(). This way we 
>> should be able to handle all the classes (except those that are 
>> loaded inside Shutdown.runHooks).
>
> I've implemented the above. No changes to the Java side.
>
> updated webrev:
>
>     http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.01/
>
> I also updated the test to test the shutdown hook and System.exit() 
> scenarios.
>
> All appcds tests passed on my linux host. I'll run more tests using 
> mach5.
>
> thanks,
>
> Calvin
>
>
>>
>> What do you think?
>>
>> - Ioi
>>
>>> Thanks,
>>> David
>>>
>>>> MetaspaceShared::link_and_cleanup_shared_classes() has been 
>>>> modified to handle both static and dynamic CDS dump. For dynamic 
>>>> CDS dump, only classes loaded by the builtin class loaders will be 
>>>> linked. Local performance testing using javac on HelloWorld.java 
>>>> shows an improvement of >5%.
>>>>
>>>> Passed tier1 - 4 tests.
>>>>
>>>> thanks,
>>>>
>>>> Calvin
>>>>
>>