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