8137756: Remove hrs_err_msg and hrs_ext_msg from heapRegionSet
authordavid
Mon, 02 Nov 2015 14:28:19 +0100
changeset 33753 3add06d0880f
parent 33752 0782a966d781
child 33754 49614940dafc
8137756: Remove hrs_err_msg and hrs_ext_msg from heapRegionSet Reviewed-by: pliden, mgerdin
hotspot/src/share/vm/gc/g1/heapRegionSet.cpp
hotspot/src/share/vm/gc/g1/heapRegionSet.hpp
hotspot/src/share/vm/gc/g1/heapRegionSet.inline.hpp
--- a/hotspot/src/share/vm/gc/g1/heapRegionSet.cpp	Mon Nov 02 10:41:39 2015 +0100
+++ b/hotspot/src/share/vm/gc/g1/heapRegionSet.cpp	Mon Nov 02 14:28:19 2015 +0100
@@ -29,12 +29,6 @@
 
 uint FreeRegionList::_unrealistically_long_length = 0;
 
-void HeapRegionSetBase::fill_in_ext_msg(hrs_ext_msg* msg, const char* message) {
-  msg->append("[%s] %s ln: %u cy: " SIZE_FORMAT,
-              name(), message, length(), total_capacity_bytes());
-  fill_in_ext_msg_extra(msg);
-}
-
 #ifndef PRODUCT
 void HeapRegionSetBase::verify_region(HeapRegion* hr) {
   assert(hr->containing_set() == this, "Inconsistent containing set for %u", hr->hrm_index());
@@ -55,16 +49,15 @@
   // verification might fail and send us on a wild goose chase.
   check_mt_safety();
 
-  guarantee(( is_empty() && length() == 0 && total_capacity_bytes() == 0) ||
-            (!is_empty() && length() > 0  && total_capacity_bytes() > 0) ,
-            "%s", hrs_ext_msg(this, "invariant").buffer());
+  guarantee_heap_region_set(( is_empty() && length() == 0 && total_capacity_bytes() == 0) ||
+                            (!is_empty() && length() > 0  && total_capacity_bytes() > 0) ,
+                            "invariant");
 }
 
 void HeapRegionSetBase::verify_start() {
   // See comment in verify() about MT safety and verification.
   check_mt_safety();
-  assert(!_verify_in_progress,
-         "%s", hrs_ext_msg(this, "verification should not be in progress").buffer());
+  assert_heap_region_set(!_verify_in_progress, "verification should not be in progress");
 
   // Do the basic verification first before we do the checks over the regions.
   HeapRegionSetBase::verify();
@@ -75,8 +68,7 @@
 void HeapRegionSetBase::verify_end() {
   // See comment in verify() about MT safety and verification.
   check_mt_safety();
-  assert(_verify_in_progress,
-         "%s", hrs_ext_msg(this, "verification should be in progress").buffer());
+  assert_heap_region_set(_verify_in_progress, "verification should be in progress");
 
   _verify_in_progress = false;
 }
@@ -104,10 +96,6 @@
   _unrealistically_long_length = len;
 }
 
-void FreeRegionList::fill_in_ext_msg_extra(hrs_ext_msg* msg) {
-  msg->append(" hd: " PTR_FORMAT " tl: " PTR_FORMAT, p2i(_head), p2i(_tail));
-}
-
 void FreeRegionList::remove_all() {
   check_mt_safety();
   verify_optional();
@@ -151,7 +139,7 @@
   #endif // ASSERT
 
   if (is_empty()) {
-    assert(length() == 0 && _tail == NULL, "%s", hrs_ext_msg(this, "invariant").buffer());
+    assert_free_region_list(length() == 0 && _tail == NULL, "invariant");
     _head = from_list->_head;
     _tail = from_list->_tail;
   } else {
@@ -198,8 +186,8 @@
 
 void FreeRegionList::remove_starting_at(HeapRegion* first, uint num_regions) {
   check_mt_safety();
-  assert(num_regions >= 1, "%s", hrs_ext_msg(this, "pre-condition").buffer());
-  assert(!is_empty(), "%s", hrs_ext_msg(this, "pre-condition").buffer());
+  assert_free_region_list(num_regions >= 1, "pre-condition");
+  assert_free_region_list(!is_empty(), "pre-condition");
 
   verify_optional();
   DEBUG_ONLY(uint old_length = length();)
@@ -212,22 +200,22 @@
     HeapRegion* prev = curr->prev();
 
     assert(count < num_regions,
-           "%s", hrs_err_msg("[%s] should not come across more regions "
-                             "pending for removal than num_regions: %u",
-                             name(), num_regions).buffer());
+           "[%s] should not come across more regions "
+           "pending for removal than num_regions: %u",
+           name(), num_regions);
 
     if (prev == NULL) {
-      assert(_head == curr, "%s", hrs_ext_msg(this, "invariant").buffer());
+      assert_free_region_list(_head == curr, "invariant");
       _head = next;
     } else {
-      assert(_head != curr, "%s", hrs_ext_msg(this, "invariant").buffer());
+      assert_free_region_list(_head != curr, "invariant");
       prev->set_next(next);
     }
     if (next == NULL) {
-      assert(_tail == curr, "%s", hrs_ext_msg(this, "invariant").buffer());
+      assert_free_region_list(_tail == curr, "invariant");
       _tail = prev;
     } else {
-      assert(_tail != curr, "%s", hrs_ext_msg(this, "invariant").buffer());
+      assert_free_region_list(_tail != curr, "invariant");
       next->set_prev(prev);
     }
     if (_last == curr) {
@@ -243,12 +231,12 @@
   }
 
   assert(count == num_regions,
-         "%s", hrs_err_msg("[%s] count: %u should be == num_regions: %u",
-                           name(), count, num_regions).buffer());
+         "[%s] count: %u should be == num_regions: %u",
+         name(), count, num_regions);
   assert(length() + num_regions == old_length,
-         "%s", hrs_err_msg("[%s] new length should be consistent "
-                           "new length: %u old length: %u num_regions: %u",
-                           name(), length(), old_length, num_regions).buffer());
+         "[%s] new length should be consistent "
+         "new length: %u old length: %u num_regions: %u",
+         name(), length(), old_length, num_regions);
 
   verify_optional();
 }
@@ -305,8 +293,8 @@
 
     count++;
     guarantee(count < _unrealistically_long_length,
-        "%s", hrs_err_msg("[%s] the calculated length: %u seems very long, is there maybe a cycle? curr: " PTR_FORMAT " prev0: " PTR_FORMAT " " "prev1: " PTR_FORMAT " length: %u",
-              name(), count, p2i(curr), p2i(prev0), p2i(prev1), length()).buffer());
+              "[%s] the calculated length: %u seems very long, is there maybe a cycle? curr: " PTR_FORMAT " prev0: " PTR_FORMAT " " "prev1: " PTR_FORMAT " length: %u",
+              name(), count, p2i(curr), p2i(prev0), p2i(prev1), length());
 
     if (curr->next() != NULL) {
       guarantee(curr->next()->prev() == curr, "Next or prev pointers messed up");
--- a/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp	Mon Nov 02 10:41:39 2015 +0100
+++ b/hotspot/src/share/vm/gc/g1/heapRegionSet.hpp	Mon Nov 02 14:28:19 2015 +0100
@@ -27,9 +27,24 @@
 
 #include "gc/g1/heapRegion.hpp"
 
-// Large buffer for some cases where the output might be larger than normal.
-#define HRS_ERR_MSG_BUFSZ 512
-typedef FormatBuffer<HRS_ERR_MSG_BUFSZ> hrs_err_msg;
+#define assert_heap_region_set(p, message)                        \
+  do {                                                            \
+    assert((p), "[%s] %s ln: %u cy: " SIZE_FORMAT,                \
+           name(), message, length(), total_capacity_bytes());    \
+  } while (0)
+
+#define guarantee_heap_region_set(p, message)                     \
+  do {                                                            \
+    guarantee((p), "[%s] %s ln: %u cy: " SIZE_FORMAT,             \
+              name(), message, length(), total_capacity_bytes()); \
+  } while (0)
+
+#define assert_free_region_list(p, message)                                              \
+  do {                                                                                   \
+    assert((p), "[%s] %s ln: %u cy: " SIZE_FORMAT " hd: " PTR_FORMAT " tl: " PTR_FORMAT, \
+           name(), message, length(), total_capacity_bytes(), p2i(_head), p2i(_tail));   \
+  } while (0)
+
 
 // Set verification will be forced either if someone defines
 // HEAP_REGION_SET_FORCE_VERIFY to be 1, or in builds in which
@@ -38,8 +53,6 @@
 #define HEAP_REGION_SET_FORCE_VERIFY defined(ASSERT)
 #endif // HEAP_REGION_SET_FORCE_VERIFY
 
-class hrs_ext_msg;
-
 class HRSMtSafeChecker : public CHeapObj<mtGC> {
 public:
   virtual void check() = 0;
@@ -112,8 +125,6 @@
     }
   }
 
-  virtual void fill_in_ext_msg_extra(hrs_ext_msg* msg) { }
-
   HeapRegionSetBase(const char* name, bool humongous, bool free, HRSMtSafeChecker* mt_safety_checker);
 
 public:
@@ -135,11 +146,6 @@
   // from the set and tags the region appropriately.
   inline void remove(HeapRegion* hr);
 
-  // fill_in_ext_msg() writes the the values of the set's attributes
-  // in the custom err_msg (hrs_ext_msg). fill_in_ext_msg_extra()
-  // allows subclasses to append further information.
-  void fill_in_ext_msg(hrs_ext_msg* msg, const char* message);
-
   virtual void verify();
   void verify_start();
   void verify_next_region(HeapRegion* hr);
@@ -156,24 +162,13 @@
   virtual void print_on(outputStream* out, bool print_contents = false);
 };
 
-// Customized err_msg for heap region sets. Apart from a
-// assert/guarantee-specific message it also prints out the values of
-// the fields of the associated set. This can be very helpful in
-// diagnosing failures.
-class hrs_ext_msg : public hrs_err_msg {
-public:
-  hrs_ext_msg(HeapRegionSetBase* set, const char* message) : hrs_err_msg("%s", "") {
-    set->fill_in_ext_msg(this, message);
-  }
-};
-
-#define hrs_assert_sets_match(_set1_, _set2_)                                 \
-  do {                                                                        \
-    assert(((_set1_)->regions_humongous() ==                                  \
-                                            (_set2_)->regions_humongous()) && \
-           ((_set1_)->regions_free() == (_set2_)->regions_free()),            \
-           hrs_err_msg("the contents of set %s and set %s should match",      \
-                       (_set1_)->name(), (_set2_)->name()));                  \
+#define hrs_assert_sets_match(_set1_, _set2_)                                  \
+  do {                                                                         \
+    assert(((_set1_)->regions_humongous() == (_set2_)->regions_humongous()) && \
+           ((_set1_)->regions_free() == (_set2_)->regions_free()),             \
+           "the contents of set %s and set %s should match",                   \
+           (_set1_)->name(),                                                   \
+           (_set2_)->name());                                                  \
   } while (0)
 
 // This class represents heap region sets whose members are not
@@ -215,8 +210,6 @@
   inline HeapRegion* remove_from_tail_impl();
 
 protected:
-  virtual void fill_in_ext_msg_extra(hrs_ext_msg* msg);
-
   // See the comment for HeapRegionSetBase::clear()
   virtual void clear();
 
--- a/hotspot/src/share/vm/gc/g1/heapRegionSet.inline.hpp	Mon Nov 02 10:41:39 2015 +0100
+++ b/hotspot/src/share/vm/gc/g1/heapRegionSet.inline.hpp	Mon Nov 02 14:28:19 2015 +0100
@@ -29,9 +29,9 @@
 
 inline void HeapRegionSetBase::add(HeapRegion* hr) {
   check_mt_safety();
-  assert(hr->containing_set() == NULL, "%s", hrs_ext_msg(this, "should not already have a containing set %u").buffer());
-  assert(hr->next() == NULL, "%s", hrs_ext_msg(this, "should not already be linked").buffer());
-  assert(hr->prev() == NULL, "%s", hrs_ext_msg(this, "should not already be linked").buffer());
+  assert_heap_region_set(hr->containing_set() == NULL, "should not already have a containing set");
+  assert_heap_region_set(hr->next() == NULL, "should not already be linked");
+  assert_heap_region_set(hr->prev() == NULL, "should not already be linked");
 
   _count.increment(1u, hr->capacity());
   hr->set_containing_set(this);
@@ -41,18 +41,18 @@
 inline void HeapRegionSetBase::remove(HeapRegion* hr) {
   check_mt_safety();
   verify_region(hr);
-  assert(hr->next() == NULL, "%s", hrs_ext_msg(this, "should already be unlinked").buffer());
-  assert(hr->prev() == NULL, "%s", hrs_ext_msg(this, "should already be unlinked").buffer());
+  assert_heap_region_set(hr->next() == NULL, "should already be unlinked");
+  assert_heap_region_set(hr->prev() == NULL, "should already be unlinked");
 
   hr->set_containing_set(NULL);
-  assert(_count.length() > 0, "%s", hrs_ext_msg(this, "pre-condition").buffer());
+  assert_heap_region_set(_count.length() > 0, "pre-condition");
   _count.decrement(1u, hr->capacity());
 }
 
 inline void FreeRegionList::add_ordered(HeapRegion* hr) {
-  assert((length() == 0 && _head == NULL && _tail == NULL && _last == NULL) ||
-         (length() >  0 && _head != NULL && _tail != NULL),
-         "%s", hrs_ext_msg(this, "invariant").buffer());
+  assert_free_region_list((length() == 0 && _head == NULL && _tail == NULL && _last == NULL) ||
+                          (length() >  0 && _head != NULL && _tail != NULL),
+                          "invariant");
   // add() will verify the region and check mt safety.
   add(hr);
 
@@ -128,8 +128,7 @@
   if (is_empty()) {
     return NULL;
   }
-  assert(length() > 0 && _head != NULL && _tail != NULL,
-         "%s", hrs_ext_msg(this, "invariant").buffer());
+  assert_free_region_list(length() > 0 && _head != NULL && _tail != NULL, "invariant");
 
   HeapRegion* hr;