RFR: 8310428: Add CollectedHeap::reserved_range() API to return reserved heap memory range

[Thomas Stuefe]

On Wed, 21 Jun 2023 18:54:45 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Please review this patch to add a new API `CollectedHeap::reserved_range()`. The changes are extracted out from https://github.com/openjdk/jdk/pull/14520 as per the suggestion [here](https://github.com/openjdk/jdk/pull/14520#discussion_r1234991165). Support for ZGC is not implemented yet as it can have discontiguous heap regions and unlike other collectors, it does not set `CollectedHeap::_reserved`.
>
> This change completely undoes the removal of CollectedHeap::reserved_region() we did in:
> 8224815: Remove non-GC uses of CollectedHeap::is_in_reserved()
> 
> We removed it because it is a false abstraction that makes it easy for people to accidentally start to use this API without realizing that it will not work for ZGC.
> 
> I'd prefer to see a plan on how to implement heap archiving for ZGC before we decide on making abstractions in CollectedHeap that doesn't work for ZGC. I know that @fisk has had earlier discussions with  @iklam about other ways to make the heap archiving GC-agnostic. In #14520 we saw that another approach was taken and @fisk stepped in and voiced again that we have concerns about this approach. Given that we haven't completed that discussion I'd like to pause this PR until we have come to an agreement on how to proceed.

Hi @stefank and @fisk,

I still opt for an abstraction like this, coupled with an added "heap_is_contiguous()". Its just a fact that for the majority of collectors this abstraction holds.

Let me give you a specific example. 

I am in the process of reworking metaspace and nKlass encoding for Lilliput. One of the things I always disliked a lot was the mechanism by which we "allocate class space adjacent to heap, but only if UseCompressedOops and heap is allocated in the lower n GB range". So, while I'm on it, I'll change that.

What we really wanted here was zero-based nKlass mode, which has nothing to do whatsoever with the heap. But if someone went to the trouble of allocating the heap in lower address ranges, the logic goes, we just piggyback on that and allocate class space adjacent to it.

That approach is flawed for a few reasons I won't detail here. But it illuminates one problem: any improved process I am thinking about (for example, scanning `/proc/self/maps` or `VirtualQuery()` for address space holes or the typical sequential-mapping-attempts approach) in practice benefits from knowing where the heap is located. Simply because it happens to be the largest known contiguous reserved address range at that point in time. Since scanning procfs or mmaping are expensive operations, it pays to avoid this address range since we know mapping attempt will fail.

In this example, the current coding mis-uses CompressedOops::encoding_range() as a (flawed) proxy for heap range. So, in practice, we already use the concept of heap range.

ZGC is a great collector, but a bit like our exotic platforms; there is a tension between keeping code super-portable and getting the job done on other platforms. With platforms, we were always good at finding compromises. For me, a "reserved_region()" coupled with "is_contiguous()" would be such a reasonable workaround.

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

PR Comment: https://git.openjdk.org/jdk/pull/14595#issuecomment-1601606326