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

[Ashutosh Mehra]

On Tue, 6 Jun 2023 09:41:03 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> If you plan to change these soon I think it's OK to leave the names as you have for this PR.
>
>> 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
> 
> I remember suggesting a rename long time ago in some earlier PR about refactoring this code..
> 
>> How about replacing `allocate_archive_range` with `allocate_archive_space` which is actually planned for the next patch?
> 
> As long as the naming is consistent throughout. 
> 
>> 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.
> 
> I think separating the arguments for the above mentioned reasons is fine.

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

Okay, then lets do the renaming in the next PR.

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

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