[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump
From:       Ioi Lam <ioi.lam () oracle ! com>
Date:       2020-02-28 7:47:32
Message-ID: 3e641f36-71ce-86be-62be-e0bb4ccb593a () oracle ! com
[Download RAW message or body]



On 2/27/20 10:14 PM, Calvin Cheung wrote:
> 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/
> 
Looks good. Thanks!

- Ioi

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


[prev in list] [next in list] [prev in thread] [next in thread]