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

Ioi Lam iklam at openjdk.org
Sat Jun 17 05:11:28 UTC 2023


On Fri, 16 Jun 2023 14:26:09 GMT, Ashutosh Mehra <duke 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/cds/filemap.cpp line 2142:

> 2140:                               r->allow_exec());
> 2141:   if (base == nullptr || base != addr) {
> 2142:     handle_failed_mapping();

`handle_failed_mapping()` is called for several reasons where we are not able to use the archive heap:
- mmap failed for the `hp` or `bm` regions
- CRC check failure

So I think the wording of "mapping" is too specific and doesn't cover all cases (and also precludes the possibility of other failures in the future.

Also "handle" could imply a possibility of recovering from the failure, which isn't the case.

I think a more neutral name would be better. Maybe `release_archive_space()` in contrast of `alloc_archive_space` (or even `allocate_archive_space`, as we prefer proper words than short hands).

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

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


More information about the hotspot-gc-dev mailing list