8256401: ZGC: Improve ZList verification

Reviewed-by: ayang, stefank

src/hotspot/share/gc/z/zList.hpp

@@ -35,18 +35,18 @@ class ZListNode {
   friend class ZList<T>;
 
 private:
-  ZListNode* _next;
-  ZListNode* _prev;
+  ZListNode<T>* _next;
+  ZListNode<T>* _prev;
 
-  ZListNode(ZListNode* next, ZListNode* prev);
+  NONCOPYABLE(ZListNode);
 
-  void set_unused();
+  void verify_links() const;
+  void verify_links_linked() const;
+  void verify_links_unlinked() const;
 
 public:
   ZListNode();
   ~ZListNode();
-
-  bool is_unused() const;
 };
 
 // Doubly linked list
@@ -58,7 +58,7 @@ class ZList {
 
   NONCOPYABLE(ZList);
 
-  void verify() const;
+  void verify_head() const;
 
   void insert(ZListNode<T>* before, ZListNode<T>* node);
 
@@ -84,8 +84,6 @@ class ZList {
   void remove(T* elem);
   T* remove_first();
   T* remove_last();
-
-  void transfer(ZList<T>* list);
 };
 
 template <typename T, bool Forward>

src/hotspot/share/gc/z/zList.inline.hpp

@@ -28,47 +28,54 @@
 #include "utilities/debug.hpp"
 
 template <typename T>
-inline ZListNode<T>::ZListNode(ZListNode* next, ZListNode* prev) :
-    _next(next),
-    _prev(prev) {}
+inline ZListNode<T>::ZListNode() :
+    _next(this),
+    _prev(this) {}
 
 template <typename T>
-inline void ZListNode<T>::set_unused() {
-  _next = NULL;
-  _prev = NULL;
+inline ZListNode<T>::~ZListNode() {
+  verify_links_unlinked();
 }
 
 template <typename T>
-inline ZListNode<T>::ZListNode() {
-  set_unused();
+inline void ZListNode<T>::verify_links() const {
+  assert(_next->_prev == this, "Corrupt list node");
+  assert(_prev->_next == this, "Corrupt list node");
 }
 
 template <typename T>
-inline ZListNode<T>::~ZListNode() {
-  set_unused();
+inline void ZListNode<T>::verify_links_linked() const {
+  assert(_next != this, "Should be in a list");
+  assert(_prev != this, "Should be in a list");
+  verify_links();
 }
 
 template <typename T>
-inline bool ZListNode<T>::is_unused() const {
-  return _next == NULL && _prev == NULL;
+inline void ZListNode<T>::verify_links_unlinked() const {
+  assert(_next == this, "Should not be in a list");
+  assert(_prev == this, "Should not be in a list");
 }
 
 template <typename T>
-inline void ZList<T>::verify() const {
-  assert(_head._next->_prev == &_head, "List corrupt");
-  assert(_head._prev->_next == &_head, "List corrupt");
+inline void ZList<T>::verify_head() const {
+  _head.verify_links();
 }
 
 template <typename T>
 inline void ZList<T>::insert(ZListNode<T>* before, ZListNode<T>* node) {
-  verify();
+  verify_head();
+
+  before->verify_links();
+  node->verify_links_unlinked();
 
-  assert(node->is_unused(), "Already in a list");
   node->_prev = before;
   node->_next = before->_next;
   before->_next = node;
   node->_next->_prev = node;
 
+  before->verify_links_linked();
+  node->verify_links_linked();
+
   _size++;
 }
 
@@ -84,20 +91,20 @@ inline T* ZList<T>::cast_to_outer(ZListNode<T>* node) const {
 
 template <typename T>
 inline ZList<T>::ZList() :
-    _head(&_head, &_head),
+    _head(),
     _size(0) {
-  verify();
+  verify_head();
 }
 
 template <typename T>
 inline size_t ZList<T>::size() const {
-  verify();
+  verify_head();
   return _size;
 }
 
 template <typename T>
 inline bool ZList<T>::is_empty() const {
-  return _size == 0;
+  return size() == 0;
 }
 
 template <typename T>
@@ -112,15 +119,27 @@ inline T* ZList<T>::last() const {
 
 template <typename T>
 inline T* ZList<T>::next(T* elem) const {
-  verify();
-  ZListNode<T>* next = cast_to_inner(elem)->_next;
+  verify_head();
+
+  ZListNode<T>* const node = cast_to_inner(elem);
+  node->verify_links_linked();
+
+  ZListNode<T>* const next = node->_next;
+  next->verify_links_linked();
+
   return (next == &_head) ? NULL : cast_to_outer(next);
 }
 
 template <typename T>
 inline T* ZList<T>::prev(T* elem) const {
-  verify();
-  ZListNode<T>* prev = cast_to_inner(elem)->_prev;
+  verify_head();
+
+  ZListNode<T>* const node = cast_to_inner(elem);
+  node->verify_links_linked();
+
+  ZListNode<T>* const prev = node->_prev;
+  prev->verify_links_linked();
+
   return (prev == &_head) ? NULL : cast_to_outer(prev);
 }
 
@@ -146,19 +165,24 @@ inline void ZList<T>::insert_after(T* after, T* elem) {
 
 template <typename T>
 inline void ZList<T>::remove(T* elem) {
-  verify();
+  verify_head();
 
   ZListNode<T>* const node = cast_to_inner(elem);
-  assert(!node->is_unused(), "Not in a list");
+  node->verify_links_linked();
 
   ZListNode<T>* const next = node->_next;
   ZListNode<T>* const prev = node->_prev;
-  assert(next->_prev == node, "List corrupt");
-  assert(prev->_next == node, "List corrupt");
+  next->verify_links_linked();
+  prev->verify_links_linked();
+
+  node->_next = prev->_next;
+  node->_prev = next->_prev;
+  node->verify_links_unlinked();
 
-  prev->_next = next;
   next->_prev = prev;
-  node->set_unused();
+  prev->_next = next;
+  next->verify_links();
+  prev->verify_links();
 
   _size--;
 }
@@ -183,28 +207,6 @@ inline T* ZList<T>::remove_last() {
   return elem;
 }
 
-template <typename T>
-inline void ZList<T>::transfer(ZList<T>* list) {
-  verify();
-
-  if (!list->is_empty()) {
-    list->_head._next->_prev = _head._prev;
-    list->_head._prev->_next = _head._prev->_next;
-
-    _head._prev->_next = list->_head._next;
-    _head._prev = list->_head._prev;
-
-    list->_head._next = &list->_head;
-    list->_head._prev = &list->_head;
-
-    _size += list->_size;
-    list->_size = 0;
-
-    list->verify();
-    verify();
-  }
-}
-
 template <typename T, bool Forward>
 inline ZListIteratorImpl<T, Forward>::ZListIteratorImpl(const ZList<T>* list) :
     _list(list),

src/hotspot/share/gc/z/zPage.cpp

@@ -40,7 +40,8 @@ ZPage::ZPage(uint8_t type, const ZVirtualMemory& vmem, const ZPhysicalMemory& pm
     _top(start()),
     _livemap(object_max_count()),
     _last_used(0),
-    _physical(pmem) {
+    _physical(pmem),
+    _node() {
   assert_initialized();
 }
 

test/hotspot/gtest/gc/z/test_zList.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -87,6 +87,13 @@ TEST_F(ZListTest, test_insert) {
 
   EXPECT_EQ(list.size(), 6u);
   assert_sorted(&list);
+
+  for (int i = 0; i < 6; i++) {
+    ZTestEntry* e = list.remove_first();
+    EXPECT_EQ(e->id(), i);
+  }
+
+  EXPECT_EQ(list.size(), 0u);
 }
 
 TEST_F(ZListTest, test_remove) {
@@ -145,79 +152,4 @@ TEST_F(ZListTest, test_remove) {
   }
 }
 
-TEST_F(ZListTest, test_transfer) {
-  // Transfer empty to empty
-  {
-    ZList<ZTestEntry> list0;
-    ZList<ZTestEntry> list1;
-
-    EXPECT_TRUE(list0.is_empty());
-    EXPECT_TRUE(list1.is_empty());
-
-    list0.transfer(&list1);
-
-    EXPECT_TRUE(list0.is_empty());
-    EXPECT_TRUE(list1.is_empty());
-  }
-
-  // Transfer non-empty to empty
-  {
-    ZList<ZTestEntry> list0;
-    ZList<ZTestEntry> list1;
-    ZTestEntry e0(0);
-    ZTestEntry e1(1);
-    ZTestEntry e2(2);
-    ZTestEntry e3(3);
-    ZTestEntry e4(4);
-    ZTestEntry e5(5);
-
-    list1.insert_last(&e0);
-    list1.insert_last(&e1);
-    list1.insert_last(&e2);
-    list1.insert_last(&e3);
-    list1.insert_last(&e4);
-    list1.insert_last(&e5);
-
-    EXPECT_EQ(list0.size(), 0u);
-    EXPECT_EQ(list1.size(), 6u);
-
-    list0.transfer(&list1);
-
-    EXPECT_EQ(list0.size(), 6u);
-    EXPECT_EQ(list1.size(), 0u);
-
-    assert_sorted(&list0);
-  }
-
-  // Transfer non-empty to non-empty
-  {
-    ZList<ZTestEntry> list0;
-    ZList<ZTestEntry> list1;
-    ZTestEntry e0(0);
-    ZTestEntry e1(1);
-    ZTestEntry e2(2);
-    ZTestEntry e3(3);
-    ZTestEntry e4(4);
-    ZTestEntry e5(5);
-
-    list0.insert_last(&e0);
-    list0.insert_last(&e1);
-    list0.insert_last(&e2);
-
-    list1.insert_last(&e3);
-    list1.insert_last(&e4);
-    list1.insert_last(&e5);
-
-    EXPECT_EQ(list0.size(), 3u);
-    EXPECT_EQ(list1.size(), 3u);
-
-    list0.transfer(&list1);
-
-    EXPECT_EQ(list0.size(), 6u);
-    EXPECT_EQ(list1.size(), 0u);
-
-    assert_sorted(&list0);
-  }
-}
-
 #endif // PRODUCT