8311035: CDS should not use dump time JVM narrow Klass encoding to pre-compute Klass ids

Reviewed-by: iklam

Author: tstuefe

src/hotspot/share/cds/archiveBuilder.cpp

@@ -809,12 +809,16 @@ uintx ArchiveBuilder::any_to_offset(address p) const {
   return buffer_to_offset(p);
 }
 
+#if INCLUDE_CDS_JAVA_HEAP
 narrowKlass ArchiveBuilder::get_requested_narrow_klass(Klass* k) {
   assert(DumpSharedSpaces, "sanity");
   k = get_buffered_klass(k);
   Klass* requested_k = to_requested(k);
-  return CompressedKlassPointers::encode_not_null(requested_k, _requested_static_archive_bottom);
+  address narrow_klass_base = _requested_static_archive_bottom; // runtime encoding base == runtime mapping start
+  const int narrow_klass_shift = ArchiveHeapWriter::precomputed_narrow_klass_shift;
+  return CompressedKlassPointers::encode_not_null(requested_k, narrow_klass_base, narrow_klass_shift);
 }
+#endif // INCLUDE_CDS_JAVA_HEAP
 
 // RelocateBufferToRequested --- Relocate all the pointers in rw/ro,
 // so that the archive can be mapped to the "requested" location without runtime relocation.

src/hotspot/share/cds/archiveHeapWriter.hpp

@@ -192,6 +192,18 @@ class ArchiveHeapWriter : AllStatic {
   static oop source_obj_to_requested_obj(oop src_obj);
   static oop buffered_addr_to_source_obj(address buffered_addr);
   static address buffered_addr_to_requested_addr(address buffered_addr);
+
+  // Archived heap object headers carry pre-computed narrow Klass ids calculated with the
+  // following scheme:
+  // 1) the encoding base must be the mapping start address.
+  // 2) shift must be large enough to result in an encoding range that covers the runtime Klass range.
+  //    That Klass range is defined by CDS archive size and runtime class space size. Luckily, the maximum
+  //    size can be predicted: archive size is assumed to be <1G, class space size capped at 3G, and at
+  //    runtime we put both regions adjacent to each other. Therefore, runtime Klass range size < 4G.
+  //    Since nKlass itself is 32 bit, our encoding range len is 4G, and since we set the base directly
+  //    at mapping start, these 4G are enough. Therefore, we don't need to shift at all (shift=0).
+  static constexpr int precomputed_narrow_klass_shift = 0;
+
 };
 #endif // INCLUDE_CDS_JAVA_HEAP
 #endif // SHARE_CDS_ARCHIVEHEAPWRITER_HPP

src/hotspot/share/cds/filemap.cpp

@@ -222,7 +222,6 @@ void FileMapHeader::populate(FileMapInfo *info, size_t core_region_alignment,
   _compressed_oops = UseCompressedOops;
   _compressed_class_ptrs = UseCompressedClassPointers;
   _max_heap_size = MaxHeapSize;
-  _narrow_klass_shift = CompressedKlassPointers::shift();
   _use_optimized_module_handling = MetaspaceShared::use_optimized_module_handling();
   _use_full_module_graph = MetaspaceShared::use_full_module_graph();
 
@@ -283,7 +282,6 @@ void FileMapHeader::print(outputStream* st) {
   st->print_cr("- compact_strings:                %d", _compact_strings);
   st->print_cr("- max_heap_size:                  " UINTX_FORMAT, _max_heap_size);
   st->print_cr("- narrow_oop_mode:                %d", _narrow_oop_mode);
-  st->print_cr("- narrow_klass_shift:             %d", _narrow_klass_shift);
   st->print_cr("- compressed_oops:                %d", _compressed_oops);
   st->print_cr("- compressed_class_ptrs:          %d", _compressed_class_ptrs);
   st->print_cr("- cloned_vtables_offset:          " SIZE_FORMAT_X, _cloned_vtables_offset);
@@ -2016,10 +2014,17 @@ bool FileMapInfo::can_use_heap_region() {
     // referenced objects are replaced. See HeapShared::initialize_from_archived_subgraph().
   }
 
+  // We pre-compute narrow Klass IDs with the runtime mapping start intended to be the base, and a shift of
+  // ArchiveHeapWriter::precomputed_narrow_klass_shift. We enforce this encoding at runtime (see
+  // CompressedKlassPointers::initialize_for_given_encoding()). Therefore, the following assertions must
+  // hold:
+  address archive_narrow_klass_base = (address)header()->mapped_base_address();
+  const int archive_narrow_klass_shift = ArchiveHeapWriter::precomputed_narrow_klass_shift;
+
   log_info(cds)("CDS archive was created with max heap size = " SIZE_FORMAT "M, and the following configuration:",
                 max_heap_size()/M);
-  log_info(cds)("    narrow_klass_base = " PTR_FORMAT ", narrow_klass_shift = %d",
-                p2i(narrow_klass_base()), narrow_klass_shift());
+  log_info(cds)("    narrow_klass_base at mapping start address, narrow_klass_shift = %d",
+                archive_narrow_klass_shift);
   log_info(cds)("    narrow_oop_mode = %d, narrow_oop_base = " PTR_FORMAT ", narrow_oop_shift = %d",
                 narrow_oop_mode(), p2i(narrow_oop_base()), narrow_oop_shift());
   log_info(cds)("    heap range = [" PTR_FORMAT " - "  PTR_FORMAT "]",
@@ -2037,11 +2042,11 @@ bool FileMapInfo::can_use_heap_region() {
                 UseCompressedOops ? p2i(CompressedOops::end()) :
                                     UseG1GC ? p2i((address)G1CollectedHeap::heap()->reserved().end()) : 0L);
 
-  if (narrow_klass_base() != CompressedKlassPointers::base() ||
-      narrow_klass_shift() != CompressedKlassPointers::shift()) {
-    log_info(cds)("CDS heap data cannot be used because the archive was created with an incompatible narrow klass encoding mode.");
-    return false;
-  }
+  assert(archive_narrow_klass_base == CompressedKlassPointers::base(), "Unexpected encoding base encountered "
+         "(" PTR_FORMAT ", expected " PTR_FORMAT ")", p2i(CompressedKlassPointers::base()), p2i(archive_narrow_klass_base));
+  assert(archive_narrow_klass_shift == CompressedKlassPointers::shift(), "Unexpected encoding shift encountered "
+         "(%d, expected %d)", CompressedKlassPointers::shift(), archive_narrow_klass_shift);
+
   return true;
 }
 

src/hotspot/share/cds/filemap.hpp

@@ -194,7 +194,6 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   bool   _compact_strings;                        // value of CompactStrings
   uintx  _max_heap_size;                          // java max heap size during dumping
   CompressedOops::Mode _narrow_oop_mode;          // compressed oop encoding mode
-  int     _narrow_klass_shift;                    // save narrow klass base and shift
   bool    _compressed_oops;                       // save the flag UseCompressedOops
   bool    _compressed_class_ptrs;                 // save the flag UseCompressedClassPointers
   size_t  _cloned_vtables_offset;                 // The address of the first cloned vtable
@@ -262,8 +261,6 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   bool compact_strings()                   const { return _compact_strings; }
   uintx max_heap_size()                    const { return _max_heap_size; }
   CompressedOops::Mode narrow_oop_mode()   const { return _narrow_oop_mode; }
-  int narrow_klass_shift()                 const { return _narrow_klass_shift; }
-  address narrow_klass_base()              const { return (address)mapped_base_address(); }
   char* cloned_vtables()                   const { return from_mapped_offset(_cloned_vtables_offset); }
   char* serialized_data()                  const { return from_mapped_offset(_serialized_data_offset); }
   address heap_begin()                     const { return _heap_begin; }
@@ -382,8 +379,6 @@ class FileMapInfo : public CHeapObj<mtInternal> {
   address narrow_oop_base()    const { return header()->narrow_oop_base(); }
   int     narrow_oop_shift()   const { return header()->narrow_oop_shift(); }
   uintx   max_heap_size()      const { return header()->max_heap_size(); }
-  address narrow_klass_base()  const { return header()->narrow_klass_base(); }
-  int     narrow_klass_shift() const { return header()->narrow_klass_shift(); }
   size_t  core_region_alignment() const { return header()->core_region_alignment(); }
 
   CompressedOops::Mode narrow_oop_mode()      const { return header()->narrow_oop_mode(); }

src/hotspot/share/cds/metaspaceShared.cpp

@@ -69,6 +69,7 @@
 #include "oops/oopHandle.hpp"
 #include "prims/jvmtiExport.hpp"
 #include "runtime/arguments.hpp"
+#include "runtime/globals.hpp"
 #include "runtime/globals_extension.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/os.inline.hpp"
@@ -1136,7 +1137,7 @@ MapArchiveResult MetaspaceShared::map_archives(FileMapInfo* static_mapinfo, File
 
   if (result == MAP_ARCHIVE_SUCCESS) {
     SharedBaseAddress = (size_t)mapped_base_address;
-    LP64_ONLY({
+#ifdef _LP64
         if (Metaspace::using_class_space()) {
           // Set up ccs in metaspace.
           Metaspace::initialize_class_space(class_space_rs);
@@ -1146,13 +1147,24 @@ MapArchiveResult MetaspaceShared::map_archives(FileMapInfo* static_mapinfo, File
           address cds_base = (address)static_mapinfo->mapped_base();
           address ccs_end = (address)class_space_rs.end();
           assert(ccs_end > cds_base, "Sanity check");
-          CompressedKlassPointers::initialize(cds_base, ccs_end - cds_base);
-
+#if INCLUDE_CDS_JAVA_HEAP
+          // We archived objects with pre-computed narrow Klass id. Set up encoding such that these Ids stay valid.
+          address precomputed_narrow_klass_base = cds_base;
+          const int precomputed_narrow_klass_shift = ArchiveHeapWriter::precomputed_narrow_klass_shift;
+          CompressedKlassPointers::initialize_for_given_encoding(
+            cds_base, ccs_end - cds_base, // Klass range
+            precomputed_narrow_klass_base, precomputed_narrow_klass_shift // precomputed encoding, see ArchiveHeapWriter
+            );
+#else
+          CompressedKlassPointers::initialize (
+            cds_base, ccs_end - cds_base // Klass range
+            );
+#endif // INCLUDE_CDS_JAVA_HEAP
           // map_or_load_heap_region() compares the current narrow oop and klass encodings
           // with the archived ones, so it must be done after all encodings are determined.
           static_mapinfo->map_or_load_heap_region();
         }
-      });
+#endif // _LP64
     log_info(cds)("optimized module handling: %s", MetaspaceShared::use_optimized_module_handling() ? "enabled" : "disabled");
     log_info(cds)("full module graph: %s", MetaspaceShared::use_full_module_graph() ? "enabled" : "disabled");
   } else {

src/hotspot/share/oops/compressedOops.cpp

@@ -186,83 +186,63 @@ NarrowPtrStruct CompressedKlassPointers::_narrow_klass = { nullptr, 0, true };
 //  are compiled for 32bit to LP64_ONLY).
 size_t CompressedKlassPointers::_range = 0;
 
+#ifdef _LP64
+
+// Given a klass range [addr, addr+len) and a given encoding scheme, assert that this scheme covers the range, then
+// set this encoding scheme. Used by CDS at runtime to re-instate the scheme used to pre-compute klass ids for
+// archived heap objects.
+void CompressedKlassPointers::initialize_for_given_encoding(address addr, size_t len, address requested_base, int requested_shift) {
+  assert(is_valid_base(requested_base), "Address must be a valid encoding base");
+  address const end = addr + len;
+
+  const int narrow_klasspointer_bits = sizeof(narrowKlass) * 8;
+  const size_t encoding_range_size = nth_bit(narrow_klasspointer_bits + requested_shift);
+  address encoding_range_end = requested_base + encoding_range_size;
+
+  // Note: it would be technically valid for the encoding base to precede the start of the Klass range. But we only call
+  // this function from CDS, and therefore know this to be true.
+  assert(requested_base == addr, "Invalid requested base");
+  assert(encoding_range_end >= end, "Encoding does not cover the full Klass range");
+
+  set_base(requested_base);
+  set_shift(requested_shift);
+  set_range(encoding_range_size);
+}
 
 // Given an address range [addr, addr+len) which the encoding is supposed to
 //  cover, choose base, shift and range.
 //  The address range is the expected range of uncompressed Klass pointers we
 //  will encounter (and the implicit promise that there will be no Klass
 //  structures outside this range).
 void CompressedKlassPointers::initialize(address addr, size_t len) {
-#ifdef _LP64
   assert(is_valid_base(addr), "Address must be a valid encoding base");
   address const end = addr + len;
 
   address base;
   int shift;
   size_t range;
 
-  if (UseSharedSpaces || DumpSharedSpaces) {
-
-    // Special requirements if CDS is active:
-    // Encoding base and shift must be the same between dump and run time.
-    //   CDS takes care that the SharedBaseAddress and CompressedClassSpaceSize
-    //   are the same. Archive size will be probably different at runtime, but
-    //   it can only be smaller than at, never larger, since archives get
-    //   shrunk at the end of the dump process.
-    //   From that it follows that the range [addr, len) we are handed in at
-    //   runtime will start at the same address then at dumptime, and its len
-    //   may be smaller at runtime then it was at dump time.
-    //
-    // To be very careful here, we avoid any optimizations and just keep using
-    //  the same address and shift value. Specifically we avoid using zero-based
-    //  encoding. We also set the expected value range to 4G (encoding range
-    //  cannot be larger than that).
-
+  // Attempt to run with encoding base == zero
+  if (end <= (address)KlassEncodingMetaspaceMax) {
+    base = 0;
+  } else {
     base = addr;
+  }
 
-    // JDK-8265705
-    // This is a temporary fix for aarch64: there, if the range-to-be-encoded is located
-    //  below 32g, either encoding base should be zero or base should be aligned to 4G
-    //  and shift should be zero. The simplest way to fix this for now is to force
-    //  shift to zero for both runtime and dumptime.
-    // Note however that this is not a perfect solution. Ideally this whole function
-    //  should be CDS agnostic, that would simplify it - and testing - a lot. See JDK-8267141
-    //  for details.
-    shift = 0;
-
-    // This must be true since at dumptime cds+ccs is 4G, at runtime it can
-    //  only be smaller, see comment above.
-    assert(len <= 4 * G, "Encoding range cannot be larger than 4G");
-    range = 4 * G;
+  // Highest offset a Klass* can ever have in relation to base.
+  range = end - base;
 
+  // We may not even need a shift if the range fits into 32bit:
+  const uint64_t UnscaledClassSpaceMax = (uint64_t(max_juint) + 1);
+  if (range < UnscaledClassSpaceMax) {
+    shift = 0;
   } else {
-
-    // Otherwise we attempt to use a zero base if the range fits in lower 32G.
-    if (end <= (address)KlassEncodingMetaspaceMax) {
-      base = 0;
-    } else {
-      base = addr;
-    }
-
-    // Highest offset a Klass* can ever have in relation to base.
-    range = end - base;
-
-    // We may not even need a shift if the range fits into 32bit:
-    const uint64_t UnscaledClassSpaceMax = (uint64_t(max_juint) + 1);
-    if (range < UnscaledClassSpaceMax) {
-      shift = 0;
-    } else {
-      shift = LogKlassAlignmentInBytes;
-    }
-
+    shift = LogKlassAlignmentInBytes;
   }
 
   set_base(base);
   set_shift(shift);
   set_range(range);
-#else
-  fatal("64bit only.");
-#endif
 }
 
 // Given an address p, return true if p can be used as an encoding base.
@@ -300,3 +280,5 @@ void CompressedKlassPointers::set_range(size_t range) {
   assert(UseCompressedClassPointers, "no compressed klass ptrs?");
   _range = range;
 }
+
+#endif // _LP64

src/hotspot/share/oops/compressedOops.hpp

@@ -155,17 +155,20 @@ class CompressedKlassPointers : public AllStatic {
 
   static void set_base(address base);
   static void set_range(size_t range);
-
-public:
-
   static void set_shift(int shift);
 
+public:
 
   // Given an address p, return true if p can be used as an encoding base.
   //  (Some platforms have restrictions of what constitutes a valid base
   //   address).
   static bool is_valid_base(address p);
 
+  // Given a klass range [addr, addr+len) and a given encoding scheme, assert that this scheme covers the range, then
+  // set this encoding scheme. Used by CDS at runtime to re-instate the scheme used to pre-compute klass ids for
+  // archived heap objects.
+  static void initialize_for_given_encoding(address addr, size_t len, address requested_base, int requested_shift);
+
   // Given an address range [addr, addr+len) which the encoding is supposed to
   //  cover, choose base, shift and range.
   //  The address range is the expected range of uncompressed Klass pointers we
@@ -182,13 +185,13 @@ class CompressedKlassPointers : public AllStatic {
   static bool is_null(Klass* v)      { return v == nullptr; }
   static bool is_null(narrowKlass v) { return v == 0; }
 
-  static inline Klass* decode_raw(narrowKlass v, address base);
+  static inline Klass* decode_raw(narrowKlass v, address base, int shift);
   static inline Klass* decode_raw(narrowKlass v);
   static inline Klass* decode_not_null(narrowKlass v);
-  static inline Klass* decode_not_null(narrowKlass v, address base);
+  static inline Klass* decode_not_null(narrowKlass v, address base, int shift);
   static inline Klass* decode(narrowKlass v);
   static inline narrowKlass encode_not_null(Klass* v);
-  static inline narrowKlass encode_not_null(Klass* v, address base);
+  static inline narrowKlass encode_not_null(Klass* v, address base, int shift);
   static inline narrowKlass encode(Klass* v);
 
 };

src/hotspot/share/oops/compressedOops.inline.hpp

@@ -123,20 +123,20 @@ static inline bool check_alignment(Klass* v) {
 }
 
 inline Klass* CompressedKlassPointers::decode_raw(narrowKlass v) {
-  return decode_raw(v, base());
+  return decode_raw(v, base(), shift());
 }
 
-inline Klass* CompressedKlassPointers::decode_raw(narrowKlass v, address narrow_base) {
-  return (Klass*)((uintptr_t)narrow_base +((uintptr_t)v << shift()));
+inline Klass* CompressedKlassPointers::decode_raw(narrowKlass v, address narrow_base, int shift) {
+  return (Klass*)((uintptr_t)narrow_base +((uintptr_t)v << shift));
 }
 
 inline Klass* CompressedKlassPointers::decode_not_null(narrowKlass v) {
-  return decode_not_null(v, base());
+  return decode_not_null(v, base(), shift());
 }
 
-inline Klass* CompressedKlassPointers::decode_not_null(narrowKlass v, address narrow_base) {
+inline Klass* CompressedKlassPointers::decode_not_null(narrowKlass v, address narrow_base, int shift) {
   assert(!is_null(v), "narrow klass value can never be zero");
-  Klass* result = decode_raw(v, narrow_base);
+  Klass* result = decode_raw(v, narrow_base, shift);
   assert(check_alignment(result), "address not aligned: " PTR_FORMAT, p2i(result));
   return result;
 }
@@ -146,17 +146,17 @@ inline Klass* CompressedKlassPointers::decode(narrowKlass v) {
 }
 
 inline narrowKlass CompressedKlassPointers::encode_not_null(Klass* v) {
-  return encode_not_null(v, base());
+  return encode_not_null(v, base(), shift());
 }
 
-inline narrowKlass CompressedKlassPointers::encode_not_null(Klass* v, address narrow_base) {
+inline narrowKlass CompressedKlassPointers::encode_not_null(Klass* v, address narrow_base, int shift) {
   assert(!is_null(v), "klass value can never be zero");
   assert(check_alignment(v), "Address not aligned");
   uint64_t pd = (uint64_t)(pointer_delta(v, narrow_base, 1));
   assert(KlassEncodingMetaspaceMax > pd, "change encoding max if new encoding");
-  uint64_t result = pd >> shift();
+  uint64_t result = pd >> shift;
   assert((result & CONST64(0xffffffff00000000)) == 0, "narrow klass pointer overflow");
-  assert(decode_not_null((narrowKlass)result, narrow_base) == v, "reversibility");
+  assert(decode_not_null((narrowKlass)result, narrow_base, shift) == v, "reversibility");
   return (narrowKlass)result;
 }