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

Yumin Qi minqi at openjdk.java.net
Thu Mar 18 03:34:53 UTC 2021


On Fri, 26 Feb 2021 21:39:48 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> 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
>
> src/hotspot/share/memory/dynamicArchive.cpp line 347:
> 
>> 345:   if (Arguments::GetSharedDynamicArchivePath() == NULL) {
>> 346:     if (!RecordDynamicDumpInfo) {
>> 347:       // If run with -XX:+RecordDynamicDumpInfo, DynamicDumpSharedSpaces will be turned on,
> 
> Is this check needed? It looks like `MetaspaceShared::cmd_dump_dynamic` will not call `DynamicArchive::dump()` unless the path was set up correctly.

Fixed. The warning is harmless so I just revert it back.

> src/hotspot/share/memory/metaspaceShared.cpp line 789:
> 
>> 787:   char filename[JVM_MAXPATHLEN];
>> 788:   const char* file = file_name;
>> 789:   assert(strcmp(cmd, "static_dump") == 0 || strcmp(cmd, "dynamic_dump") == 0, "Sanity check");
> 
> Since the caller of this function already performed the string validity check, I think it's better to pass `bool is_static` as a parameter and not pass `cmd`.

Moved to CDS.java, code is simple than this.

> src/hotspot/share/memory/metaspaceShared.cpp line 863:
> 
>> 861:     MutexLocker lock(ClassLoaderDataGraph_lock);
>> 862:     DumpClassListCLDClosure collect_classes(stream);
>> 863:     ClassLoaderDataGraph::loaded_cld_do(&collect_classes);
> 
> Need to close the stream.

Changed to use stack object so it will close the file at destrutor.

> src/hotspot/share/runtime/globals.hpp line 1896:
> 
>> 1894:                                                                             \
>> 1895:   product(bool, RecordDynamicDumpInfo, false,                               \
>> 1896:           "Record class info for jcmd Dynamic dump")                        \
> 
> "Record class info for jcmd VM.cds dynamic_dump"?

Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2737


More information about the hotspot-runtime-dev mailing list