RFR: 8309065: Move the logic to determine archive heap location from CDS to G1 GC [v2]

[Thomas Schatzl]

On Wed, 31 May 2023 14:49:23 GMT, Ashutosh Mehra <duke at openjdk.org> wrote:

>> This patch is the first step towards having a single set of GC APIs for allocating heap space for the archived objects (See https://bugs.openjdk.org/browse/JDK-8296263).
>> It moves some of the G1 specific logic from CDS to G1 gc without changing the functionality.
>> 
>> Changes that add/update GC APIs for handling archive heap would be introduced in upcoming patches.
>
> Ashutosh Mehra has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove unused method FileMapInfo::heap_region_mapped_address
>   
>   Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

src/hotspot/share/cds/filemap.cpp line 2129:

> 2127: 
> 2128:   // allocate from java heap
> 2129:   HeapWord* start = G1CollectedHeap::heap()->alloc_archive_regions(word_size);

I'm not convinced not giving a preferred location is a good idea. That seems to reduce the opportunity to directly map archives significantly. Previously, with only heap size changes, the archive could be mapped in still.

How that reduces the API to the GC seems unclear, this call is still embedded in G1 specific code.

Since this change is an intermediate step, could you provide an overview of the final API/change too? It is hard to comment on this without knowing where you are going with that.

Without knowing where this ends up, I would prefer if this method passed in the full suggested memory range (or like memory reservation works, pass in a `requested_addr`/`requested_start` that is null in case it's not used if you prefer) instead of just the size.
The method may still return the actual location. Other collectors may simply ignore that hint (which is better than the caller not even bothering to give the hint).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14208#discussion_r1212820720