8308603: Removing do_pending_ref/enclosing_ref from MetaspaceClosure

Reviewed-by: ccheung

src/hotspot/share/cds/archiveBuilder.cpp

@@ -68,7 +68,7 @@ ArchiveBuilder::SourceObjList::~SourceObjList() {
   delete _objs;
 }
 
-void ArchiveBuilder::SourceObjList::append(MetaspaceClosure::Ref* enclosing_ref, SourceObjInfo* src_info) {
+void ArchiveBuilder::SourceObjList::append(SourceObjInfo* src_info) {
   // Save this source object for copying
   _objs->append(src_info);
 
@@ -86,12 +86,7 @@ void ArchiveBuilder::SourceObjList::append(MetaspaceClosure::Ref* enclosing_ref,
 
 void ArchiveBuilder::SourceObjList::remember_embedded_pointer(SourceObjInfo* src_info, MetaspaceClosure::Ref* ref) {
   // src_obj contains a pointer. Remember the location of this pointer in _ptrmap,
-  // so that we can copy/relocate it later. E.g., if we have
-  //    class Foo { intx scala; Bar* ptr; }
-  //    Foo *f = 0x100;
-  // To mark the f->ptr pointer on 64-bit platform, this function is called with
-  //    src_info()->obj() == 0x100
-  //    ref->addr() == 0x108
+  // so that we can copy/relocate it later.
   address src_obj = src_info->source_addr();
   address* field_addr = ref->addr();
   assert(src_info->ptrmap_start() < _total_bytes, "sanity");
@@ -409,23 +404,16 @@ class GatherSortedSourceObjs : public MetaspaceClosure {
   GatherSortedSourceObjs(ArchiveBuilder* builder) : _builder(builder) {}
 
   virtual bool do_ref(Ref* ref, bool read_only) {
-    return _builder->gather_one_source_obj(enclosing_ref(), ref, read_only);
-  }
-
-  virtual void do_pending_ref(Ref* ref) {
-    if (ref->obj() != nullptr) {
-      _builder->remember_embedded_pointer_in_gathered_obj(enclosing_ref(), ref);
-    }
+    return _builder->gather_one_source_obj(ref, read_only);
   }
 };
 
-bool ArchiveBuilder::gather_one_source_obj(MetaspaceClosure::Ref* enclosing_ref,
-                                           MetaspaceClosure::Ref* ref, bool read_only) {
+bool ArchiveBuilder::gather_one_source_obj(MetaspaceClosure::Ref* ref, bool read_only) {
   address src_obj = ref->obj();
   if (src_obj == nullptr) {
     return false;
   }
-  remember_embedded_pointer_in_gathered_obj(enclosing_ref, ref);
+  remember_embedded_pointer_in_enclosing_obj(ref);
 
   FollowMode follow_mode = get_follow_mode(ref);
   SourceObjInfo src_info(ref, read_only, follow_mode);
@@ -440,33 +428,53 @@ bool ArchiveBuilder::gather_one_source_obj(MetaspaceClosure::Ref* enclosing_ref,
   assert(p->read_only() == src_info.read_only(), "must be");
 
   if (created && src_info.should_copy()) {
-    ref->set_user_data((void*)p);
     if (read_only) {
-      _ro_src_objs.append(enclosing_ref, p);
+      _ro_src_objs.append(p);
     } else {
-      _rw_src_objs.append(enclosing_ref, p);
+      _rw_src_objs.append(p);
     }
     return true; // Need to recurse into this ref only if we are copying it
   } else {
     return false;
   }
 }
 
-void ArchiveBuilder::remember_embedded_pointer_in_gathered_obj(MetaspaceClosure::Ref* enclosing_ref,
-                                                               MetaspaceClosure::Ref* ref) {
+// Remember that we have a pointer inside ref->enclosing_obj() that points to ref->obj()
+void ArchiveBuilder::remember_embedded_pointer_in_enclosing_obj(MetaspaceClosure::Ref* ref) {
   assert(ref->obj() != nullptr, "should have checked");
 
-  if (enclosing_ref != nullptr) {
-    SourceObjInfo* src_info = (SourceObjInfo*)enclosing_ref->user_data();
-    if (src_info == nullptr) {
-      // source objects of point_to_it/set_to_null types are not copied
-      // so we don't need to remember their pointers.
+  address enclosing_obj = ref->enclosing_obj();
+  if (enclosing_obj == nullptr) {
+    return;
+  }
+
+  // We are dealing with 3 addresses:
+  // address o    = ref->obj(): We have found an object whose address is o.
+  // address* mpp = ref->mpp(): The object o is pointed to by a pointer whose address is mpp.
+  //                            I.e., (*mpp == o)
+  // enclosing_obj            : If non-null, it is the object which has a field that points to o.
+  //                            mpp is the address if that field.
+  //
+  // Example: We have an array whose first element points to a Method:
+  //     Method* o                     = 0x0000abcd;
+  //     Array<Method*>* enclosing_obj = 0x00001000;
+  //     enclosing_obj->at_put(0, o);
+  //
+  // We the MetaspaceClosure iterates on the very first element of this array, we have
+  //     ref->obj()           == 0x0000abcd   (the Method)
+  //     ref->mpp()           == 0x00001008   (the location of the first element in the array)
+  //     ref->enclosing_obj() == 0x00001000   (the Array that contains the Method)
+  //
+  // We use the above information to mark the bitmap to indicate that there's a pointer on address 0x00001008.
+  SourceObjInfo* src_info = _src_obj_table.get(enclosing_obj);
+  if (src_info == nullptr || !src_info->should_copy()) {
+    // source objects of point_to_it/set_to_null types are not copied
+    // so we don't need to remember their pointers.
+  } else {
+    if (src_info->read_only()) {
+      _ro_src_objs.remember_embedded_pointer(src_info, ref);
     } else {
-      if (src_info->read_only()) {
-        _ro_src_objs.remember_embedded_pointer(src_info, ref);
-      } else {
-        _rw_src_objs.remember_embedded_pointer(src_info, ref);
-      }
+      _rw_src_objs.remember_embedded_pointer(src_info, ref);
     }
   }
 }

src/hotspot/share/cds/archiveBuilder.hpp

@@ -177,7 +177,7 @@ class ArchiveBuilder : public StackObj {
 
     GrowableArray<SourceObjInfo*>* objs() const { return _objs; }
 
-    void append(MetaspaceClosure::Ref* enclosing_ref, SourceObjInfo* src_info);
+    void append(SourceObjInfo* src_info);
     void remember_embedded_pointer(SourceObjInfo* pointing_obj, MetaspaceClosure::Ref* ref);
     void relocate(int i, ArchiveBuilder* builder);
 
@@ -330,8 +330,8 @@ class ArchiveBuilder : public StackObj {
   void gather_klasses_and_symbols();
   void gather_source_objs();
   bool gather_klass_and_symbol(MetaspaceClosure::Ref* ref, bool read_only);
-  bool gather_one_source_obj(MetaspaceClosure::Ref* enclosing_ref, MetaspaceClosure::Ref* ref, bool read_only);
-  void remember_embedded_pointer_in_gathered_obj(MetaspaceClosure::Ref* enclosing_ref, MetaspaceClosure::Ref* ref);
+  bool gather_one_source_obj(MetaspaceClosure::Ref* ref, bool read_only);
+  void remember_embedded_pointer_in_enclosing_obj(MetaspaceClosure::Ref* ref);
 
   DumpRegion* rw_region() { return &_rw_region; }
   DumpRegion* ro_region() { return &_ro_region; }

src/hotspot/share/memory/metaspaceClosure.cpp

@@ -25,19 +25,17 @@
 #include "precompiled.hpp"
 #include "memory/metaspaceClosure.hpp"
 
-// Update the reference to point to new_loc.
-void MetaspaceClosure::Ref::update(address new_loc) const {
-  log_trace(cds)("Ref: [" PTR_FORMAT "] -> " PTR_FORMAT " => " PTR_FORMAT,
-                 p2i(mpp()), p2i(obj()), p2i(new_loc));
-  *addr() = new_loc;
-}
-
 void MetaspaceClosure::push_impl(MetaspaceClosure::Ref* ref) {
+  if (_enclosing_ref != nullptr) {
+    assert(_nest_level > 0, "sanity");
+    ref->set_enclosing_obj(_enclosing_ref->obj());
+  } else {
+    assert(_nest_level == 0, "sanity");
+  }
   if (_nest_level < MAX_NEST_LEVEL) {
     do_push(ref);
     delete ref;
   } else {
-    do_pending_ref(ref);
     ref->set_next(_pending_refs);
     _pending_refs = ref;
   }

src/hotspot/share/memory/metaspaceClosure.hpp

@@ -106,13 +106,13 @@ class MetaspaceClosure {
   // [2] All Array<T> dimensions are statically declared.
   class Ref : public CHeapObj<mtMetaspace> {
     Writability _writability;
+    address _enclosing_obj;
     Ref* _next;
-    void* _user_data;
     NONCOPYABLE(Ref);
 
   protected:
     virtual void** mpp() const = 0;
-    Ref(Writability w) : _writability(w), _next(nullptr), _user_data(nullptr) {}
+    Ref(Writability w) : _writability(w), _enclosing_obj(nullptr), _next(nullptr) {}
   public:
     virtual bool not_null() const = 0;
     virtual int size() const = 0;
@@ -130,11 +130,15 @@ class MetaspaceClosure {
       return (address*)mpp();
     }
 
-    void update(address new_loc) const;
+    // See comments in ArchiveBuilder::remember_embedded_pointer_in_enclosing_obj()
+    address enclosing_obj() const {
+      return _enclosing_obj;
+    }
+    void set_enclosing_obj(address obj) {
+      _enclosing_obj = obj;
+    }
 
     Writability writability() const { return _writability; };
-    void set_user_data(void* data)  { _user_data = data; }
-    void* user_data()               { return _user_data; }
     void set_next(Ref* n)           { _next = n; }
     Ref* next() const               { return _next; }
   };
@@ -251,6 +255,9 @@ class MetaspaceClosure {
   // Normally, chains of references like a->b->c->d are iterated recursively. However,
   // if recursion is too deep, we save the Refs in _pending_refs, and push them later in
   // MetaspaceClosure::finish(). This avoids overflowing the C stack.
+  //
+  // When we are visting d, the _enclosing_ref is c,
+  // When we are visting c, the _enclosing_ref is b, ... and so on.
   static const int MAX_NEST_LEVEL = 5;
   Ref* _pending_refs;
   int _nest_level;
@@ -265,32 +272,14 @@ class MetaspaceClosure {
 
   void finish();
 
-  // enclosing_ref() is used to compute the offset of a field in a C++ class. For example
-  // class Foo { intx scala; Bar* ptr; }
-  //    Foo *f = 0x100;
-  // when the f->ptr field is iterated with do_ref() on 64-bit platforms, we will have
-  //    do_ref(Ref* r) {
-  //       r->addr() == 0x108;                // == &f->ptr;
-  //       enclosing_ref()->obj() == 0x100;   // == foo
-  // So we know that we are iterating upon a field at offset 8 of the object at 0x100.
-  //
-  // Note that if we have stack overflow, do_pending_ref(r) will be called first and
-  // do_ref(r) will be called later, for the same r. In this case, enclosing_ref() is valid only
-  // when do_pending_ref(r) is called, and will return null when do_ref(r) is called.
-  Ref* enclosing_ref() const {
-    return _enclosing_ref;
-  }
-
-  // This is called when a reference is placed in _pending_refs. Override this
-  // function if you're using enclosing_ref(). See notes above.
-  virtual void do_pending_ref(Ref* ref) {}
-
   // returns true if we want to keep iterating the pointers embedded inside <ref>
   virtual bool do_ref(Ref* ref, bool read_only) = 0;
 
 private:
   template <class REF_TYPE, typename T>
   void push_with_ref(T** mpp, Writability w) {
+    // We cannot make stack allocation because the Ref may need to be saved in
+    // _pending_refs to avoid overflowing the C call stack
     push_impl(new REF_TYPE(mpp, w));
   }
 
@@ -308,8 +297,8 @@ class MetaspaceClosure {
   // Note that the following will fail to compile (to prevent you from adding new fields
   // into the MetaspaceObj subtypes that cannot be properly copied by CDS):
   //
-  // Hashtable*             h  = ...;  it->push(&h);     => Hashtable is not a subclass of MetaspaceObj
-  // Array<Hashtable*>*     a6 = ...;  it->push(&a6);    => Hashtable is not a subclass of MetaspaceObj
+  // MemoryPool*            p  = ...;  it->push(&p);     => MemoryPool is not a subclass of MetaspaceObj
+  // Array<MemoryPool*>*    a6 = ...;  it->push(&a6);    => MemoryPool is not a subclass of MetaspaceObj
   // Array<int*>*           a7 = ...;  it->push(&a7);    => int       is not a subclass of MetaspaceObj
 
   template <typename T>
@@ -333,22 +322,6 @@ class MetaspaceClosure {
     static_assert(std::is_base_of<MetaspaceObj, T>::value, "Do not push Arrays of arbitrary pointer types");
     push_with_ref<MSOPointerArrayRef<T>>(mpp, w);
   }
-
-#if 0
-  // Enable this block if you're changing the push(...) methods, to test for types that should be
-  // disallowed. Each of the following "push" calls should result in a compile-time error.
-  void test_disallowed_types(MetaspaceClosure* it) {
-    Hashtable<bool, mtInternal>* h  = nullptr;
-    it->push(&h);
-
-    Array<Hashtable<bool, mtInternal>*>* a6 = nullptr;
-    it->push(&a6);
-
-    Array<int*>* a7 = nullptr;
-    it->push(&a7);
-  }
-#endif
-
 };
 
 // This is a special MetaspaceClosure that visits each unique MetaspaceObj once.