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

[Calvin Cheung]

Hi Ioi,

On 2/27/20 8:22 PM, Ioi Lam wrote:
> Hi Calvin,
>
> This looks good to me.
Thanks for taking another look.
>
> 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();
>     }
> }

The following updated webrev includes the above changes in the test case:

     http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.02/

thanks,

Calvin

>
> 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
>>>>>
>>>
>