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

Ashutosh Mehra duke at openjdk.org
Mon Jun 19 16:02:09 UTC 2023


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

>> 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).
>
>> 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).
> 
> "on_load_error"?

To be honest I don't like these names either and am open for suggestions. I liked @tstuefe's [suggestion](https://github.com/openjdk/jdk/pull/14520/files#r1233100361) for `on_cds_post_load` and `on_cds_load_error` but probably name them `on_archive_heap_post_load` and `on_archive_heap_load_error`.
Also in CDS code "load" is currently associated with the mechanism used by non-G1 collecters to read the archive space into the heap memory and "map" is associated with the G1 collector's mechanism to mmap the archive space. For instance FileMapInfo::map_or_load_heap_region.
Going forward I think we would need to get rid of this distinction, at least in public APIs and just use the term "load". Internally the "loading" may be achieved through IO read or mmap-ing the archive space.
@iklam what do you think?

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

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


More information about the hotspot-dev mailing list