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

[Ioi Lam]

On Fri, 2 Jun 2023 20:41:53 GMT, Ashutosh Mehra <duke at openjdk.org> wrote:

>> 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
>
> How about replacing `allocate_archive_range` with  `allocate_archive_space` which is actually planned for the next patch?
> I am more inclined to keep size and address as separate parameters because the address is just a hint for the collectors, while size is not. With MemRegion as the type this information is not conveyed to the reader without reading the comments/code. This is also the reason why I prefer using "preferred_addr" rather than "requested_addr".
> But if you feel otherwise I will update the API to use MemRegion.
> 
>> Maybe we should also change populate_archive_regions_bot_part and dealloc_archive_regions to use _range as well
> 
> I am not sure if it is necessary to update these APIs, as I would anyway be replacing them with fixup_archive_space() and handle_archive_space_failure() as mentioned in the description of main issue [JDK-8296263](https://bugs.openjdk.org/browse/JDK-8296263). IMO these names are more generic. If you want I can update the code to use these new names in this patch.

If you plan to change these soon I think it's OK to leave the names as you have for this PR.

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

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