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

[Thomas Stuefe]

On Thu, 28 Jan 2021 18:14:14 GMT, Ioi Lam <iklam 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

Hi Ioi,

Just some initial remarks. Had a cursory glance at this patch, but this is a very large change :) seems to be a good cleanup though.

I am trying to understand how allocation works now. Just to make sure I get the gist right, where before we had three cases (with UseCompressedClassPoitners):
1) DumpSharedSpaces = cds+ccs in a 4G range, realized by just reserving 4g
2) UseSharedSpaces = cds+ccs in a 4G range, realized with our complicated mapping dance
3) no sharing, where class space is allocated like in old times

now you eliminated (1)? So the restriction that ccs has to be within narrow klass ptr range of cds does not apply anymore at dump time? And we can map ccs wherever?

You know, at some point a flowchart would be helpful.

src/hotspot/share/memory/metaspace.cpp line 681:

> 679: 
> 680:   // If UseCompressedClassPointers=1, we have two cases:
> 681:   // a) if CDS archive has been mapped, it will create the ccs

This comment is confusing. So, at this point the ccs archive has already be mapped? Where?

src/hotspot/share/memory/metaspaceShared.cpp line 283:

> 281:       _extra_symbols->append(SymbolTable::new_permanent_symbol(utf8_buffer));
> 282:     } else{
> 283:       assert(prefix_type == HashtableTextDump::StringPrefix, "Sanity");

So this whole comment is wrong now and obsolete?

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

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