Lessen verification costs stuefe-new-metaspace-branch
authorstuefe
Thu, 17 Oct 2019 16:39:45 +0200
branchstuefe-new-metaspace-branch
changeset 58683 2d5dd194c65c
parent 58651 a2d3074db7a9
child 58716 960e372a6065
Lessen verification costs
src/hotspot/share/memory/metaspace/chunkManager.cpp
src/hotspot/share/memory/metaspace/commitMask.cpp
src/hotspot/share/memory/metaspace/commitMask.hpp
src/hotspot/share/memory/metaspace/metaDebug.hpp
src/hotspot/share/memory/metaspace/rootChunkArea.cpp
src/hotspot/share/memory/metaspace/spaceManager.cpp
src/hotspot/share/memory/metaspace/spaceManager.hpp
test/hotspot/gtest/metaspace/test_commitmask.cpp
--- a/src/hotspot/share/memory/metaspace/chunkManager.cpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/chunkManager.cpp	Thu Oct 17 16:39:45 2019 +0200
@@ -33,6 +33,7 @@
 #include "memory/metaspace/chunkManager.hpp"
 #include "memory/metaspace/internStat.hpp"
 #include "memory/metaspace/metachunk.hpp"
+#include "memory/metaspace/metaDebug.hpp"
 #include "memory/metaspace/metaspaceCommon.hpp"
 #include "memory/metaspace/metaspaceStatistics.hpp"
 #include "memory/metaspace/virtualSpaceNode.hpp"
@@ -110,7 +111,6 @@
   DEBUG_ONLY(chklvl::check_valid_level(target_level);)
 
   DEBUG_ONLY(c->verify(true);)
-  DEBUG_ONLY(c->vsnode()->verify(true);)
 
   // Chunk must be outside of our freelists
   assert(_chunks.contains(c) == false, "Chunk is in freelist.");
@@ -129,7 +129,7 @@
 
   DEBUG_ONLY(verify_locked(true);)
 
-  DEBUG_ONLY(c->vsnode()->verify(true);)
+  SOMETIMES(c->vsnode()->verify(true);)
 
   return c;
 }
@@ -238,6 +238,7 @@
   c->set_in_use();
 
   DEBUG_ONLY(verify_locked(false);)
+  SOMETIMES(c->vsnode()->verify(true);)
 
   log_debug(metaspace)("ChunkManager %s: handing out chunk " METACHUNK_FORMAT ".",
                        _name, METACHUNK_FORMAT_ARGS(c));
@@ -263,8 +264,7 @@
   log_debug(metaspace)("ChunkManager %s: returning chunk " METACHUNK_FORMAT ".",
                        _name, METACHUNK_FORMAT_ARGS(c));
 
-  DEBUG_ONLY(verify_locked(false);)
-  DEBUG_ONLY(c->verify(false);)
+  DEBUG_ONLY(c->verify(true);)
 
   assert(!_chunks.contains(c), "A chunk to be added to the freelist must not be in the freelist already.");
 
@@ -308,7 +308,7 @@
   return_chunk_simple(c);
 
   DEBUG_ONLY(verify_locked(false);)
-  DEBUG_ONLY(c->vsnode()->verify(true);)
+  SOMETIMES(c->vsnode()->verify(true);)
 
   DEBUG_ONLY(InternalStats::inc_num_chunks_returned_to_freelist();)
 
--- a/src/hotspot/share/memory/metaspace/commitMask.cpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/commitMask.cpp	Thu Oct 17 16:39:45 2019 +0200
@@ -24,7 +24,6 @@
  */
 
 
-#include <memory/metaspace/settings.hpp>
 #include "precompiled.hpp"
 
 #include "memory/metaspace/commitMask.hpp"
@@ -49,36 +48,44 @@
 
 #ifdef ASSERT
 
+// This is very expensive
 static const bool TEST_UNCOMMITTED_REGION = false;
 
 volatile u1 x;
 
-void CommitMask::verify(bool slow, bool do_touch_test) const {
+static void check_range_is_accessible(const MetaWord* p, size_t word_size) {
+  const MetaWord* const p_end = p + word_size;
+  u1 x2 = 0;
+  for (const MetaWord* q = p; q < p_end; q += os::vm_page_size() / BytesPerWord) {
+    x2 += *(u1*)q;
+  }
+  x = x2;
+}
+
+void CommitMask::verify(bool slow) const {
 
   // Walk the whole commit mask.
   // For each 1 bit, check if the associated granule is accessible.
   // For each 0 bit, check if the associated granule is not accessible. Slow mode only.
 
+  assert(_base != NULL && _word_size > 0 && _words_per_bit > 0, "Sanity");
   assert_is_aligned(_base, _words_per_bit * BytesPerWord);
   assert_is_aligned(_word_size, _words_per_bit);
 
-  if (do_touch_test) {
+  if (slow) {
     for (idx_t i = 0; i < size(); i ++) {
       const MetaWord* const p = _base + (i * _words_per_bit);
       if (at(i)) {
         // Should be accessible. Just touch it.
-        x ^= *(u1*)p;
+        check_range_is_accessible(p, _words_per_bit);
       } else {
-        // Should not be accessible.
-        if (slow) {
-          // Note: results may differ between platforms. On Linux, this should be true since
-          // we uncommit memory by setting protection to PROT_NONE. We may have to look if
-          // this works as expected on other platforms.
-          if (TEST_UNCOMMITTED_REGION && CanUseSafeFetch32()) {
-            assert(os::is_readable_pointer(p) == false,
-                   "index %u, pointer " PTR_FORMAT ", should not be accessible.",
-                   (unsigned)i, p2i(p));
-          }
+        // Note: results may differ between platforms. On Linux, this should be true since
+        // we uncommit memory by setting protection to PROT_NONE. We may have to look if
+        // this works as expected on other platforms.
+        if (TEST_UNCOMMITTED_REGION && CanUseSafeFetch32()) {
+          assert(os::is_readable_pointer(p) == false,
+                 "index %u, pointer " PTR_FORMAT ", should not be accessible.",
+                 (unsigned)i, p2i(p));
         }
       }
     }
--- a/src/hotspot/share/memory/metaspace/commitMask.hpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/commitMask.hpp	Thu Oct 17 16:39:45 2019 +0200
@@ -180,7 +180,7 @@
 
 
   //// Debug stuff ////
-  DEBUG_ONLY(void verify(bool slow, bool do_touch_test = true) const;)
+  DEBUG_ONLY(void verify(bool slow) const;)
 
   void print_on(outputStream* st) const;
 
--- a/src/hotspot/share/memory/metaspace/metaDebug.hpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/metaDebug.hpp	Thu Oct 17 16:39:45 2019 +0200
@@ -26,6 +26,7 @@
 #define SHARE_MEMORY_METASPACE_METADEBUG_HPP
 
 #include "memory/allocation.hpp"
+#include "runtime/globals.hpp" // For MetaspaceVerifyInterval
 
 namespace metaspace {
 
--- a/src/hotspot/share/memory/metaspace/rootChunkArea.cpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/rootChunkArea.cpp	Thu Oct 17 16:39:45 2019 +0200
@@ -30,6 +30,7 @@
 #include "memory/metaspace/chunkManager.hpp"
 #include "memory/metaspace/internStat.hpp"
 #include "memory/metaspace/metachunk.hpp"
+#include "memory/metaspace/metaDebug.hpp"
 #include "memory/metaspace/metaspaceCommon.hpp"
 #include "memory/metaspace/rootChunkArea.hpp"
 #include "runtime/mutexLocker.hpp"
@@ -81,8 +82,6 @@
 // Returns NULL if chunk cannot be split at least once.
 Metachunk* RootChunkArea::split(chklvl_t target_level, Metachunk* c, MetachunkListCluster* freelists) {
 
-  DEBUG_ONLY(c->verify(true);)
-
   // Splitting a chunk once works like this:
   //
   // For a given chunk we want to split:
@@ -119,8 +118,6 @@
   DEBUG_ONLY(chklvl::check_valid_level(target_level));
   assert(target_level > c->level(), "Wrong target level");
 
-  DEBUG_ONLY(verify(true);)
-
   const chklvl_t starting_level = c->level();
 
   Metachunk* result = c;
@@ -423,8 +420,8 @@
 
 void RootChunkArea::verify(bool slow) const {
 
+
   assert_lock_strong(MetaspaceExpand_lock);
-
   assert_is_aligned(_base, chklvl::MAX_CHUNK_BYTE_SIZE);
 
   // Iterate thru all chunks in this area. They must be ordered correctly,
@@ -468,7 +465,7 @@
 
 void RootChunkArea::verify_area_is_ideally_merged() const {
 
-  assert_lock_strong(MetaspaceExpand_lock);
+  SOMETIMES(assert_lock_strong(MetaspaceExpand_lock);)
 
   int num_chunk = 0;
   for (const Metachunk* c = _first_chunk; c != NULL; c = c->next_in_vs()) {
--- a/src/hotspot/share/memory/metaspace/spaceManager.cpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/spaceManager.cpp	Thu Oct 17 16:39:45 2019 +0200
@@ -411,6 +411,7 @@
   }
 
   SOMETIMES(out->verify();)
+
 }
 
 #ifdef ASSERT
--- a/src/hotspot/share/memory/metaspace/spaceManager.hpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/spaceManager.hpp	Thu Oct 17 16:39:45 2019 +0200
@@ -34,6 +34,7 @@
 #include "memory/metaspace/metachunk.hpp"
 #include "memory/metaspace/metaspaceCommon.hpp"
 
+
 class outputStream;
 class Mutex;
 
--- a/test/hotspot/gtest/metaspace/test_commitmask.cpp	Wed Oct 16 17:41:03 2019 +0200
+++ b/test/hotspot/gtest/metaspace/test_commitmask.cpp	Thu Oct 17 16:39:45 2019 +0200
@@ -39,7 +39,7 @@
   void verify_mask() {
     // Note: we omit the touch test since we operate on fictional
     // memory
-    DEBUG_ONLY(_mask.verify(true, false);)
+    DEBUG_ONLY(_mask.verify(false);)
   }
 
   // Return a random sub range within [_base.._base + word_size),
@@ -258,7 +258,7 @@
 
 };
 
-TEST(metaspace, commit_mask_basics) {
+TEST_VM(metaspace, commit_mask_basics) {
 
   const MetaWord* const base = (const MetaWord*) 0x100000;
 
@@ -282,7 +282,7 @@
 
 }
 
-TEST(metaspace, commit_mask_small) {
+TEST_VM(metaspace, commit_mask_small) {
 
   const MetaWord* const base = (const MetaWord*) 0x100000;
 
@@ -291,7 +291,7 @@
 
 }
 
-TEST(metaspace, commit_mask_range) {
+TEST_VM(metaspace, commit_mask_range) {
 
   const MetaWord* const base = (const MetaWord*) 0x100000;
   const size_t len = Settings::commit_granule_words() * 4;
@@ -329,7 +329,7 @@
 }
 
 
-TEST(metaspace, commit_mask_random) {
+TEST_VM(metaspace, commit_mask_random) {
 
   for (int i = 0; i < 5; i ++) {