RFR: 8250989: Consolidate buffer allocation code for CDS static/dynamic dumping

[Ioi Lam]

On Mon, 1 Feb 2021 17:42:41 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Before this PR, when a static CDS archive is dumped, we would unconditionally allocate a 4GB ReservedSpace and use that as the buffer for writing the archive content. We usually don't need such a big ReservedSpace. this also unnecessarily complicates Metaspace::global_initialize().
>> 
>> After this PR, both we first load all the classes and then allocate a buffer with an appropriate size.
>> 
>> I also simplified the pointer relocation code that prepares the archive to be mapped at the "requested location" (usually 0x800000000), and improved the comments.
>> 
>> We used to have lots of special cases for adjusting pointers during dynamic dump. All of those are removed.
>> 
>> Reviewers, please start with
>> - Header comments in archiveBuilder.hpp that describe the overall process of archive dumping
>> - `MetaspaceShared::initialize_for_static_dump()`
>> - `ArchiveBuilder::reserve_buffer()`
>> - Comments around `RelocateBufferToRequested` in archiveBuilder.cpp
>
> src/hotspot/share/memory/archiveBuilder.cpp line 732:
> 
>> 730:   }
>> 731:   return buffer_to_offset(p);
>> 732: }
> 
> Is it possible to add an `assert(DynamicDumpSharedSpaces)` after the `if` statement at line 728?

Added per suggestion.

> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/DynamicArchiveRelocationTest.java line 96:
> 
>> 94:         // (1) Dump base archive (static)
>> 95: 
>> 96:         OutputAnalyzer out = TestCommon.dumpBaseArchive(baseArchiveName, unlockArg, logArg);
> 
> Should the check for `out.shouldContain("Relocating archive from")` be kept?
> Otherwise, no need to save the `OutputAnalyzer` returned from `TestCommon.dumpBaseArchive`.

I added the `out.shouldContain("Relocating archive from")` back (so that the changes are the same as in ArchiveRelocationTest.java)

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

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