8256172: Clean up CDS handling of i2i_entry

Reviewed-by: minqi, ccheung

Author: iklam

src/hotspot/share/interpreter/abstractInterpreter.cpp

@@ -200,19 +200,27 @@ AbstractInterpreter::MethodKind AbstractInterpreter::method_kind(const methodHan
 
 #if INCLUDE_CDS
 
-address AbstractInterpreter::get_trampoline_code_buffer(AbstractInterpreter::MethodKind kind) {
+// For a shared Method m, to improve sharing across processes, we avoid writing to m->_i2i_entry
+// at runtime. Instead, m->_i2i_entry points to a fixed location inside the CDS archive.
+// This location contains a trampoline (generated by generate_entry_for_cds_method)
+// which jumps to _entry_table[kind].
+address AbstractInterpreter::entry_for_cds_method(const methodHandle& m) {
+  MethodKind kind = method_kind(m);
+  assert(0 <= kind && kind < number_of_method_entries, "illegal kind");
+  return entry_for_cds_method(kind);
+}
+
+address AbstractInterpreter::entry_for_cds_method(AbstractInterpreter::MethodKind kind) {
   const size_t trampoline_size = SharedRuntime::trampoline_size();
-  address addr = MetaspaceShared::i2i_entry_code_buffers((size_t)(AbstractInterpreter::number_of_method_entries) * trampoline_size);
+  address addr = MetaspaceShared::i2i_entry_code_buffers();
   addr += (size_t)(kind) * trampoline_size;
 
   return addr;
 }
 
-void AbstractInterpreter::update_cds_entry_table(AbstractInterpreter::MethodKind kind) {
-  if (DumpSharedSpaces || UseSharedSpaces) {
-    address trampoline = get_trampoline_code_buffer(kind);
-    _cds_entry_table[kind] = trampoline;
-
+void AbstractInterpreter::generate_entry_for_cds_method(AbstractInterpreter::MethodKind kind) {
+  if (UseSharedSpaces) {
+    address trampoline = entry_for_cds_method(kind);
     CodeBuffer buffer(trampoline, (int)(SharedRuntime::trampoline_size()));
     MacroAssembler _masm(&buffer);
     SharedRuntime::generate_trampoline(&_masm, _entry_table[kind]);
@@ -232,7 +240,7 @@ void AbstractInterpreter::set_entry_for_kind(AbstractInterpreter::MethodKind kin
   assert(_entry_table[kind] == _entry_table[abstract], "previous value must be AME entry");
   _entry_table[kind] = entry;
 
-  update_cds_entry_table(kind);
+  generate_entry_for_cds_method(kind);
 }
 
 // Return true if the interpreter can prove that the given bytecode has
@@ -468,6 +476,6 @@ void AbstractInterpreter::initialize_method_handle_entries() {
   for (int i = method_handle_invoke_FIRST; i <= method_handle_invoke_LAST; i++) {
     MethodKind kind = (MethodKind) i;
     _entry_table[kind] = _entry_table[Interpreter::abstract];
-    Interpreter::update_cds_entry_table(kind);
+    Interpreter::generate_entry_for_cds_method(kind);
   }
 }

src/hotspot/share/interpreter/abstractInterpreter.hpp

@@ -133,16 +133,10 @@ class AbstractInterpreter: AllStatic {
   static address    entry_for_kind(MethodKind k)                { assert(0 <= k && k < number_of_method_entries, "illegal kind"); return _entry_table[k]; }
   static address    entry_for_method(const methodHandle& m)     { return entry_for_kind(method_kind(m)); }
 
-  static address entry_for_cds_method(const methodHandle& m) {
-    MethodKind k = method_kind(m);
-    assert(0 <= k && k < number_of_method_entries, "illegal kind");
-    return _cds_entry_table[k];
-  }
-
   // used by class data sharing
-  static void       update_cds_entry_table(MethodKind kind) NOT_CDS_RETURN;
-
-  static address    get_trampoline_code_buffer(AbstractInterpreter::MethodKind kind) NOT_CDS_RETURN_(0);
+  static address    entry_for_cds_method(const methodHandle& m) NOT_CDS_RETURN_(NULL);
+  static address    entry_for_cds_method(AbstractInterpreter::MethodKind kind) NOT_CDS_RETURN_(NULL);
+  static void       generate_entry_for_cds_method(MethodKind kind) NOT_CDS_RETURN;
 
   // used for bootstrapping method handles:
   static void       set_entry_for_kind(MethodKind k, address e);

src/hotspot/share/interpreter/templateInterpreterGenerator.cpp

@@ -181,7 +181,7 @@ void TemplateInterpreterGenerator::generate_all() {
 #define method_entry(kind)                                              \
   { CodeletMark cm(_masm, "method entry point (kind = " #kind ")"); \
     Interpreter::_entry_table[Interpreter::kind] = generate_method_entry(Interpreter::kind); \
-    Interpreter::update_cds_entry_table(Interpreter::kind); \
+    Interpreter::generate_entry_for_cds_method(Interpreter::kind); \
   }
 
   // all non-native method kinds

src/hotspot/share/memory/archiveBuilder.cpp

@@ -281,8 +281,6 @@ void ArchiveBuilder::gather_klasses_and_symbols() {
     // DynamicArchiveBuilder::sort_methods()).
     sort_symbols_and_fix_hash();
     sort_klasses();
-    allocate_method_trampoline_info();
-    allocate_method_trampolines();
   }
 }
 

src/hotspot/share/memory/filemap.cpp

@@ -278,7 +278,6 @@ void FileMapHeader::print(outputStream* st) {
   st->print_cr("- cloned_vtables_offset:          " SIZE_FORMAT_HEX, _cloned_vtables_offset);
   st->print_cr("- serialized_data_offset:         " SIZE_FORMAT_HEX, _serialized_data_offset);
   st->print_cr("- i2i_entry_code_buffers_offset:  " SIZE_FORMAT_HEX, _i2i_entry_code_buffers_offset);
-  st->print_cr("- i2i_entry_code_buffers_size:    " SIZE_FORMAT, _i2i_entry_code_buffers_size);
   st->print_cr("- heap_end:                       " INTPTR_FORMAT, p2i(_heap_end));
   st->print_cr("- base_archive_is_default:        %d", _base_archive_is_default);
   st->print_cr("- jvm_ident:                      %s", _jvm_ident);

src/hotspot/share/memory/filemap.hpp

@@ -202,7 +202,6 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   size_t  _cloned_vtables_offset;   // The address of the first cloned vtable
   size_t  _serialized_data_offset;  // Data accessed using {ReadClosure,WriteClosure}::serialize()
   size_t  _i2i_entry_code_buffers_offset;
-  size_t  _i2i_entry_code_buffers_size;
   address _heap_end;                // heap end at dump time.
   bool _base_archive_is_default;    // indicates if the base archive is the system default one
 
@@ -270,7 +269,6 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   char* cloned_vtables()                   const { return from_mapped_offset(_cloned_vtables_offset); }
   char* serialized_data()                  const { return from_mapped_offset(_serialized_data_offset); }
   address i2i_entry_code_buffers()         const { return (address)from_mapped_offset(_i2i_entry_code_buffers_offset); }
-  size_t i2i_entry_code_buffers_size()     const { return _i2i_entry_code_buffers_size; }
   address heap_end()                       const { return _heap_end; }
   bool base_archive_is_default()           const { return _base_archive_is_default; }
   const char* jvm_ident()                  const { return _jvm_ident; }
@@ -297,10 +295,8 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   void set_ptrmap_size_in_bits(size_t s)         { _ptrmap_size_in_bits = s; }
   void set_mapped_base_address(char* p)          { _mapped_base_address = p; }
   void set_heap_obj_roots(narrowOop r)           { _heap_obj_roots = r; }
-
-  void set_i2i_entry_code_buffers(address p, size_t s) {
+  void set_i2i_entry_code_buffers(address p) {
     set_mapped_offset((char*)p, &_i2i_entry_code_buffers_offset);
-    _i2i_entry_code_buffers_size = s;
   }
 
   void set_shared_path_table(SharedPathTable table) {
@@ -417,9 +413,8 @@ class FileMapInfo : public CHeapObj<mtInternal> {
   void  align_file_position();
 
   address i2i_entry_code_buffers()            const { return header()->i2i_entry_code_buffers();  }
-  size_t i2i_entry_code_buffers_size()        const { return header()->i2i_entry_code_buffers_size(); }
-  void set_i2i_entry_code_buffers(address addr, size_t s) const {
-    header()->set_i2i_entry_code_buffers(addr, s);
+  void set_i2i_entry_code_buffers(address addr) const {
+    header()->set_i2i_entry_code_buffers(addr);
   }
 
   bool is_static()                            const { return _is_static; }

src/hotspot/share/memory/metaspaceShared.cpp

@@ -36,6 +36,7 @@
 #include "classfile/systemDictionary.hpp"
 #include "classfile/systemDictionaryShared.hpp"
 #include "code/codeCache.hpp"
+#include "interpreter/abstractInterpreter.hpp"
 #include "interpreter/bytecodeStream.hpp"
 #include "interpreter/bytecodes.hpp"
 #include "logging/log.hpp"
@@ -61,6 +62,7 @@
 #include "runtime/handles.inline.hpp"
 #include "runtime/os.hpp"
 #include "runtime/safepointVerifiers.hpp"
+#include "runtime/sharedRuntime.hpp"
 #include "runtime/timerTrace.hpp"
 #include "runtime/vmThread.hpp"
 #include "runtime/vmOperations.hpp"
@@ -82,7 +84,6 @@ bool MetaspaceShared::_has_error_classes;
 bool MetaspaceShared::_archive_loading_failed = false;
 bool MetaspaceShared::_remapped_readwrite = false;
 address MetaspaceShared::_i2i_entry_code_buffers = NULL;
-size_t MetaspaceShared::_i2i_entry_code_buffers_size = 0;
 void* MetaspaceShared::_shared_metaspace_static_top = NULL;
 intx MetaspaceShared::_relocation_delta;
 char* MetaspaceShared::_requested_base_address;
@@ -339,10 +340,6 @@ void MetaspaceShared::initialize_dumptime_shared_and_meta_spaces() {
   log_info(cds)("Allocated shared space: " SIZE_FORMAT " bytes at " PTR_FORMAT,
                 _shared_rs.size(), p2i(_shared_rs.base()));
 
-  // We don't want any valid object to be at the very bottom of the archive.
-  // See ArchivePtrMarker::mark_pointer().
-  MetaspaceShared::misc_code_space_alloc(16);
-
   size_t symbol_rs_size = LP64_ONLY(3 * G) NOT_LP64(128 * M);
   _symbol_rs = ReservedSpace(symbol_rs_size);
   if (!_symbol_rs.is_reserved()) {
@@ -510,19 +507,19 @@ void MetaspaceShared::serialize(SerializeClosure* soc) {
   soc->do_tag(666);
 }
 
-address MetaspaceShared::i2i_entry_code_buffers(size_t total_size) {
-  if (DumpSharedSpaces) {
-    if (_i2i_entry_code_buffers == NULL) {
-      _i2i_entry_code_buffers = (address)misc_code_space_alloc(total_size);
-      _i2i_entry_code_buffers_size = total_size;
-    }
-  } else if (UseSharedSpaces) {
-    assert(_i2i_entry_code_buffers != NULL, "must already been initialized");
-  } else {
-    return NULL;
-  }
+void MetaspaceShared::init_misc_code_space() {
+  // We don't want any valid object to be at the very bottom of the archive.
+  // See ArchivePtrMarker::mark_pointer().
+  MetaspaceShared::misc_code_space_alloc(16);
 
-  assert(_i2i_entry_code_buffers_size == total_size, "must not change");
+  size_t trampoline_size = SharedRuntime::trampoline_size();
+  size_t buf_size = (size_t)AbstractInterpreter::number_of_method_entries * trampoline_size;
+  _i2i_entry_code_buffers = (address)misc_code_space_alloc(buf_size);
+}
+
+address MetaspaceShared::i2i_entry_code_buffers() {
+  assert(DumpSharedSpaces || UseSharedSpaces, "must be");
+  assert(_i2i_entry_code_buffers != NULL, "must already been initialized");
   return _i2i_entry_code_buffers;
 }
 
@@ -733,6 +730,10 @@ void VM_PopulateDumpSharedSpace::doit() {
 
   builder.gather_source_objs();
 
+  MetaspaceShared::init_misc_code_space();
+  builder.allocate_method_trampoline_info();
+  builder.allocate_method_trampolines();
+
   char* cloned_vtables = CppVtables::dumptime_init();
 
   {
@@ -785,8 +786,6 @@ void VM_PopulateDumpSharedSpace::doit() {
   // The vtable clones contain addresses of the current process.
   // We don't want to write these addresses into the archive. Same for i2i buffer.
   CppVtables::zero_archived_vtables();
-  memset(MetaspaceShared::i2i_entry_code_buffers(), 0,
-         MetaspaceShared::i2i_entry_code_buffers_size());
 
   // relocate the data so that it can be mapped to MetaspaceShared::requested_base_address()
   // without runtime relocation.
@@ -798,8 +797,7 @@ void VM_PopulateDumpSharedSpace::doit() {
   mapinfo->populate_header(os::vm_allocation_granularity());
   mapinfo->set_serialized_data(serialized_data);
   mapinfo->set_cloned_vtables(cloned_vtables);
-  mapinfo->set_i2i_entry_code_buffers(MetaspaceShared::i2i_entry_code_buffers(),
-                                      MetaspaceShared::i2i_entry_code_buffers_size());
+  mapinfo->set_i2i_entry_code_buffers(MetaspaceShared::i2i_entry_code_buffers());
   mapinfo->open_for_write();
   size_t bitmap_size_in_bytes;
   char* bitmap = MetaspaceShared::write_core_archive_regions(mapinfo, _closed_archive_heap_oopmaps,
@@ -1720,7 +1718,6 @@ void MetaspaceShared::unmap_archive(FileMapInfo* mapinfo) {
 void MetaspaceShared::initialize_shared_spaces() {
   FileMapInfo *static_mapinfo = FileMapInfo::current_info();
   _i2i_entry_code_buffers = static_mapinfo->i2i_entry_code_buffers();
-  _i2i_entry_code_buffers_size = static_mapinfo->i2i_entry_code_buffers_size();
 
   // Verify various attributes of the archive, plus initialize the
   // shared string/symbol tables

src/hotspot/share/memory/metaspaceShared.hpp

@@ -76,7 +76,6 @@ class MetaspaceShared : AllStatic {
   static bool _archive_loading_failed;
   static bool _remapped_readwrite;
   static address _i2i_entry_code_buffers;
-  static size_t  _i2i_entry_code_buffers_size;
   static size_t  _core_spaces_size;
   static void* _shared_metaspace_static_top;
   static intx _relocation_delta;
@@ -237,16 +236,10 @@ class MetaspaceShared : AllStatic {
     return align_up(byte_size, SharedSpaceObjectAlignment);
   }
 
-  static address i2i_entry_code_buffers(size_t total_size);
+  static void init_misc_code_space();
+  static address i2i_entry_code_buffers();
 
-  static address i2i_entry_code_buffers() {
-    return _i2i_entry_code_buffers;
-  }
-  static size_t i2i_entry_code_buffers_size() {
-    return _i2i_entry_code_buffers_size;
-  }
   static void relocate_klass_ptr(oop o);
-
   static Klass* get_relocated_klass(Klass *k, bool is_final=false);
 
   static void initialize_ptr_marker(CHeapBitMap* ptrmap);