RFR: 8261608: Move common CDS archive building code to archiveBuilder.cpp [v2]

[Calvin Cheung]

On Fri, 12 Feb 2021 04:08:03 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This is a follow-up to https://git.openjdk.java.net/jdk/pull/2296:
>> 
>> - Move common code for writing the CDS archive from metaspaceShared.cpp to archiveBuilder.cpp
>> 
>> - Data structures related to dumping were haphazardly organized in several classes (e.g., `DumpRegions`). We needed various APIs to access them across classes. These should be consolidated in archiveBuilder.cpp and the API should be cleaned up
>> 
>> - Detailed stats (`DumpAllocStats::print_stats`) were available only for static dump. Refactor the code so they are also printed for dynamic dump
>
> Ioi Lam has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fixed spaces
>  - use member initializer list; clean up log message

Looks good overall. Couple of minor comments.

src/hotspot/share/memory/archiveBuilder.cpp line 197:

> 195: 
> 196:   assert(_current == NULL, "must be");
> 197:   _current = this;

These lines used to be at the beginning of the function. Any reasons why they are moved?

test/hotspot/jtreg/runtime/cds/appcds/LotsOfClasses.java line 53:

> 51:         opts.addSuffix("-Xlog:gc+region+cds");
> 52:         //opts.addSuffix("-Xlog:gc+region=trace");
> 53:         opts.addSuffix("-Xlog:cds=debug");  // test detailed metadata info printing

Remove the commented line #52?

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

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