From 5ccc9629424c802a5c676553776ee5d2fb2ca3e3 Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Sun, 21 May 2023 05:32:36 +0000 Subject: [PATCH] 8308342: Remove MetaspaceClosure::Ref::keep_after_pushing() Reviewed-by: ccheung --- src/hotspot/share/cds/archiveBuilder.cpp | 19 ++++--------- src/hotspot/share/cds/archiveBuilder.hpp | 28 ++++++------------- src/hotspot/share/memory/metaspaceClosure.cpp | 8 ++---- src/hotspot/share/memory/metaspaceClosure.hpp | 5 +--- 4 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index f8a6bc293b5..263efbeec6b 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/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). diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index ca174e6fea8..1440ea577ec 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -123,20 +123,17 @@ public: 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 @@ private: } 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 @@ private: 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 @@ private: 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; diff --git a/src/hotspot/share/memory/metaspaceClosure.cpp b/src/hotspot/share/memory/metaspaceClosure.cpp index ef9759a238d..dc40f56f1d7 100644 --- a/src/hotspot/share/memory/metaspaceClosure.cpp +++ b/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; } } diff --git a/src/hotspot/share/memory/metaspaceClosure.hpp b/src/hotspot/share/memory/metaspaceClosure.hpp index 84106f9fb20..bd99720219a 100644 --- a/src/hotspot/share/memory/metaspaceClosure.hpp +++ b/src/hotspot/share/memory/metaspaceClosure.hpp @@ -106,14 +106,13 @@ public: // [2] All Array dimensions are statically declared. class Ref : public CHeapObj { 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 @@ public: 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; }