RFR(M): 8243392: Remodel CDS/Metaspace storage reservation

[Thomas Stüfe]

Hi all,

Could I have reviews for the following proposal of reworking cds/class
space reservation?

Bug: https://bugs.openjdk.java.net/browse/JDK-8243392

Webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/rework-cds-ccs-reservation/webrev.00/webrev/

(Many thanks to Ioi Lam for so patiently explaining CDS internals to me,
and to Andrew Haley and Nick Gasson for help with aarch64!)

Reservation of the compressed class space is needlessly complicated and has
some minor issues. It can be simplified and made clearer.

The complexity stems from the fact that this area lives at the intersection
of two to three sub systems, depending on how one counts. Metaspace, CDS,
and the platform which may or may not its own view of how to reserve class
space. And this code has been growing organically over time.

One small example:

ReservedSpace Metaspace::reserve_preferred_space(size_t size, size_t
alignment,
                                                 bool large_pages, char
*requested_addr,
                                                 bool use_requested_addr)

which I spent hours decoding, resulting in a very confused mail to
hs-runtime and aarch64-port-dev [2].

This patch attempts to simplify cds and metaspace setup a bit; to comment
implicit knowledge which is not immediately clear; to cleanly abstract
platform concerns like optimized class space placement; and to disentangle
cds from metaspace to solve issues which may bite us later with Elastic
Metaspace [4].

---

The main change is the reworked reservation mechanism. This is based on
Ioi's proposal [5].

When reserving class space, three things must happen:

1) reservation of the space obviously. If cds is active that space must be
in the vicinity of cds archives to be covered by compressed class pointer
encoding.
2) setting up the internal Metaspace structures atop of that space
3) setting up compressed class pointer encoding.

In its current form, Metaspace may or may not do some or all of that in one
function (Metaspace::allocate_metaspace_compressed_klass_ptrs(ReservedSpace
metaspace_rs, char* requested_addr, address cds_base);) - if cds is active,
it will reserve the space for Metaspace and hand it in, otherwise it will
create it itself.

When discussing this in [2], Ioi proposed to move the reservation of the
class space completely out of Metaspace and make it a responsibility of the
caller always. This would reduce some complexity, and this patch follows
the proposal.

I removed Metaspace::allocate_metaspace_compressed_klass_ptrs(ReservedSpace
metaspace_rs, char* requested_addr, address cds_base); and all its sub
functions.

(1) now has to always be done outside - a ReservedSpace for class space has
to be provided by the caller. However, Metaspace now offers a utility
function for reserving space at a "nice" location, and explicitly doing
nothing else:

ReservedSpace
Metaspace::reserve_address_space_for_compressed_classes(size_t size);

this function can be redefined on a platform level for platform optimized
reservation, see below for details.

(2) is taken care of by a new function,
Metaspace::initialize_class_space(ReservedSpace rs)

(3) is taken care of a new function CompressedKlassPointers::initialize(),
see below for details.


So, class space now is set up three explicit steps:

- First, reserve a suitable space by however means you want. For
convenience you may use
Metaspace::reserve_address_space_for_compressed_classes(), or you may roll
your own reservation.
- Next, tell Metaspace to use that range as backing storage for class
space: Metaspace::initialize_class_space(ReservedSpace rs)
- Finally, set up encoding. Encoding is independent from the concept of a
ReservedSpace, it just gets an address range, see below for details.

Separating these steps and moving them out of the responsibility of
Metaspace makes this whole thing more flexible; it also removes unnecessary
knowledge (e.g. Metaspace does not need to know anything about either ccp
encoding or cds).

---

How it comes together:

If CDS is off, we just reserve a space using
Metaspace::reserve_address_space_for_compressed_classes(), initialize it
with Metaspace::initialize_class_space(ReservedSpace rs), then set up
compressed class pointer encoding covering the range of this class space.

If CDS is on (dump time), we reserve large 4G space, either at
SharedBaseAddress or using
Metaspace::reserve_address_space_for_compressed_classes(); we then split
that into 3G archive space and 1G class space; we set up that space with
Metaspace as class space; then we set up compressed class pointer encoding
covering both archive space and cds.

If CDS is on (run time), we reserve a large space, split it into archive
space (large enough to hold both archives) and class space, then basically
proceed as above.

Note that this is almost exactly how things worked before (modulo some
minor fixes, e.g. alignment issues), only the code is reformed and made
more explicit.

---

I moved compressed class pointer setup over to CompressedKlassPointers and
changed the interface:

-void Metaspace::set_narrow_klass_base_and_shift(ReservedSpace
metaspace_rs, address cds_base)
+void CompressedKlassPointers::initialize(address addr, size_t len);

Instead of feeding it a single ReservedSpace, which is supposed to
represent class space, and an optional alternate base if cds is on, now we
give it just an numeric address range. That range marks the limits to where
Klass structures are to be expected, and is the implicit promise that
outside that range no Klass structures will exist, so encoding has to cover
only this range.

This range may contain just the class space; or class space+cds; or
whatever allocation scheme we come up with in the future. Encoding does not
really care how the memory is organized as long as the input range covers
all possible Klass locations. That way we remove knowledge about class
space/cds from compressed class pointer encoding.

Moving it away from metaspace.cpp into the CompressedKlassPointers class
also mirrors CompressedOops::initialize().

---

I renamed _narrow_klass_range to just _range, because strictly speaking
this is the range un-narrow Klass pointers can have.

As for the implementation of CompressedKlassPointers::initialize(address
addr, size_t len), I mimicked very closely what happened before, so there
should be almost no differences. Since "almost no differences" sounds scary
:) here are the differences:

- When CDS is active (dump or run time) we now always, unconditionally, set
the encoding range to 4G. This fixes a theoretical bug discussed on
aarch64-port-dev [1].

- When CDS is not active, we set the encoding range to the minimum required
length. Before, it was left at its default value of 4G.

Both differences only affect aarch64, since they are currently the only one
using the range field in CompressedKlassPointers.

I wanted to add an assert somewhere to test encoding of the very last
address of the CompressedKlassPointers range, again to prevent errors like
[3]. But I did not come up with a good place for this assert which would
cover also the encoding done by C1/C2.

For the same reason I thought about introducing a mode where Klass
structures would be allocated in reverse order, starting at the end of the
ccs, but again left it out as too big a change.

---

OS abstraction: platforms may have restrictions of what constitutes a valid
compressed class pointer encoding base. Or if not, they may have at least
preferences. There was logic like this in metaspace.cpp, which I removed
and cleanly factored out into platform dependent files, giving each
platform the option to add special logic.

These are two new methods:

- bool CompressedKlassPointers::is_valid_base(address p)

to let the platform tell you whether it considers p to be a valid encoding
base. The only platform having these restrictions currently is aarch64.

- ReservedSpace
Metaspace::reserve_address_space_for_compressed_classes(size_t size);

this hands over the process of allocating a range suitable for compressed
class pointer encoding to the platform. Most platforms will allocate just
anywhere, but some platforms may have a better strategy (e.g. trying low
memory first, trying only correctly aligned addresses and so on).

Beforehand, this coding existed in a similar form in metaspace.cpp for
aarch64 and AIX. For now, I left the AIX part out - it seems only half
done, and I want to check further if we even need it, if yes why not on
Linux ppc, and C1 does not seem to support anything other than base+offset
with shift either, but I may be mistaken.

These two methods should give the platform enough control to implement
their own scheme for optimized class space placement without bothering any
shared code about it.

Note about the form, I introduced two new platform dependent files,
"metaspace_<cpu>.cpp" and "compressedOops_<cpu>.cpp". I am not happy about
this but this seems to be what we generally do in hotspot, right?

---

Metaspace reserve alignment vs cds alignment

CDS was using Metaspace reserve alignment for CDS internal purposes. I
guess this was just a copy paste issue. It never caused problems since
Metaspace reserve alignment == page size, but that is not true anymore in
the upcoming Elastic Metaspace where reserve alignment will be larger. This
causes a number of issues.

I separated those two cleanly. CDS now uses os::vm_allocation_granularity.
Metaspace::reserve_alignment is only used in those two places where it is
needed, when CDS creates the address space for class space on behalf of the
Metaspace.

---

Windows special handling in CDS

To simplify coding I removed the windows specific handling which left out
reservation of the archive. This was needed because windows cannot mmap
files into reserved regions. But fallback code exists in filemap.cpp for
this case which just reads in the region instead of mapping it.

Should that turn out to be a performance problem, I will reinstate the
feature. But a simpler way would be reserve the archive and later just
before mmapping the archive file to release the archive space. That would
not only be simpler but give us the best guarantee that that address space
is actually available. But I'd be happy to leave that part out completely
if we do not see any performance problems on windows x64.

---

NMT cannot deal with spaces which are split. This problem manifests in that
bookkeeping for class space is done under "Shared Classes", not "Classes"
as it should. This problem exists today too at dump time and randomly at
run time. But since I simplified the reservation, this problem now shows up
always, whether or not we map at the SharedBaseAddress.
While I could work around this problem, I'd prefer this problem to be
solved at the core, and NMT to have an option to recognize reservation
splits. So I'd rather not put a workaround for this into the patch but
leave it for fixing as a separate issue. I opened this issue to track it
[6].

---

Jtreg tests:

I expanded the CompressedOops/CompressedClassPointers.java. I also extended
them to Windows. The tests now optionally omit strict class space placement
tests, since these tests heavily depend on ASLR and were the reason they
were excluded on Windows. However I think even without checking for class
space placement they make sense, just to see that the VM comes up and lives
with the many different settings we can run in.

---

Tests:

- I ran the patch through Oracles submit repo
- I ran tests manually for aarch64, zero, linux 32bit and windows x64
- The whole battery of nightly tests at SAP, including ppc, ppcle and
aarch64, unfortunately excluding windows because of unrelated errors.
Windows x64 tests will be redone tonight.


Thank you,

Thomas

[1]
https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-April/008804.html
[2]
https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-April/008757.html
[3] https://bugs.openjdk.java.net/browse/JDK-8193266
[4] https://bugs.openjdk.java.net/browse/JDK-8221173
[5]
https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-April/008765.html
[6] https://bugs.openjdk.java.net/browse/JDK-8243535