On Tue, 9 Mar 2021 21:50:25 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 incrementally with one additional
> commit since the last revision:
>
> Fix white space in CDS.java
Changes requested by iklam (Reviewer).
src/hotspot/share/services/diagnosticCommand.cpp line 1124:
> 1122: }
> 1123: Symbol* cds_name = vmSymbols::jdk_internal_misc_CDS();
> 1124: Klass* cds_klass = SystemDictionary::resolve_or_null(cds_name,
> THREAD);
Should be `cds_klass = SystemDictionary::resolve_or_fail(cds_name, CHECK);`
src/java.base/share/classes/jdk/internal/misc/CDS.java line 278:
> 276: dumpDynamicArchive(archiveFile);
> 277: }
> 278: }
I think we should have some error checks and clean up:
- Remove the classlist file
- Check if if the process exit status is 0
- Remove the JSA file first, then try to dump it, and check if the file exists
afterwards. If not, report the error. (For both dynamic and static dumps)
src/java.base/share/classes/jdk/internal/misc/CDS.java line 256:
> 254:
> 255: // Do not take parent env which will cause dumping fail.
> 256: Process proc = Runtime.getRuntime().exec(cmds.toArray(new
> String[0]),
Could you explain why the parent's env variables will cause dumping to fail?
src/java.base/share/classes/jdk/internal/misc/CDS.java line 213:
> 211: testStr.contains("-XX:+DynamicDumpSharedSpaces") ||
> 212: testStr.contains("-XX:+RecordDynamicDumpInfo");
> 213: }
The following flags should also be excluded:
- -XX:-DumpSharedSpaces
- -Xshare:
- -XX:SharedClassListFile=
- -XX:SharedArchiveFile=
- -XX:ArchiveClassesAtExit=
- -XX:+UseSharedSpaces
- -XX:+RequireSharedSpaces
We also need to have a few test cases when the LingeredApp is started with
these flags.
src/java.base/share/classes/jdk/internal/misc/CDS.java line 262:
> 260: String line;
> 261: InputStreamReader isr = new
> InputStreamReader(proc.getInputStream());
> 262: BufferedReader rdr = new BufferedReader(isr);
Also, I think the output should always be logged. Otherwise if an error
happens, it's very difficult for the user to diagnose (and they won't know
about the "CDS.Debug" property).
-------------
PR: https://git.openjdk.java.net/jdk/pull/2737
