On Thu, 11 Mar 2021 04:03:29 GMT, Yumin Qi <[email protected]> wrote:
>> Hi, Please review >> >> Added jcmd option for dumping CDS archive during application runtime. >> Before this change, user has to dump shared archive in two steps: first run >> application with >> `java -XX:DumpLoadedClassList=<classlist> .... ` >> to collect shareable class names and saved in file `<classlist>` , then >> `java -Xshare:dump -XX:SharedClassListFile=<classlist> >> -XX:SharedArchiveFile=<archivefile> ...` >> With this change, user can use jcmd to dump CDS without going through >> above steps. Also user can choose a moment during the app runtime to dump >> an archive. >> The bug is associated with the CSR: >> https://bugs.openjdk.java.net/browse/JDK-8259798 which has been approved. >> New added jcmd option: >> `jcmd <pid or AppName> VM.cds static_dump <filename>` >> or >> `jcmd <pid or AppName> VM.cds dynamic_dump <filename>` >> To dump dynamic archive, requires start app with newly added flag >> `-XX:+RecordDynamicDumpInfo`, with this flag, some information related to >> dynamic dump like loader constraints will be recorded. Note the dumping >> process changed some object memory locations so for dumping dynamic archive, >> can only done once for a running app. For static dump, user can dump >> multiple times against same process. >> The file name is optional, if the file name is not supplied, the file >> name will take format of `java_pid<number>_static.jsa` or >> `java_pid<number>_dynamic.jsa` for static and dynamic respectively. The >> `<number>` is the application process ID. >> >> Tests: tier1,tier2,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Fix filter more flags to exclude in static dump, add more test cases > - Merge branch 'master' into jdk-8259070 > - Fix white space in CDS.java > - Add function CDS.dumpSharedArchive in java to dump shared archive > - 8259070: Add jcmd option to dump CDS Changes requested by iklam (Reviewer). src/hotspot/share/memory/metaspaceShared.cpp line 50: > 48: #include "memory/cppVtables.hpp" > 49: #include "memory/dumpAllocStats.hpp" > 50: #include "memory/dynamicArchive.hpp" This is not needed anymore. src/hotspot/share/prims/jvm.cpp line 3766: > 3764: JVM_ENTRY(void, JVM_DumpDynamicArchive(JNIEnv *env, jstring > archiveName)) > 3765: #if INCLUDE_CDS > 3766: assert(UseSharedSpaces && RecordDynamicDumpInfo, "Sanity check"); I think the message should say why this is true. How about `assert(UseSharedSpaces && RecordDynamicDumpInfo, "already checked in arguments.cpp");`? (same for the next assert statement at line 3774) src/hotspot/share/services/diagnosticCommand.cpp line 1139: > 1137: Handle throwable(THREAD, PENDING_EXCEPTION); > 1138: CLEAR_PENDING_EXCEPTION; > 1139: java_lang_Throwable::print_stack_trace(throwable, output()); Actually, it's not necessary to print the stack trace here. This function is called by `jcmd` in attachListener.cpp which will print the stack trace for you. static jint jcmd(AttachOperation* op, outputStream* out) { Thread* THREAD = Thread::current(); // All the supplied jcmd arguments are stored as a single // string (op->arg(0)). This is parsed by the Dcmd framework. DCmd::parse_and_execute(DCmd_Source_AttachAPI, out, op->arg(0), ' ', THREAD); if (HAS_PENDING_EXCEPTION) { java_lang_Throwable::print(PENDING_EXCEPTION, out); out->cr(); CLEAR_PENDING_EXCEPTION; return JNI_ERR; } return JNI_OK; } ------------- PR: https://git.openjdk.java.net/jdk/pull/2737
