Hi Ioi,
On 28/02/2020 2:09 pm, Ioi Lam wrote:
Hi David,
There classes are usually loaded during verification. They are not
linked because they are referenced only by methods that were never
executed during dynamic dumping. You can see an example here
http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.00/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LinkClassApp.java.html
The "Child" and "Parent" classes are loaded during verification but
never linked.
Note that the LinkClassApp has been verified during dump time, so we can
skip most of the bytecode verification steps during run time. However,
when LinkClassApp is linked at run time, Child/Parent will be resolved
to ensure that Child is still a subclass of Parent (otherwise the
bytecodes in LinkClassApp.test() will be invalid). This is done here:
http://hg.openjdk.java.net/jdk/jdk/file/0edc7fd0d7a3/src/hotspot/share/classfile/systemDictionaryShared.cpp#l1267
Before this fix, Child/Parent will be loaded from classfiles, which
slows down the link time of LinkClassApp.
So in essence we do extra work at dynamic dump time, and increase the
size of the archive, so that we can speedup the subsequent link time of
LinkClassApp.
(I am adding this info to the bug report for clarification).
Thanks for doing that.
David
Thanks
- Ioi
On 2/27/20 5:40 PM, David Holmes wrote:
Hi Calvin, Ioi,
Looking good - comments below.
A meta-question: normal application classes are rarely loaded but not
linked so I'm a little surprised this is an issue. What is the main
source of such classes - generated classes like lambda forms? Also why
do we need to link them when loading from the archive if they were
never linked when the application actually executed ??
On 28/02/2020 10:48 am, 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/
This looks much better to me. I wasn't sure if you were that concerned
about catching the classes loaded by the Shutdown hooks, but it is
good you are not as it seems problematic to me to trigger class
loading etc after the shutdown hooks have executed - you may hit
unexpected conditions. So this approach is good.
A few minor comments:
src/hotspot/share/memory/metaspaceShared.cpp
src/hotspot/share/memory/metaspaceShared.hpp
Why the change from TRAPS to "Thread* THREAD"?
---
src/hotspot/share/oops/instanceKlass.cpp
I'm not clear how verify_on is used so am unsure how the additional
error state check may affect code unrelated to dynamic dumping ??
Also can I suggest (as you are touching this code) to delete the
ancient comment:
3580 // $$$ This used to be done only for m/s collections. Doing it
3581 // always seemed a valid generalization. (DLD -- 6/00)
Thanks,
David
-----
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
