RFR: 8310160: Make GC APIs for handling archive heap objects agnostic of GC policy

[Ashutosh Mehra]

On Sat, 17 Jun 2023 17:39:10 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This PR adds GC APIs to be implemented by all collectors for proper handling of archive heap space. Currently only G1 is updated to use these APIs which just involves renaming the existing G1 APIs.
>> In addition to that filemap.cpp is updated to replace calls to `G1CollectedHeap::heap()` with `Universe::heap()` to avoid G1 specific code as much as possible.
>> 
>> At many places in filemap.cpp heap range is requested from GC. All collectors except ZGC have contiguous heap and set `CollectedHeap::_reserved` to the heap range, so it can be easily exposed to the CDS code. This is done in this patch through `CollectedHeap::reserved` API. But for ZGC the heap can be discontiguous which makes it tricky to expose the heap range.
>> Another point to note is that most of the usage for heap range is for logging purpose, but there is one place where it is used for setting the `mapping_offset` in `FileMapInfo::write_region()` based on the heap start. So purely based on the functional requirement, we only need the heap start address, not the range.
>> 
>> To keep things simple and considering ZGC does not currently support archive heap, i refrained from tackling the issue of discontiguous heap range in this PR.
>
> src/hotspot/share/gc/shared/collectedHeap.hpp line 536:
> 
>> 534: 
>> 535:   // Handle any failure in loading the CDS archive area.
>> 536:   virtual void handle_archive_space_failure(MemRegion range) { return; }
> 
> For both `fixup_archive_space` and `handle_archive_space_failure`: Why do we need to pass in the MemRegion? The only valid argument in both cases is the MemRegion that had been previously established by a call to `alloc_archive_space`. 
> 
> Why not let CollectedHeap remember the MemRegion established by `alloc_archive_space`, thereby also guaranteeing that `alloc_archive_space` can only be called once, and refer to that in the followup calls?

I don't see any reason why we can't do that. Only reason could be that we need the MemRegion in FileMapInfo as well, so we end up duplicating the MemRegion in CollectedHeap and FileMapInfo. 
But looking at the uses of `_mapped_heap_memregion` in FileMapInfo.cpp I think cds only needs the start address of the archive space in the heap. Size of the region must be same as the used bytes of `MetaspaceShared::hp` region. So we can ideally replace `_mapped_heap_memregion` with something like`_archive_space_load_address`.
@iklam @tschatzl what is your opinion?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14520#discussion_r1234255520