On Wed, 31 Mar 2021 20:58:46 GMT, Yumin Qi <mi...@openjdk.org> 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 pull request now contains 10 commits: > > - Fix revert unintentionally comment, merge master. > - Merge branch 'master' of ssh://github.com/yminqi/jdk into jdk-8259070 > - Remove CDS.getVMArguments, changed to use VM.getRuntimeVMArguments. > Removed unused function from ClassLoader. Improved > InstanceKlass::is_shareable() and related test. Added more test scenarios. > - Remove redundant check for if a class is shareable > - Fix according to review comment and add more tests > - 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/services/diagnosticCommand.cpp line 1106: > 1104: output()->print_cr("Dynamic dump:"); > 1105: if (!UseSharedSpaces) { > 1106: output()->print_cr("CDS is not available for the JDK"); I think we should use a message similar to this: $ java -Xshare:off -XX:ArchiveClassesAtExit=xxx -version Error occurred during initialization of VM DynamicDumpSharedSpaces is unsupported when base CDS archive is not loaded How about "dynamic_dump is unsupported when base CDS archive is not loaded". src/hotspot/share/services/diagnosticCommand.cpp line 1125: > 1123: Symbol* cds_name = vmSymbols::jdk_internal_misc_CDS(); > 1124: Klass* cds_klass = SystemDictionary::resolve_or_fail(cds_name, true > /*throw error*/, CHECK); > 1125: JavaValue result(T_OBJECT); Should be `result(T_VOID)` to match the signature of `CDS.dumpSharedArchive()` src/hotspot/share/services/diagnosticCommand.cpp line 1133: > 1131: vmSymbols::dumpSharedArchive(), > 1132: vmSymbols::dumpSharedArchive_signature(), > 1133: &args, THREAD); Should be `&args, CHECK);`. Recently, we have started to to avoid using `THREAD` if the callee function may throw an exception. src/java.base/share/classes/jdk/internal/misc/CDS.java line 218: > 216: try { > 217: PrintStream prt = new PrintStream(fileName); > 218: prt.println("Command:"); [Try-with-resources](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html) should be used to make sure the streams are closed when the method exits (normally or on exception). try (InputStreamReader isr = new InputStreamReader(stream); BufferedReader rdr = new BufferedReader(isr); PrintStream prt = new PrintStream(fileName);) { prt.println("Command:"); .... src/java.base/share/classes/jdk/internal/misc/CDS.java line 307: > 305: outputStdStream(proc.getErrorStream(), stdErrFile, > cmds); > 306: }).start(); > 307: I think the above code can be refactored to avoid duplication. Something like: String stdOutFile = drainOutput(proc.getInputStream(), "stdout"); String stdErrFile = drainOutput(proc.getErrorStream(), "stderr"); and move the common code into drainOutput(). ------------- PR: https://git.openjdk.java.net/jdk/pull/2737
