RFR: 8296263: Uniform APIs for using archived heap regions

[Ashutosh Mehra]

On Wed, 9 Nov 2022 12:02:17 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> This is an attempt to unify the two different approaches for using archived heap regions. Main goal is to restructure and modify the code to have a single set of GC APIs that can be called for using archived heap regions.
>> 
>> In current state, the VM either tries to "map" (for G1) or "load" (for non-G1 GC policies) the archived heap regions into the java heap.
>> When mapping, the VM determines the address range in the java heap where the archived regions should be mapped. It tries to map the regions towards the end of the heap. The APIs used for this purpose are G1 specific.
>> When loading, the VM asks the GC to provide a chunk of memory from the heap, into which it reads the contents of the archived heap regions. The APIs used are GC policy agnostic but challenging to use for region based collectors.
>> 
>> This PR attempts to add new set of GC APIs that can be used by the VM to reserve space in the heap for mapping the archived heap regions. It combines the good parts of the two existing approaches. Similar to the "loading" API, in this new approach VM is not responsible for determining the mapping address. That responsibility always resides with the GC policy. This also allows the flexibility for the GC implementation to decide where and how to reserve the space for the archived regions. For instance, G1 implementation can continue to attempt to allocate the space towards the end of the heap.
>> This PR also provides the implementation of the new APIs for all the existing GC policies that currently support archived heap regions viz G1, serial, parallel and epsilon.
>
> Some additional initial notes about the code:
> 
> * please document the new APIs in `CollectedHeap` (something like "// Support for mapping archive regions into the heap" for 6 methods with tons of parameters is not sufficient) - it would also help reviewing.
> * also we should be careful with the naming as the term "region" is overloaded enough already; e.g.  `heap_region_dealloc_supported` seems to miss an `archive`.
> * while we are at changing the API, the change should use appropriate types, i.e. unsigned ints/size_t for sizes.
> * there is quite a bit inconsistency in the naming: sometimes the MemRegions are called "something_regions", sometimes "something_ranges", there is also "something_spaces"; the same with the associated counts that are sometimes called "count", and sometimes "num_regions".
> 
> Summarizing all this, I would strongly suggest to use the term "ranges" instead of regions for the areas/MemRegions from the archive. This would help a lot with code clarity in collectors that already use the term regions.
> 
> Similar to @iklam I strongly suggest splitting this change by collector (or at least basic+g1 and epsilon/serial/parallel) anyway. There will likely be considerable changes on top of your current stack of changes which may require working/reviewing on the tip.
> 
> There are additional initial comments in the PR. I understand that some of them may become obsolete as we change the API.
> 
> I did not really look at dumptime considerations which seem to be under discussion right now; I do not have the overview about the exact requirements/problems in that area, particularly with regards to JDK-8296344.
> The chosen API (and the intended execution flow during dump/runtime) is underdocumented at the moment to give good comments. It would be nice to summarize that to start a discussion; maybe even discuss this on some mailing list instead in the PR of a 2k LOC change.
> 
> However, from the existing commetns here, I do think it is a good idea to write out all chosen assumptions when dumping (e.g. the alignment boundary of 1MB), even if currently that is the minimum for all collectors.

@tschatzl thanks for reviewing the PR. Given that JDK-8296344 is likely to affect this PR and there are still details to be figured out and finalized for the dumping, I guess its best to put this on hold for now. Once we have the details sorted out, I can resurrect this patch if required.

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

PR: https://git.openjdk.org/jdk/pull/10970