RFR: JDK-8311035: CDS should not use dump time JVM narrow Klass encoding to pre-compute Klass ids [v4]

[Thomas Stuefe]

> I recently spent a day trying to understand the interleaving of narrow Klass decoding with CDS. Thanks to @iklam for clarifying some details. This patch aims to make these interleavings easier to understand and more correct.
> 
> ----
> 
> CDS narrow Klass handling plays a role for archived heap objects. 
> 
> When dumping heap objects, we must recompute their narrow Klass ids since the relative positions of archived Klass instances change compared to their live counterparts in the dump time JVM.
> We recompute those narrow Klass ids using the following encoding scheme:
> - base = future assumed mapping start address
> - shift = dump time (!) JVMs encoding shift  (A)
> see `ArchiveHeapWriter::update_header_for_requested_obj` https://github.com/openjdk/jdk/blob/c3f10e847999ec254893de5a1a5de32fd07f715a/src/hotspot/share/cds/archiveHeapWriter.cpp#L419-L425
> 
> At CDS runtime, we load the CDS archive, then place the class space behind it. We then initialize narrow Klass encoding for the resulting combined Klass range such that:
> - encoding base is the range start address (mapping base)
> - encoding shift is always zero
> see `CompressedKlassPointers::initialize` : https://github.com/openjdk/jdk/blob/c3f10e847999ec254893de5a1a5de32fd07f715a/src/hotspot/share/oops/compressedOops.cpp#L195-L231
> 
> The lengthy comment there is misleading and partly wrong (regrettable since it was mine :)
> 
> At dump time, when initializing the JVM, we also set the encoding base to klass range start and shift to zero (also `CompressedKlassPointers::initialize`). That is the shift we later use for (A); hence, that shift is zero.
> 
> -------------------
> 
> There are some small things wrong with the current code. That wrongness does not lead to errors but makes understanding the code difficult.
> 
> Mainly, the *dump time* VM's narrow Klass encoding scheme is irrelevant for the encoding employed on the future runtime archive since we recalculate the narrow Klass ids for archived heap objects. That means:
> 
> In `CompressedKlassPointers::initialize` there is no need to fix the encoding base and shift for the *dump time* JVM. Dump time JVM can use whatever base+shift it pleases; it can use the same code path as when CDS is off (e.g. use zero-based encoding).
> 
> In `ArchiveHeapWriter::update_header_for_requested_obj`, we should not use the dump time JVM shift for pre-computing the narrow Klass ids. Instead, we should use the *minimal shift needed for the maximal possible future Klass range size*. The comment in ArchiveHeapWriter expla...

Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:

  fix comment

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14688/files
  - new: https://git.openjdk.org/jdk/pull/14688/files/807412ea..26a2b9d1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14688&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14688&range=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14688.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14688/head:pull/14688

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