8204097: Simplify OopStorage::AllocateList block entry access
authorkbarrett
Mon, 11 Jun 2018 22:35:07 -0400
changeset 50513 7f166e010af4
parent 50512 2a94ab65b026
child 50514 2a524e603529
child 56742 25d779d6bafc
8204097: Simplify OopStorage::AllocateList block entry access Summary: Removed obsolete support for blocks being in multiple lists. Reviewed-by: coleenp, tschatzl
src/hotspot/share/gc/shared/oopStorage.cpp
src/hotspot/share/gc/shared/oopStorage.hpp
src/hotspot/share/gc/shared/oopStorage.inline.hpp
test/hotspot/gtest/gc/shared/test_oopStorage.cpp
--- a/src/hotspot/share/gc/shared/oopStorage.cpp	Tue Jun 12 08:51:28 2018 +0800
+++ b/src/hotspot/share/gc/shared/oopStorage.cpp	Mon Jun 11 22:35:07 2018 -0400
@@ -52,9 +52,7 @@
   assert(_next == NULL, "deleting attached block");
 }
 
-OopStorage::AllocateList::AllocateList(const AllocateEntry& (*get_entry)(const Block& block)) :
-  _head(NULL), _tail(NULL), _get_entry(get_entry)
-{}
+OopStorage::AllocateList::AllocateList() : _head(NULL), _tail(NULL) {}
 
 OopStorage::AllocateList::~AllocateList() {
   // ~OopStorage() empties its lists before destroying them.
@@ -68,8 +66,8 @@
     assert(_tail == NULL, "invariant");
     _head = _tail = █
   } else {
-    _get_entry(block)._next = old;
-    _get_entry(*old)._prev = █
+    block.allocate_entry()._next = old;
+    old->allocate_entry()._prev = █
     _head = █
   }
 }
@@ -80,14 +78,14 @@
     assert(_head == NULL, "invariant");
     _head = _tail = █
   } else {
-    _get_entry(*old)._next = █
-    _get_entry(block)._prev = old;
+    old->allocate_entry()._next = █
+    block.allocate_entry()._prev = old;
     _tail = █
   }
 }
 
 void OopStorage::AllocateList::unlink(const Block& block) {
-  const AllocateEntry& block_entry = _get_entry(block);
+  const AllocateEntry& block_entry = block.allocate_entry();
   const Block* prev_blk = block_entry._prev;
   const Block* next_blk = block_entry._next;
   block_entry._prev = NULL;
@@ -98,15 +96,15 @@
     _head = _tail = NULL;
   } else if (prev_blk == NULL) {
     assert(_head == &block, "invariant");
-    _get_entry(*next_blk)._prev = NULL;
+    next_blk->allocate_entry()._prev = NULL;
     _head = next_blk;
   } else if (next_blk == NULL) {
     assert(_tail == &block, "invariant");
-    _get_entry(*prev_blk)._next = NULL;
+    prev_blk->allocate_entry()._next = NULL;
     _tail = prev_blk;
   } else {
-    _get_entry(*next_blk)._prev = prev_blk;
-    _get_entry(*prev_blk)._next = next_blk;
+    next_blk->allocate_entry()._prev = prev_blk;
+    prev_blk->allocate_entry()._next = next_blk;
   }
 }
 
@@ -232,10 +230,6 @@
   const_cast<OopStorage* volatile&>(_owner) = NULL;
 }
 
-const OopStorage::AllocateEntry& OopStorage::Block::get_allocate_entry(const Block& block) {
-  return block._allocate_entry;
-}
-
 size_t OopStorage::Block::allocation_size() {
   // _data must be first member, so aligning Block aligns _data.
   STATIC_ASSERT(_data_pos == 0);
@@ -769,7 +763,7 @@
                        Mutex* active_mutex) :
   _name(dup_name(name)),
   _active_array(ActiveArray::create(initial_active_array_size)),
-  _allocate_list(&Block::get_allocate_entry),
+  _allocate_list(),
   _deferred_updates(NULL),
   _allocate_mutex(allocate_mutex),
   _active_mutex(active_mutex),
--- a/src/hotspot/share/gc/shared/oopStorage.hpp	Tue Jun 12 08:51:28 2018 +0800
+++ b/src/hotspot/share/gc/shared/oopStorage.hpp	Mon Jun 11 22:35:07 2018 -0400
@@ -178,14 +178,13 @@
   class AllocateList {
     const Block* _head;
     const Block* _tail;
-    const AllocateEntry& (*_get_entry)(const Block& block);
 
     // Noncopyable.
     AllocateList(const AllocateList&);
     AllocateList& operator=(const AllocateList&);
 
   public:
-    AllocateList(const AllocateEntry& (*get_entry)(const Block& block));
+    AllocateList();
     ~AllocateList();
 
     Block* head();
--- a/src/hotspot/share/gc/shared/oopStorage.inline.hpp	Tue Jun 12 08:51:28 2018 +0800
+++ b/src/hotspot/share/gc/shared/oopStorage.inline.hpp	Mon Jun 11 22:35:07 2018 -0400
@@ -158,7 +158,7 @@
   Block& operator=(const Block&);
 
 public:
-  static const AllocateEntry& get_allocate_entry(const Block& block);
+  const AllocateEntry& allocate_entry() const;
 
   static size_t allocation_size();
   static size_t allocation_alignment_shift();
@@ -214,19 +214,19 @@
 }
 
 inline OopStorage::Block* OopStorage::AllocateList::prev(Block& block) {
-  return const_cast<Block*>(_get_entry(block)._prev);
+  return const_cast<Block*>(block.allocate_entry()._prev);
 }
 
 inline OopStorage::Block* OopStorage::AllocateList::next(Block& block) {
-  return const_cast<Block*>(_get_entry(block)._next);
+  return const_cast<Block*>(block.allocate_entry()._next);
 }
 
 inline const OopStorage::Block* OopStorage::AllocateList::prev(const Block& block) const {
-  return _get_entry(block)._prev;
+  return block.allocate_entry()._prev;
 }
 
 inline const OopStorage::Block* OopStorage::AllocateList::next(const Block& block) const {
-  return _get_entry(block)._next;
+  return block.allocate_entry()._next;
 }
 
 template<typename Closure>
@@ -296,7 +296,11 @@
   return SkipNullFn<F>(f);
 }
 
-// Inline Block accesses for use in iteration inner loop.
+// Inline Block accesses for use in iteration loops.
+
+inline const OopStorage::AllocateEntry& OopStorage::Block::allocate_entry() const {
+  return _allocate_entry;
+}
 
 inline void OopStorage::Block::check_index(unsigned index) const {
   assert(index < ARRAY_SIZE(_data), "Index out of bounds: %u", index);
--- a/test/hotspot/gtest/gc/shared/test_oopStorage.cpp	Tue Jun 12 08:51:28 2018 +0800
+++ b/test/hotspot/gtest/gc/shared/test_oopStorage.cpp	Mon Jun 11 22:35:07 2018 -0400
@@ -1203,7 +1203,7 @@
 class OopStorageAllocateListTest : public OopStorageBlockCollectionTest {};
 
 TEST_F(OopStorageAllocateListTest, empty_list) {
-  AllocateList list(&OopBlock::get_allocate_entry);
+  AllocateList list;
 
   EXPECT_TRUE(is_list_empty(list));
   EXPECT_EQ(NULL_BLOCK, list.head());
@@ -1212,7 +1212,7 @@
 }
 
 TEST_F(OopStorageAllocateListTest, push_back) {
-  AllocateList list(&OopBlock::get_allocate_entry);
+  AllocateList list;
 
   for (size_t i = 0; i < nvalues; ++i) {
     list.push_back(*values[i]);
@@ -1242,7 +1242,7 @@
 }
 
 TEST_F(OopStorageAllocateListTest, push_front) {
-  AllocateList list(&OopBlock::get_allocate_entry);
+  AllocateList list;
 
   for (size_t i = 0; i < nvalues; ++i) {
     list.push_front(*values[i]);
@@ -1273,7 +1273,7 @@
 
 class OopStorageAllocateListTestWithList : public OopStorageAllocateListTest {
 public:
-  OopStorageAllocateListTestWithList() : list(&OopBlock::get_allocate_entry) {
+  OopStorageAllocateListTestWithList() : list() {
     for (size_t i = 0; i < nvalues; ++i) {
       list.push_back(*values[i]);
     }
@@ -1345,7 +1345,7 @@
 }
 
 TEST_F(OopStorageAllocateListTest, single) {
-  AllocateList list(&OopBlock::get_allocate_entry);
+  AllocateList list;
 
   list.push_back(*values[0]);
   EXPECT_EQ(NULL_BLOCK, list.next(*values[0]));