8241071: Generation of classes.jsa with -Xshare:dump is not deterministic

Reviewed-by: dholmes, stuefe

make/hotspot/symbols/symbols-unix

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -122,6 +122,7 @@ JVM_GetNestMembers
 JVM_GetPrimitiveArrayElement
 JVM_GetProperties
 JVM_GetProtectionDomain
+JVM_GetRandomSeedForCDSDump
 JVM_GetRecordComponents
 JVM_GetSimpleBinaryName
 JVM_GetStackAccessControlContext

src/hotspot/share/classfile/symbolTable.cpp

@@ -176,6 +176,11 @@ void SymbolTable::create_table ()  {
 }
 
 void SymbolTable::delete_symbol(Symbol* sym) {
+  if (Arguments::is_dumping_archive()) {
+    // Do not delete symbols as we may be in the middle of preparing the
+    // symbols for dumping.
+    return;
+  }
   if (sym->is_permanent()) {
     MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena
     // Deleting permanent symbol should not occur very often (insert race condition),
@@ -221,12 +226,18 @@ Symbol* SymbolTable::allocate_symbol(const char* name, int len, bool c_heap) {
 
   Symbol* sym;
   if (Arguments::is_dumping_archive()) {
+    // Need to make all symbols permanent -- or else some symbols may be GC'ed
+    // during the archive dumping code that's executed outside of a safepoint.
     c_heap = false;
   }
   if (c_heap) {
     // refcount starts as 1
     sym = new (len) Symbol((const u1*)name, len, 1);
     assert(sym != NULL, "new should call vm_exit_out_of_memory if C_HEAP is exhausted");
+  } else if (DumpSharedSpaces) {
+    // See comments inside Symbol::operator new(size_t, int)
+    sym = new (len) Symbol((const u1*)name, len, PERM_REFCOUNT);
+    assert(sym != NULL, "new should call vm_exit_out_of_memory if failed to allocate symbol during DumpSharedSpaces");
   } else {
     // Allocate to global arena
     MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -174,10 +174,22 @@ public:
   }
 };
 
+inline unsigned DumpTimeSharedClassTable_hash(InstanceKlass* const& k) {
+  if (DumpSharedSpaces) {
+    // Deterministic archive contents
+    uintx delta = k->name() - MetaspaceShared::symbol_rs_base();
+    return primitive_hash<uintx>(delta);
+  } else {
+    // Deterministic archive is not possible because classes can be loaded
+    // in multiple threads.
+    return primitive_hash<InstanceKlass*>(k);
+  }
+}
+
 class DumpTimeSharedClassTable: public ResourceHashtable<
   InstanceKlass*,
   DumpTimeSharedClassInfo,
-  primitive_hash<InstanceKlass*>,
+  &DumpTimeSharedClassTable_hash,
   primitive_equals<InstanceKlass*>,
   15889, // prime number
   ResourceObj::C_HEAP>

src/hotspot/share/include/jvm.h

@@ -176,6 +176,9 @@ JVM_GetVmArguments(JNIEnv *env);
 JNIEXPORT void JNICALL
 JVM_InitializeFromArchive(JNIEnv* env, jclass cls);
 
+JNIEXPORT jlong JNICALL
+JVM_GetRandomSeedForCDSDump();
+
 /*
  * java.lang.Throwable
  */

src/hotspot/share/memory/dynamicArchive.cpp

@@ -503,14 +503,13 @@ private:
   void write_archive(char* serialized_data);
 
   void init_first_dump_space(address reserved_bottom) {
-    address first_space_base = reserved_bottom;
     DumpRegion* mc_space = MetaspaceShared::misc_code_dump_space();
     DumpRegion* rw_space = MetaspaceShared::read_write_dump_space();
 
     // Use the same MC->RW->RO ordering as in the base archive.
-    MetaspaceShared::init_shared_dump_space(mc_space, first_space_base);
+    MetaspaceShared::init_shared_dump_space(mc_space);
     _current_dump_space = mc_space;
-    _last_verified_top = first_space_base;
+    _last_verified_top = reserved_bottom;
     _num_dump_regions_used = 1;
   }
 

src/hotspot/share/memory/filemap.cpp

@@ -1177,6 +1177,10 @@ void FileMapRegion::init(int region_index, char* base, size_t size, bool read_on
   _mapped_base = NULL;
 }
 
+static const char* region_names[] = {
+  "mc", "rw", "ro", "bm", "ca0", "ca1", "oa0", "oa1"
+};
+
 void FileMapInfo::write_region(int region, char* base, size_t size,
                                bool read_only, bool allow_exec) {
   Arguments::assert_is_dumping_archive();
@@ -1184,6 +1188,9 @@ void FileMapInfo::write_region(int region, char* base, size_t size,
   FileMapRegion* si = space_at(region);
   char* target_base;
 
+  const int num_regions = sizeof(region_names)/sizeof(region_names[0]);
+  assert(0 <= region && region < num_regions, "sanity");
+
   if (region == MetaspaceShared::bm) {
     target_base = NULL; // always NULL for bm region.
   } else {
@@ -1197,11 +1204,13 @@ void FileMapInfo::write_region(int region, char* base, size_t size,
 
   si->set_file_offset(_file_offset);
   char* requested_base = (target_base == NULL) ? NULL : target_base + MetaspaceShared::final_delta();
-  log_debug(cds)("Shared file region  %d: " SIZE_FORMAT_HEX_W(08)
-                 " bytes, addr " INTPTR_FORMAT " file offset " SIZE_FORMAT_HEX_W(08),
-                 region, size, p2i(requested_base), _file_offset);
-
   int crc = ClassLoader::crc32(0, base, (jint)size);
+  if (size > 0) {
+    log_debug(cds)("Shared file region (%-3s)  %d: " SIZE_FORMAT_W(8)
+                   " bytes, addr " INTPTR_FORMAT " file offset " SIZE_FORMAT_HEX_W(08)
+                   " crc 0x%08x",
+                   region_names[region], region, size, p2i(requested_base), _file_offset, crc);
+  }
   si->init(region, target_base, size, read_only, allow_exec, crc);
 
   if (base != NULL) {
@@ -1246,8 +1255,6 @@ void FileMapInfo::write_bitmap_region(const CHeapBitMap* ptrmap,
     write_oopmaps(open_oopmaps, curr_offset, buffer);
   }
 
-  log_debug(cds)("ptrmap = " INTPTR_FORMAT " (" SIZE_FORMAT " bytes)",
-                 p2i(buffer), size_in_bytes);
   write_region(MetaspaceShared::bm, (char*)buffer, size_in_bytes, /*read_only=*/true, /*allow_exec=*/false);
 }
 
@@ -1297,23 +1304,20 @@ size_t FileMapInfo::write_archive_heap_regions(GrowableArray<MemRegion> *heap_me
   }
 
   size_t total_size = 0;
-  for (int i = first_region_id, arr_idx = 0;
+  for (int i = 0; i < max_num_regions; i++) {
-           i < first_region_id + max_num_regions;
-           i++, arr_idx++) {
     char* start = NULL;
     size_t size = 0;
-    if (arr_idx < arr_len) {
+    if (i < arr_len) {
-      start = (char*)heap_mem->at(arr_idx).start();
+      start = (char*)heap_mem->at(i).start();
-      size = heap_mem->at(arr_idx).byte_size();
+      size = heap_mem->at(i).byte_size();
       total_size += size;
     }
 
-    log_debug(cds)("Archive heap region %d: " INTPTR_FORMAT " - " INTPTR_FORMAT " = " SIZE_FORMAT_W(8) " bytes",
+    int region_idx = i + first_region_id;
-                   i, p2i(start), p2i(start + size), size);
+    write_region(region_idx, start, size, false, false);
-    write_region(i, start, size, false, false);
     if (size > 0) {
-      space_at(i)->init_oopmap(oopmaps->at(arr_idx)._offset,
+      space_at(region_idx)->init_oopmap(oopmaps->at(i)._offset,
-                               oopmaps->at(arr_idx)._oopmap_size_in_bits);
+                                        oopmaps->at(i)._oopmap_size_in_bits);
     }
   }
   return total_size;

src/hotspot/share/memory/heapShared.cpp

@@ -142,6 +142,10 @@ oop HeapShared::archive_heap_object(oop obj, Thread* THREAD) {
   if (archived_oop != NULL) {
     Copy::aligned_disjoint_words(cast_from_oop<HeapWord*>(obj), cast_from_oop<HeapWord*>(archived_oop), len);
     MetaspaceShared::relocate_klass_ptr(archived_oop);
+    // Clear age -- it might have been set if a GC happened during -Xshare:dump
+    markWord mark = archived_oop->mark_raw();
+    mark = mark.set_age(0);
+    archived_oop->set_mark_raw(mark);
     ArchivedObjectCache* cache = archived_object_cache();
     cache->put(obj, archived_oop);
     log_debug(cds, heap)("Archived heap object " PTR_FORMAT " ==> " PTR_FORMAT,

src/hotspot/share/memory/metaspaceShared.cpp

@@ -77,6 +77,8 @@
 
 ReservedSpace MetaspaceShared::_shared_rs;
 VirtualSpace MetaspaceShared::_shared_vs;
+ReservedSpace MetaspaceShared::_symbol_rs;
+VirtualSpace MetaspaceShared::_symbol_vs;
 MetaspaceSharedStats MetaspaceShared::_stats;
 bool MetaspaceShared::_has_error_classes;
 bool MetaspaceShared::_archive_loading_failed = false;
@@ -120,21 +122,24 @@ char* DumpRegion::expand_top_to(char* newtop) {
     MetaspaceShared::report_out_of_space(_name, newtop - _top);
     ShouldNotReachHere();
   }
-  uintx delta;
+
-  if (DynamicDumpSharedSpaces) {
+  if (_rs == MetaspaceShared::shared_rs()) {
-    delta = DynamicArchive::object_delta_uintx(newtop);
+    uintx delta;
-  } else {
+    if (DynamicDumpSharedSpaces) {
-    delta = MetaspaceShared::object_delta_uintx(newtop);
+      delta = DynamicArchive::object_delta_uintx(newtop);
-  }
+    } else {
-  if (delta > MAX_SHARED_DELTA) {
+      delta = MetaspaceShared::object_delta_uintx(newtop);
-    // This is just a sanity check and should not appear in any real world usage. This
+    }
-    // happens only if you allocate more than 2GB of shared objects and would require
+    if (delta > MAX_SHARED_DELTA) {
-    // millions of shared classes.
+      // This is just a sanity check and should not appear in any real world usage. This
-    vm_exit_during_initialization("Out of memory in the CDS archive",
+      // happens only if you allocate more than 2GB of shared objects and would require
-                                  "Please reduce the number of shared classes.");
+      // millions of shared classes.
+      vm_exit_during_initialization("Out of memory in the CDS archive",
+                                    "Please reduce the number of shared classes.");
+    }
   }
 
-  MetaspaceShared::commit_shared_space_to(newtop);
+  MetaspaceShared::commit_to(_rs, _vs, newtop);
   _top = newtop;
   return _top;
 }
@@ -172,26 +177,35 @@ void DumpRegion::print_out_of_space_msg(const char* failing_region, size_t neede
   }
 }
 
+void DumpRegion::init(ReservedSpace* rs, VirtualSpace* vs) {
+  _rs = rs;
+  _vs = vs;
+  // Start with 0 committed bytes. The memory will be committed as needed by
+  // MetaspaceShared::commit_to().
+  if (!_vs->initialize(*_rs, 0)) {
+    fatal("Unable to allocate memory for shared space");
+  }
+  _base = _top = _rs->base();
+  _end = _rs->end();
+}
+
 void DumpRegion::pack(DumpRegion* next) {
   assert(!is_packed(), "sanity");
   _end = (char*)align_up(_top, Metaspace::reserve_alignment());
   _is_packed = true;
   if (next != NULL) {
+    next->_rs = _rs;
+    next->_vs = _vs;
     next->_base = next->_top = this->_end;
-    next->_end = MetaspaceShared::shared_rs()->end();
+    next->_end = _rs->end();
   }
 }
 
-static DumpRegion _mc_region("mc"), _ro_region("ro"), _rw_region("rw");
+static DumpRegion _mc_region("mc"), _ro_region("ro"), _rw_region("rw"), _symbol_region("symbols");
 static size_t _total_closed_archive_region_size = 0, _total_open_archive_region_size = 0;
 
-void MetaspaceShared::init_shared_dump_space(DumpRegion* first_space, address first_space_bottom) {
+void MetaspaceShared::init_shared_dump_space(DumpRegion* first_space) {
-  // Start with 0 committed bytes. The memory will be committed as needed by
+  first_space->init(&_shared_rs, &_shared_vs);
-  // MetaspaceShared::commit_shared_space_to().
-  if (!_shared_vs.initialize(_shared_rs, 0)) {
-    fatal("Unable to allocate memory for shared space");
-  }
-  first_space->init(&_shared_rs, (char*)first_space_bottom);
 }
 
 DumpRegion* MetaspaceShared::misc_code_dump_space() {
@@ -211,6 +225,10 @@ void MetaspaceShared::pack_dump_space(DumpRegion* current, DumpRegion* next,
   current->pack(next);
 }
 
+char* MetaspaceShared::symbol_space_alloc(size_t num_bytes) {
+  return _symbol_region.allocate(num_bytes);
+}
+
 char* MetaspaceShared::misc_code_space_alloc(size_t num_bytes) {
   return _mc_region.allocate(num_bytes);
 }
@@ -320,6 +338,14 @@ void MetaspaceShared::initialize_dumptime_shared_and_meta_spaces() {
   SharedBaseAddress = (size_t)_shared_rs.base();
   log_info(cds)("Allocated shared space: " SIZE_FORMAT " bytes at " PTR_FORMAT,
                 _shared_rs.size(), p2i(_shared_rs.base()));
+
+  size_t symbol_rs_size = LP64_ONLY(3 * G) NOT_LP64(128 * M);
+  _symbol_rs = ReservedSpace(symbol_rs_size);
+  if (!_symbol_rs.is_reserved()) {
+    vm_exit_during_initialization("Unable to reserve memory for symbols",
+                                  err_msg(SIZE_FORMAT " bytes.", symbol_rs_size));
+  }
+  _symbol_region.init(&_symbol_rs, &_symbol_vs);
 }
 
 // Called by universe_post_init()
@@ -397,33 +423,37 @@ void MetaspaceShared::read_extra_data(const char* filename, TRAPS) {
   }
 }
 
-void MetaspaceShared::commit_shared_space_to(char* newtop) {
+void MetaspaceShared::commit_to(ReservedSpace* rs, VirtualSpace* vs, char* newtop) {
   Arguments::assert_is_dumping_archive();
-  char* base = _shared_rs.base();
+  char* base = rs->base();
   size_t need_committed_size = newtop - base;
-  size_t has_committed_size = _shared_vs.committed_size();
+  size_t has_committed_size = vs->committed_size();
   if (need_committed_size < has_committed_size) {
     return;
   }
 
   size_t min_bytes = need_committed_size - has_committed_size;
   size_t preferred_bytes = 1 * M;
-  size_t uncommitted = _shared_vs.reserved_size() - has_committed_size;
+  size_t uncommitted = vs->reserved_size() - has_committed_size;
 
   size_t commit =MAX2(min_bytes, preferred_bytes);
   commit = MIN2(commit, uncommitted);
   assert(commit <= uncommitted, "sanity");
 
-  bool result = _shared_vs.expand_by(commit, false);
+  bool result = vs->expand_by(commit, false);
-  ArchivePtrMarker::expand_ptr_end((address*)_shared_vs.high());
+  if (rs == &_shared_rs) {
+    ArchivePtrMarker::expand_ptr_end((address*)vs->high());
+  }
 
   if (!result) {
     vm_exit_during_initialization(err_msg("Failed to expand shared space to " SIZE_FORMAT " bytes",
                                           need_committed_size));
   }
 
-  log_debug(cds)("Expanding shared spaces by " SIZE_FORMAT_W(7) " bytes [total " SIZE_FORMAT_W(9)  " bytes ending at %p]",
+  assert(rs == &_shared_rs || rs == &_symbol_rs, "must be");
-                 commit, _shared_vs.actual_committed_size(), _shared_vs.high());
+  const char* which = (rs == &_shared_rs) ? "shared" : "symbol";
+  log_debug(cds)("Expanding %s spaces by " SIZE_FORMAT_W(7) " bytes [total " SIZE_FORMAT_W(9)  " bytes ending at %p]",
+                 which, commit, vs->actual_committed_size(), vs->high());
 }
 
 void MetaspaceShared::initialize_ptr_marker(CHeapBitMap* ptrmap) {
@@ -506,6 +536,10 @@ uintx MetaspaceShared::object_delta_uintx(void* obj) {
 // is run at a safepoint just before exit, this is the entire set of classes.
 static GrowableArray<Klass*>* _global_klass_objects;
 
+static int global_klass_compare(Klass** a, Klass **b) {
+  return a[0]->name()->fast_compare(b[0]->name());
+}
+
 GrowableArray<Klass*>* MetaspaceShared::collected_klasses() {
   return _global_klass_objects;
 }
@@ -1331,7 +1365,14 @@ public:
       RefRelocator ext_reloc;
       iterate_roots(&ext_reloc);
     }
-
+    {
+      log_info(cds)("Fixing symbol identity hash ... ");
+      os::init_random(0x12345678);
+      GrowableArray<Symbol*>* symbols = _ssc->get_sorted_symbols();
+      for (int i=0; i<symbols->length(); i++) {
+        symbols->at(i)->update_identity_hash();
+      }
+    }
 #ifdef ASSERT
     {
       log_info(cds)("Verifying external roots ... ");
@@ -1364,6 +1405,21 @@ public:
   }
 
   static void iterate_roots(MetaspaceClosure* it) {
+    // To ensure deterministic contents in the archive, we just need to ensure that
+    // we iterate the MetsapceObjs in a deterministic order. It doesn't matter where
+    // the MetsapceObjs are located originally, as they are copied sequentially into
+    // the archive during the iteration.
+    //
+    // The only issue here is that the symbol table and the system directories may be
+    // randomly ordered, so we copy the symbols and klasses into two arrays and sort
+    // them deterministically.
+    //
+    // During -Xshare:dump, the order of Symbol creation is strictly determined by
+    // the SharedClassListFile (class loading is done in a single thread and the JIT
+    // is disabled). Also, Symbols are allocated in monotonically increasing addresses
+    // (see Symbol::operator new(size_t, int)). So if we iterate the Symbols by
+    // ascending address order, we ensure that all Symbols are copied into deterministic
+    // locations in the archive.
     GrowableArray<Symbol*>* symbols = _ssc->get_sorted_symbols();
     for (int i=0; i<symbols->length(); i++) {
       it->push(symbols->adr_at(i));
@@ -1525,6 +1581,7 @@ void VM_PopulateDumpSharedSpace::doit() {
   _global_klass_objects = new GrowableArray<Klass*>(1000);
   CollectClassesClosure collect_classes;
   ClassLoaderDataGraph::loaded_classes_do(&collect_classes);
+  _global_klass_objects->sort(global_klass_compare);
 
   print_class_stats();
 
@@ -1558,8 +1615,10 @@ void VM_PopulateDumpSharedSpace::doit() {
   _ro_region.pack();
 
   // The vtable clones contain addresses of the current process.
-  // We don't want to write these addresses into the archive.
+  // We don't want to write these addresses into the archive. Same for i2i buffer.
   MetaspaceShared::zero_cpp_vtable_clones_for_writing();
+  memset(MetaspaceShared::i2i_entry_code_buffers(), 0,
+         MetaspaceShared::i2i_entry_code_buffers_size());
 
   // relocate the data so that it can be mapped to Arguments::default_SharedBaseAddress()
   // without runtime relocation.
@@ -1631,7 +1690,7 @@ void VM_PopulateDumpSharedSpace::print_region_stats(FileMapInfo *map_info) {
   _mc_region.print(total_reserved);
   _rw_region.print(total_reserved);
   _ro_region.print(total_reserved);
-  print_bitmap_region_stats(bitmap_reserved, total_reserved);
+  print_bitmap_region_stats(bitmap_used, total_reserved);
   print_heap_region_stats(_closed_archive_heap_regions, "ca", total_reserved);
   print_heap_region_stats(_open_archive_heap_regions, "oa", total_reserved);
 
@@ -1640,8 +1699,8 @@ void VM_PopulateDumpSharedSpace::print_region_stats(FileMapInfo *map_info) {
 }
 
 void VM_PopulateDumpSharedSpace::print_bitmap_region_stats(size_t size, size_t total_size) {
-  log_debug(cds)("bm  space: " SIZE_FORMAT_W(9) " [ %4.1f%% of total] out of " SIZE_FORMAT_W(9) " bytes [100.0%% used] at " INTPTR_FORMAT,
+  log_debug(cds)("bm  space: " SIZE_FORMAT_W(9) " [ %4.1f%% of total] out of " SIZE_FORMAT_W(9) " bytes [100.0%% used]",
-                 size, size/double(total_size)*100.0, size, p2i(NULL));
+                 size, size/double(total_size)*100.0, size);
 }
 
 void VM_PopulateDumpSharedSpace::print_heap_region_stats(GrowableArray<MemRegion> *heap_mem,

src/hotspot/share/memory/metaspaceShared.hpp

@@ -63,6 +63,8 @@ private:
   char* _top;
   char* _end;
   bool _is_packed;
+  ReservedSpace* _rs;
+  VirtualSpace* _vs;
 
 public:
   DumpRegion(const char* name) : _name(name), _base(NULL), _top(NULL), _end(NULL), _is_packed(false) {}
@@ -85,19 +87,7 @@ public:
   void print(size_t total_bytes) const;
   void print_out_of_space_msg(const char* failing_region, size_t needed_bytes);
 
-  void init(const ReservedSpace* rs, char* base) {
+  void init(ReservedSpace* rs, VirtualSpace* vs);
-    if (base == NULL) {
-      base = rs->base();
-    }
-    assert(rs->contains(base), "must be");
-    _base = _top = base;
-    _end = rs->end();
-  }
-  void init(char* b, char* t, char* e) {
-    _base = b;
-    _top = t;
-    _end = e;
-  }
 
   void pack(DumpRegion* next = NULL);
 
@@ -178,6 +168,8 @@ class MetaspaceShared : AllStatic {
   // CDS support
   static ReservedSpace _shared_rs;
   static VirtualSpace _shared_vs;
+  static ReservedSpace _symbol_rs;
+  static VirtualSpace _symbol_vs;
   static int _max_alignment;
   static MetaspaceSharedStats _stats;
   static bool _has_error_classes;
@@ -222,11 +214,15 @@ class MetaspaceShared : AllStatic {
     NOT_CDS(return NULL);
   }
 
+  static Symbol* symbol_rs_base() {
+    return (Symbol*)_symbol_rs.base();
+  }
+
   static void set_shared_rs(ReservedSpace rs) {
     CDS_ONLY(_shared_rs = rs);
   }
 
-  static void commit_shared_space_to(char* newtop) NOT_CDS_RETURN;
+  static void commit_to(ReservedSpace* rs, VirtualSpace* vs, char* newtop) NOT_CDS_RETURN;
   static void initialize_dumptime_shared_and_meta_spaces() NOT_CDS_RETURN;
   static void initialize_runtime_shared_and_meta_spaces() NOT_CDS_RETURN;
   static void post_initialize(TRAPS) NOT_CDS_RETURN;
@@ -302,7 +298,7 @@ class MetaspaceShared : AllStatic {
 #if INCLUDE_CDS
   static ReservedSpace reserve_shared_space(size_t size, char* requested_address = NULL);
   static size_t reserved_space_alignment();
-  static void init_shared_dump_space(DumpRegion* first_space, address first_space_bottom = NULL);
+  static void init_shared_dump_space(DumpRegion* first_space);
   static DumpRegion* misc_code_dump_space();
   static DumpRegion* read_write_dump_space();
   static DumpRegion* read_only_dump_space();
@@ -312,7 +308,10 @@ class MetaspaceShared : AllStatic {
   static void rewrite_nofast_bytecodes_and_calculate_fingerprints(Thread* thread, InstanceKlass* ik);
 #endif
 
-  // Allocate a block of memory from the "mc", "ro", or "rw" regions.
+  // Allocate a block of memory from the temporary "symbol" region.
+  static char* symbol_space_alloc(size_t num_bytes);
+
+  // Allocate a block of memory from the "mc" or "ro" regions.
   static char* misc_code_space_alloc(size_t num_bytes);
   static char* read_only_space_alloc(size_t num_bytes);
 

src/hotspot/share/oops/instanceKlass.cpp

@@ -505,10 +505,6 @@ InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind, Klass
   assert(is_instance_klass(), "is layout incorrect?");
   assert(size_helper() == parser.layout_size(), "incorrect size_helper?");
 
-  if (Arguments::is_dumping_archive()) {
-    SystemDictionaryShared::init_dumptime_info(this);
-  }
-
   // Set biased locking bit for all instances of this class; it will be
   // cleared if revocation occurs too often for this type
   if (UseBiasedLocking && BiasedLocking::enabled()) {

src/hotspot/share/oops/klass.cpp

@@ -29,6 +29,7 @@
 #include "classfile/javaClasses.hpp"
 #include "classfile/moduleEntry.hpp"
 #include "classfile/systemDictionary.hpp"
+#include "classfile/systemDictionaryShared.hpp"
 #include "classfile/vmSymbols.hpp"
 #include "gc/shared/collectedHeap.inline.hpp"
 #include "logging/log.hpp"
@@ -79,6 +80,10 @@ void Klass::set_is_cloneable() {
 void Klass::set_name(Symbol* n) {
   _name = n;
   if (_name != NULL) _name->increment_refcount();
+
+  if (Arguments::is_dumping_archive() && is_instance_klass()) {
+    SystemDictionaryShared::init_dumptime_info(InstanceKlass::cast(this));
+  }
 }
 
 bool Klass::is_subclass_of(const Klass* k) const {

src/hotspot/share/oops/symbol.cpp

@@ -30,6 +30,7 @@
 #include "logging/log.hpp"
 #include "logging/logStream.hpp"
 #include "memory/allocation.inline.hpp"
+#include "memory/metaspaceShared.hpp"
 #include "memory/resourceArea.hpp"
 #include "memory/universe.hpp"
 #include "oops/symbol.hpp"
@@ -56,6 +57,20 @@ Symbol::Symbol(const u1* name, int length, int refcount) {
 }
 
 void* Symbol::operator new(size_t sz, int len) throw() {
+ if (DumpSharedSpaces) {
+    // To get deterministic output from -Xshare:dump, we ensure that Symbols are allocated in
+    // increasing addresses. When the symbols are copied into the archive, we preserve their
+    // relative address order (see SortedSymbolClosure in metaspaceShared.cpp)
+    //
+    // We cannot use arena because arena chunks are allocated by the OS. As a result, for example,
+    // the archived symbol of "java/lang/Object" may sometimes be lower than "java/lang/String", and
+    // sometimes be higher. This would cause non-deterministic contents in the archive.
+   DEBUG_ONLY(static void* last = 0);
+   void* p = (void*)MetaspaceShared::symbol_space_alloc(size(len)*wordSize);
+   assert(p > last, "must increase monotonically");
+   DEBUG_ONLY(last = p);
+   return p;
+ }
   int alloc_size = size(len)*wordSize;
   address res = (address) AllocateHeap(alloc_size, mtSymbol);
   return res;
@@ -72,11 +87,21 @@ void Symbol::operator delete(void *p) {
   FreeHeap(p);
 }
 
+#if INCLUDE_CDS
+void Symbol::update_identity_hash() {
+  // This is called at a safepoint during dumping of a static CDS archive. The caller should have
+  // called os::init_random() with a deterministic seed and then iterate all archived Symbols in
+  // a deterministic order.
+  assert(SafepointSynchronize::is_at_safepoint(), "must be at a safepoint");
+  _hash_and_refcount =  pack_hash_and_refcount((short)os::random(), PERM_REFCOUNT);
+}
+
 void Symbol::set_permanent() {
   // This is called at a safepoint during dumping of a dynamic CDS archive.
   assert(SafepointSynchronize::is_at_safepoint(), "must be at a safepoint");
   _hash_and_refcount =  pack_hash_and_refcount(extract_hash(_hash_and_refcount), PERM_REFCOUNT);
 }
+#endif
 
 // ------------------------------------------------------------------
 // Symbol::index_of

src/hotspot/share/oops/symbol.hpp

@@ -168,7 +168,8 @@ class Symbol : public MetaspaceObj {
   bool is_permanent() const {
     return (refcount() == PERM_REFCOUNT);
   }
-  void set_permanent();
+  void update_identity_hash() NOT_CDS_RETURN;
+  void set_permanent() NOT_CDS_RETURN;
   void make_permanent();
 
   // Function char_at() returns the Symbol's selected u1 byte as a char type.

src/hotspot/share/prims/jvm.cpp

@@ -3733,6 +3733,29 @@ JVM_ENTRY(void, JVM_InitializeFromArchive(JNIEnv* env, jclass cls))
   HeapShared::initialize_from_archived_subgraph(k);
 JVM_END
 
+JVM_ENTRY_NO_ENV(jlong, JVM_GetRandomSeedForCDSDump())
+  JVMWrapper("JVM_GetRandomSeedForCDSDump");
+  if (DumpSharedSpaces) {
+    const char* release = Abstract_VM_Version::vm_release();
+    const char* dbg_level = Abstract_VM_Version::jdk_debug_level();
+    const char* version = VM_Version::internal_vm_info_string();
+    jlong seed = (jlong)(java_lang_String::hash_code((const jbyte*)release, (int)strlen(release)) ^
+                         java_lang_String::hash_code((const jbyte*)dbg_level, (int)strlen(dbg_level)) ^
+                         java_lang_String::hash_code((const jbyte*)version, (int)strlen(version)));
+    seed += (jlong)Abstract_VM_Version::vm_major_version();
+    seed += (jlong)Abstract_VM_Version::vm_minor_version();
+    seed += (jlong)Abstract_VM_Version::vm_security_version();
+    seed += (jlong)Abstract_VM_Version::vm_patch_version();
+    if (seed == 0) { // don't let this ever be zero.
+      seed = 0x87654321;
+    }
+    log_debug(cds)("JVM_GetRandomSeedForCDSDump() = " JLONG_FORMAT, seed);
+    return seed;
+  } else {
+    return 0;
+  }
+JVM_END
+
 // Returns an array of all live Thread objects (VM internal JavaThreads,
 // jvmti agent threads, and JNI attaching threads  are skipped)
 // See CR 6404306 regarding JNI attaching threads

src/java.base/share/classes/java/util/ImmutableCollections.java

@@ -64,14 +64,22 @@ class ImmutableCollections {
     private static final boolean REVERSE;
     static {
         // to generate a reasonably random and well-mixed SALT, use an arbitrary
-        // value (a slice of pi), multiply with the System.nanoTime, then pick
+        // value (a slice of pi), multiply with a random seed, then pick
         // the mid 32-bits from the product. By picking a SALT value in the
         // [0 ... 0xFFFF_FFFFL == 2^32-1] range, we ensure that for any positive
         // int N, (SALT32L * N) >> 32 is a number in the [0 ... N-1] range. This
         // property will be used to avoid more expensive modulo-based
         // calculations.
         long color = 0x243F_6A88_85A3_08D3L; // slice of pi
-        long seed = System.nanoTime();
+
+        // When running with -Xshare:dump, the VM will supply a "random" seed that's
+        // derived from the JVM build/version, so can we generate the exact same
+        // CDS archive for the same JDK build. This makes it possible to verify the
+        // consistency of the JDK build.
+        long seed = VM.getRandomSeedForCDSDump();
+        if (seed == 0) {
+          seed = System.nanoTime();
+        }
         SALT32L = (int)((color * seed) >> 16) & 0xFFFF_FFFFL;
         // use the lowest bit to determine if we should reverse iteration
         REVERSE = (SALT32L & 1) == 0;

src/java.base/share/classes/jdk/internal/misc/VM.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -421,6 +421,8 @@ public class VM {
      */
     public static native void initializeFromArchive(Class<?> c);
 
+    public static native long getRandomSeedForCDSDump();
+
     /**
      * Provides access to information on buffer usage.
      */

src/java.base/share/native/libjava/VM.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -61,3 +61,8 @@ Java_jdk_internal_misc_VM_initializeFromArchive(JNIEnv *env, jclass ignore,
                                                 jclass c) {
     JVM_InitializeFromArchive(env, c);
 }
+
+JNIEXPORT jlong JNICALL
+Java_jdk_internal_misc_VM_getRandomSeedForCDSDump(JNIEnv *env, jclass ignore) {
+    return JVM_GetRandomSeedForCDSDump();
+}

test/hotspot/jtreg/TEST.groups

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -348,7 +348,8 @@ hotspot_cds_relocation = \
   runtime/modules/PatchModule/PatchModuleCDS.java \
   runtime/modules/PatchModule/PatchModuleClassList.java \
   runtime/NMT \
-  serviceability/sa
+  serviceability/sa \
+ -runtime/cds/DeterministicDump.java
 
 # A subset of AppCDS tests to be run in tier1
 tier1_runtime_appcds = \

test/hotspot/jtreg/runtime/cds/DeterministicDump.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8241071
+ * @summary The same JDK build should always generate the same archive file (no randomness).
+ * @requires vm.cds
+ * @library /test/lib
+ * @run driver DeterministicDump
+ */
+
+import jdk.test.lib.cds.CDSTestUtils;
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+import java.io.FileInputStream;
+import java.io.IOException;
+
+public class DeterministicDump {
+    public static void main(String[] args) throws Exception {
+        for (int c = 0; c < 2; c++) { //  oop/klass compression
+            String sign = (c == 0) ?  "+" : "-";
+            String coop = "-XX:" + sign + "UseCompressedOops";
+            String ckls = "-XX:" + sign + "UseCompressedClassPointers";
+
+            if (!Platform.is64bit()) {
+                coop = "-showversion"; // no-op
+                ckls = "-showversion"; // no-op
+            }
+
+            for (int gc = 0; gc < 2; gc++) { // should we trigger GC during dump
+                for (int i = 0; i < 2; i++) {
+                    String metaspaceSize = "-showversion"; // no-op
+                    if (gc == 1 && i == 1) {
+                        // This will cause GC to happen after we've allocated 1MB of metaspace objects
+                        // while processing the built-in SharedClassListFile.
+                        metaspaceSize = "-XX:MetaspaceSize=1M";
+                    }
+                    ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+                                      coop, ckls, metaspaceSize,
+                                      "-XX:SharedArchiveFile=SharedArchiveFile" + i + ".jsa",
+                                      "-Xshare:dump", "-Xlog:cds=debug");
+                    OutputAnalyzer out = CDSTestUtils.executeAndLog(pb, "SharedArchiveFile" + i);
+                    CDSTestUtils.checkDump(out);
+                }
+                compare("SharedArchiveFile0.jsa", "SharedArchiveFile1.jsa");
+            }
+        }
+    }
+
+    static void compare(String file0, String file1) throws Exception {
+        byte[] buff0 = new byte[4096];
+        byte[] buff1 = new byte[4096];
+        try (FileInputStream in0 = new FileInputStream(file0);
+             FileInputStream in1 = new FileInputStream(file1)) {
+            int total = 0;
+            while (true) {
+                int n0 = read(in0, buff0);
+                int n1 = read(in1, buff1);
+                if (n0 != n1) {
+                    throw new RuntimeException("File contents (file sizes?) are different after " + total + " bytes; n0 = "
+                                               + n0 + ", n1 = " + n1);
+                }
+                if (n0 == 0) {
+                    System.out.println("File contents are the same: " + total + " bytes");
+                    break;
+                }
+                for (int i = 0; i < n0; i++) {
+                    byte b0 = buff0[i];
+                    byte b1 = buff1[i];
+                    if (b0 != b1) {
+                        throw new RuntimeException("File content different at byte #" + (total + i) + ", b0 = " + b0 + ", b1 = " + b1);
+                    }
+                }
+                total += n0;
+            }
+        }
+    }
+
+    static int read(FileInputStream in, byte[] buff) throws IOException {
+        int total = 0;
+        while (total < buff.length) {
+            int n = in.read(buff, total, buff.length - total);
+            if (n <= 0) {
+                return total;
+            }
+            total += n;
+        }
+
+        return total;
+    }
+}

test/hotspot/jtreg/runtime/cds/SpaceUtilizationCheck.java

@@ -43,6 +43,7 @@ import java.util.Hashtable;
 import java.lang.Integer;
 
 public class SpaceUtilizationCheck {
+    // For the MC/RW/RO regions:
     // [1] Each region must have strictly less than
     //     WhiteBox.metaspaceReserveAlignment() bytes of unused space.
     // [2] There must be no gap between two consecutive regions.
@@ -64,11 +65,21 @@ public class SpaceUtilizationCheck {
         opts.addSuffix(extra_options);
         OutputAnalyzer output = CDSTestUtils.createArchive(opts);
         CDSTestUtils.checkDump(output);
-        Pattern pattern = Pattern.compile("(..) *space: *([0-9]+).* out of *([0-9]+) bytes .* at 0x([0-9a0-f]+)");
+        Pattern pattern = Pattern.compile("(..)  space: *([0-9]+).* out of *([0-9]+) bytes .* at 0x([0-9a0-f]+)");
         WhiteBox wb = WhiteBox.getWhiteBox();
         long reserve_alignment = wb.metaspaceReserveAlignment();
         System.out.println("Metaspace::reserve_alignment() = " + reserve_alignment);
 
+        // Look for output like this. The pattern will only match the first 3 regions, which is what we need to check
+        //
+        // [4.682s][debug][cds] mc  space:     24912 [  0.2% of total] out of     28672 bytes [ 86.9% used] at 0x0000000800000000
+        // [4.682s][debug][cds] rw  space:   4391632 [ 33.7% of total] out of   4395008 bytes [ 99.9% used] at 0x0000000800007000
+        // [4.682s][debug][cds] ro  space:   7570632 [ 58.0% of total] out of   7573504 bytes [100.0% used] at 0x0000000800438000
+        // [4.682s][debug][cds] bm  space:    213528 [  1.6% of total] out of    213528 bytes [100.0% used]
+        // [4.682s][debug][cds] ca0 space:    507904 [  3.9% of total] out of    507904 bytes [100.0% used] at 0x00000000fff00000
+        // [4.682s][debug][cds] oa0 space:    327680 [  2.5% of total] out of    327680 bytes [100.0% used] at 0x00000000ffe00000
+        // [4.682s][debug][cds] total    :  13036288 [100.0% of total] out of  13049856 bytes [ 99.9% used]
+
         long last_region = -1;
         Hashtable<String,String> checked = new Hashtable<>();
         for (String line : output.getStdout().split("\n")) {
@@ -76,13 +87,8 @@ public class SpaceUtilizationCheck {
                 Matcher matcher = pattern.matcher(line);
                 if (matcher.find()) {
                     String name = matcher.group(1);
-                    if (name.equals("bm")) {
+                    System.out.println("Checking " + name + " in : " + line);
-                      // Bitmap space does not have a requested address.
+                    checked.put(name, name);
-                      break;
-                    } else {
-                      System.out.println("Checking " + name + " in : " + line);
-                      checked.put(name, name);
-                    }
                     long used = Long.parseLong(matcher.group(2));
                     long capacity = Long.parseLong(matcher.group(3));
                     long address = Long.parseLong(matcher.group(4), 16);