8308342: Remove MetaspaceClosure::Ref::keep_after_pushing()

Reviewed-by: ccheung

Author: iklam

src/hotspot/share/cds/archiveBuilder.cpp

@@ -92,7 +92,7 @@ void ArchiveBuilder::SourceObjList::remember_embedded_pointer(SourceObjInfo* src
   // To mark the f->ptr pointer on 64-bit platform, this function is called with
   //    src_info()->obj() == 0x100
   //    ref->addr() == 0x108
-  address src_obj = src_info->obj();
+  address src_obj = src_info->source_addr();
   address* field_addr = ref->addr();
   assert(src_info->ptrmap_start() < _total_bytes, "sanity");
   assert(src_info->ptrmap_end() <= _total_bytes, "sanity");
@@ -176,8 +176,6 @@ ArchiveBuilder::~ArchiveBuilder() {
   assert(_current == this, "must be");
   _current = nullptr;
 
-  clean_up_src_obj_table();
-
   for (int i = 0; i < _symbols->length(); i++) {
     _symbols->at(i)->decrement_refcount();
   }
@@ -427,7 +425,6 @@ bool ArchiveBuilder::gather_one_source_obj(MetaspaceClosure::Ref* enclosing_ref,
   if (src_obj == nullptr) {
     return false;
   }
-  ref->set_keep_after_pushing();
   remember_embedded_pointer_in_gathered_obj(enclosing_ref, ref);
 
   FollowMode follow_mode = get_follow_mode(ref);
@@ -589,15 +586,14 @@ void ArchiveBuilder::make_shallow_copies(DumpRegion *dump_region,
 }
 
 void ArchiveBuilder::make_shallow_copy(DumpRegion *dump_region, SourceObjInfo* src_info) {
-  MetaspaceClosure::Ref* ref = src_info->ref();
-  address src = ref->obj();
+  address src = src_info->source_addr();
   int bytes = src_info->size_in_bytes();
   char* dest;
   char* oldtop;
   char* newtop;
 
   oldtop = dump_region->top();
-  if (ref->msotype() == MetaspaceObj::ClassType) {
+  if (src_info->msotype() == MetaspaceObj::ClassType) {
     // Save a pointer immediate in front of an InstanceKlass, so
     // we can do a quick lookup from InstanceKlass* -> RunTimeClassInfo*
     // without building another hashtable. See RunTimeClassInfo::get_for()
@@ -621,7 +617,7 @@ void ArchiveBuilder::make_shallow_copy(DumpRegion *dump_region, SourceObjInfo* s
     }
   }
 
-  intptr_t* archived_vtable = CppVtables::get_archived_vtable(ref->msotype(), (address)dest);
+  intptr_t* archived_vtable = CppVtables::get_archived_vtable(src_info->msotype(), (address)dest);
   if (archived_vtable != nullptr) {
     *(address*)dest = (address)archived_vtable;
     ArchivePtrMarker::mark_pointer((address*)dest);
@@ -630,7 +626,7 @@ void ArchiveBuilder::make_shallow_copy(DumpRegion *dump_region, SourceObjInfo* s
   log_trace(cds)("Copy: " PTR_FORMAT " ==> " PTR_FORMAT " %d", p2i(src), p2i(dest), bytes);
   src_info->set_buffered_addr((address)dest);
 
-  _alloc_stats.record(ref->msotype(), int(newtop - oldtop), src_info->read_only());
+  _alloc_stats.record(src_info->msotype(), int(newtop - oldtop), src_info->read_only());
 }
 
 // This is used by code that hand-assemble data structures, such as the LambdaProxyClassKey, that are
@@ -1097,11 +1093,6 @@ void ArchiveBuilder::print_stats() {
   _alloc_stats.print_stats(int(_ro_region.used()), int(_rw_region.used()));
 }
 
-void ArchiveBuilder::clean_up_src_obj_table() {
-  SrcObjTableCleaner cleaner;
-  _src_obj_table.iterate(&cleaner);
-}
-
 void ArchiveBuilder::write_archive(FileMapInfo* mapinfo, ArchiveHeapInfo* heap_info) {
   // Make sure NUM_CDS_REGIONS (exported in cds.h) agrees with
   // MetaspaceShared::n_regions (internal to hotspot).

src/hotspot/share/cds/archiveBuilder.hpp

@@ -123,20 +123,17 @@ class ArchiveBuilder : public StackObj {
 
 private:
   class SourceObjInfo {
-    MetaspaceClosure::Ref* _ref; // The object that's copied into the buffer
     uintx _ptrmap_start;     // The bit-offset of the start of this object (inclusive)
     uintx _ptrmap_end;       // The bit-offset of the end   of this object (exclusive)
     bool _read_only;
     FollowMode _follow_mode;
     int _size_in_bytes;
     MetaspaceObj::Type _msotype;
-    address _source_addr;    // The value of the source object (_ref->obj()) when this
-                             // SourceObjInfo was created. Note that _ref->obj() may change
-                             // later if _ref is relocated.
-    address _buffered_addr;  // The copy of _ref->obj() insider the buffer.
+    address _source_addr;    // The source object to be copied.
+    address _buffered_addr;  // The copy of this object insider the buffer.
   public:
     SourceObjInfo(MetaspaceClosure::Ref* ref, bool read_only, FollowMode follow_mode) :
-      _ref(ref), _ptrmap_start(0), _ptrmap_end(0), _read_only(read_only), _follow_mode(follow_mode),
+      _ptrmap_start(0), _ptrmap_end(0), _read_only(read_only), _follow_mode(follow_mode),
       _size_in_bytes(ref->size() * BytesPerWord), _msotype(ref->msotype()),
       _source_addr(ref->obj()) {
       if (follow_mode == point_to_it) {
@@ -147,7 +144,6 @@ class ArchiveBuilder : public StackObj {
     }
 
     bool should_copy() const { return _follow_mode == make_a_copy; }
-    MetaspaceClosure::Ref* ref() const { return  _ref; }
     void set_buffered_addr(address addr)  {
       assert(should_copy(), "must be");
       assert(_buffered_addr == nullptr, "cannot be copied twice");
@@ -161,11 +157,13 @@ class ArchiveBuilder : public StackObj {
     bool read_only()      const    { return _read_only;    }
     int size_in_bytes()   const    { return _size_in_bytes; }
     address source_addr() const    { return _source_addr; }
-    address buffered_addr() const  { return _buffered_addr; }
+    address buffered_addr() const  {
+      if (_follow_mode != set_to_null) {
+        assert(_buffered_addr != nullptr, "must be initialized");
+      }
+      return _buffered_addr;
+    }
     MetaspaceObj::Type msotype() const { return _msotype; }
-
-    // convenience accessor
-    address obj() const { return ref()->obj(); }
   };
 
   class SourceObjList {
@@ -187,14 +185,6 @@ class ArchiveBuilder : public StackObj {
     SourceObjInfo* at(int i) const { return objs()->at(i); }
   };
 
-  class SrcObjTableCleaner {
-  public:
-    bool do_entry(address key, const SourceObjInfo& value) {
-      delete value.ref();
-      return true;
-    }
-  };
-
   class CDSMapLogger;
 
   static const int INITIAL_TABLE_SIZE = 15889;

src/hotspot/share/memory/metaspaceClosure.cpp

@@ -35,9 +35,7 @@ void MetaspaceClosure::Ref::update(address new_loc) const {
 void MetaspaceClosure::push_impl(MetaspaceClosure::Ref* ref) {
   if (_nest_level < MAX_NEST_LEVEL) {
     do_push(ref);
-    if (!ref->keep_after_pushing()) {
-      delete ref;
-    }
+    delete ref;
   } else {
     do_pending_ref(ref);
     ref->set_next(_pending_refs);
@@ -80,9 +78,7 @@ void MetaspaceClosure::finish() {
     Ref* ref = _pending_refs;
     _pending_refs = _pending_refs->next();
     do_push(ref);
-    if (!ref->keep_after_pushing()) {
-      delete ref;
-    }
+    delete ref;
   }
 }
 

src/hotspot/share/memory/metaspaceClosure.hpp

@@ -106,14 +106,13 @@ class MetaspaceClosure {
   // [2] All Array<T> dimensions are statically declared.
   class Ref : public CHeapObj<mtMetaspace> {
     Writability _writability;
-    bool _keep_after_pushing;
     Ref* _next;
     void* _user_data;
     NONCOPYABLE(Ref);
 
   protected:
     virtual void** mpp() const = 0;
-    Ref(Writability w) : _writability(w), _keep_after_pushing(false), _next(nullptr), _user_data(nullptr) {}
+    Ref(Writability w) : _writability(w), _next(nullptr), _user_data(nullptr) {}
   public:
     virtual bool not_null() const = 0;
     virtual int size() const = 0;
@@ -134,8 +133,6 @@ class MetaspaceClosure {
     void update(address new_loc) const;
 
     Writability writability() const { return _writability; };
-    void set_keep_after_pushing()   { _keep_after_pushing = true; }
-    bool keep_after_pushing()       { return _keep_after_pushing; }
     void set_user_data(void* data)  { _user_data = data; }
     void* user_data()               { return _user_data; }
     void set_next(Ref* n)           { _next = n; }