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

Ioi Lam iklam at openjdk.org
Fri Jun 2 20:13:15 UTC 2023


On Fri, 2 Jun 2023 19:46:31 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:
> 
>   Review comments - updates to alloc_archive_regions() api
>   
>   Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

src/hotspot/share/gc/g1/g1CollectedHeap.hpp line 712:

> 710:   // the location of the archive space in the heap. The returned address may or may
> 711:   // not be same as the preferred address.
> 712:   HeapWord* alloc_archive_region(size_t word_size, HeapWord* preferred_addr);

Sorry to be picky, but I think `region` implies a single region, but G1 could allocate one or more regions to satisfy the request. I think it's better to use `allocate_archive_range(MemRegion requested_range)` to be more neutral. Passing the range in a `MemRegion` will also look similar to the API right above this one.

Maybe we should also change `populate_archive_regions_bot_part` and `dealloc_archive_regions` to use `_range` as well. What do you think, @tschatzl

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

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


More information about the hotspot-dev mailing list