RFR: 8259070: Add jcmd option to dump CDS [v3]

[Ioi Lam]

On Tue, 9 Mar 2021 21:50:25 GMT, Yumin Qi <minqi at 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