8255885: Metaspace: freelist commit counter is not updated when purging

Reviewed-by: coleenp, iklam

Author: tstuefe

src/hotspot/share/memory/metaspace/chunkManager.cpp

@@ -382,12 +382,23 @@ ChunkManager* ChunkManager::chunkmanager_nonclass() {
   return MetaspaceContext::context_nonclass() == NULL ? NULL : MetaspaceContext::context_nonclass()->cm();
 }
 
+// Calculates the total number of committed words over all chunks. Walks chunks.
+size_t ChunkManager::calc_committed_word_size() const {
+  MutexLocker fcl(MetaspaceExpand_lock, Mutex::_no_safepoint_check_flag);
+  return calc_committed_word_size_locked();
+}
+
+size_t ChunkManager::calc_committed_word_size_locked() const {
+  assert_lock_strong(MetaspaceExpand_lock);
+  return _chunks.calc_committed_word_size();
+}
+
 // Update statistics.
 void ChunkManager::add_to_statistics(ChunkManagerStats* out) const {
   MutexLocker fcl(MetaspaceExpand_lock, Mutex::_no_safepoint_check_flag);
   for (chunklevel_t l = chunklevel::ROOT_CHUNK_LEVEL; l <= chunklevel::HIGHEST_CHUNK_LEVEL; l++) {
     out->_num_chunks[l] += _chunks.num_chunks_at_level(l);
-    out->_committed_word_size[l] += _chunks.committed_word_size_at_level(l);
+    out->_committed_word_size[l] += _chunks.calc_committed_word_size_at_level(l);
   }
   DEBUG_ONLY(out->verify();)
 }
@@ -418,8 +429,8 @@ void ChunkManager::print_on(outputStream* st) const {
 
 void ChunkManager::print_on_locked(outputStream* st) const {
   assert_lock_strong(MetaspaceExpand_lock);
-  st->print_cr("cm %s: %d chunks, total word size: " SIZE_FORMAT ", committed word size: " SIZE_FORMAT, _name,
-               total_num_chunks(), total_word_size(), _chunks.committed_word_size());
+  st->print_cr("cm %s: %d chunks, total word size: " SIZE_FORMAT ".", _name,
+               total_num_chunks(), total_word_size());
   _chunks.print_on(st);
 }
 

src/hotspot/share/memory/metaspace/chunkManager.hpp

@@ -107,6 +107,9 @@ class ChunkManager : public CHeapObj<mtMetaspace> {
   // See return_chunk().
   void return_chunk_locked(Metachunk* c);
 
+  // Calculates the total number of committed words over all chunks. Walks chunks.
+  size_t calc_committed_word_size_locked() const;
+
 public:
 
   // Creates a chunk manager with a given name (which is for debug purposes only)
@@ -167,8 +170,8 @@ class ChunkManager : public CHeapObj<mtMetaspace> {
   // Returns number of words in all free chunks (regardless of commit state).
   size_t total_word_size() const            { return _chunks.word_size(); }
 
-  // Returns number of committed words in all free chunks.
-  size_t total_committed_word_size() const  { return _chunks.committed_word_size(); }
+  // Calculates the total number of committed words over all chunks. Walks chunks.
+  size_t calc_committed_word_size() const;
 
   // Update statistics.
   void add_to_statistics(ChunkManagerStats* out) const;

src/hotspot/share/memory/metaspace/freeChunkList.cpp

@@ -31,6 +31,15 @@
 
 namespace metaspace {
 
+// Calculates total number of committed words over all chunks (walks chunks).
+size_t FreeChunkList::calc_committed_word_size() const {
+  size_t s = 0;
+  for (const Metachunk* c = _first; c != NULL; c = c->next()) {
+    s += c->committed_words();
+  }
+  return s;
+}
+
 void FreeChunkList::print_on(outputStream* st) const {
   if (_num_chunks.get() > 0) {
     for (const Metachunk* c = _first; c != NULL; c = c->next()) {
@@ -60,7 +69,6 @@ void FreeChunkList::verify() const {
     assert(_last == NULL, "Sanity");
   } else {
     assert(_last != NULL, "Sanity");
-    size_t committed = 0;
     int num = 0;
     bool uncommitted = (_first->committed_words() == 0);
     for (Metachunk* c = _first; c != NULL; c = c->next()) {
@@ -71,11 +79,9 @@ void FreeChunkList::verify() const {
       assert(c->prev() == NULL || c->prev()->next() == c, "back link broken");
       assert(c != c->prev() && c != c->next(), "circle");
       c->verify();
-      committed += c->committed_words();
       num++;
     }
     _num_chunks.check(num);
-    _committed_word_size.check(committed);
   }
 }
 
@@ -90,15 +96,19 @@ size_t FreeChunkListVector::word_size() const {
   return sum;
 }
 
-// Returns total committed size in all lists
-size_t FreeChunkListVector::committed_word_size() const {
+// Calculates total number of committed words over all chunks (walks chunks).
+size_t FreeChunkListVector::calc_committed_word_size() const {
   size_t sum = 0;
   for (chunklevel_t l = chunklevel::LOWEST_CHUNK_LEVEL; l <= chunklevel::HIGHEST_CHUNK_LEVEL; l++) {
-    sum += list_for_level(l)->committed_word_size();
+    sum += calc_committed_word_size_at_level(l);
   }
   return sum;
 }
 
+size_t FreeChunkListVector::calc_committed_word_size_at_level(chunklevel_t lvl) const {
+  return list_for_level(lvl)->calc_committed_word_size();
+}
+
 // Returns total committed size in all lists
 int FreeChunkListVector::num_chunks() const {
   int n = 0;
@@ -146,8 +156,8 @@ void FreeChunkListVector::print_on(outputStream* st) const {
     list_for_level(l)->print_on(st);
     st->cr();
   }
-  st->print_cr("total chunks: %d, total word size: " SIZE_FORMAT ", committed word size: " SIZE_FORMAT ".",
-               num_chunks(), word_size(), committed_word_size());
+  st->print_cr("total chunks: %d, total word size: " SIZE_FORMAT ".",
+               num_chunks(), word_size());
 }
 
 #ifdef ASSERT

src/hotspot/share/memory/metaspace/freeChunkList.hpp

@@ -70,7 +70,6 @@ class FreeChunkList {
   Metachunk* _last;
 
   IntCounter _num_chunks;
-  SizeCounter _committed_word_size;
 
   void add_front(Metachunk* c) {
     if (_first == NULL) {
@@ -129,7 +128,6 @@ class FreeChunkList {
     }
     c->set_next(NULL);
     c->set_prev(NULL);
-    _committed_word_size.decrement_by(c->committed_words());
     _num_chunks.decrement();
     return c;
   }
@@ -144,7 +142,6 @@ class FreeChunkList {
     } else {
       add_front(c);
     }
-    _committed_word_size.increment_by(c->committed_words());
     _num_chunks.increment();
   }
 
@@ -186,8 +183,8 @@ class FreeChunkList {
   // Returns number of chunks
   int num_chunks() const { return _num_chunks.get(); }
 
-  // Returns total committed word size
-  size_t committed_word_size() const { return _committed_word_size.get(); }
+  // Calculates total number of committed words over all chunks (walks chunks).
+  size_t calc_committed_word_size() const;
 
   void print_on(outputStream* st) const;
 
@@ -226,11 +223,6 @@ class FreeChunkListVector {
     return list_for_level(lvl)->num_chunks();
   }
 
-  // Returns number of chunks for a given level.
-  size_t committed_word_size_at_level(chunklevel_t lvl) const {
-    return list_for_level(lvl)->committed_word_size();
-  }
-
   // Returns reference to first chunk at this level, or NULL if sublist is empty.
   Metachunk* first_at_level(chunklevel_t lvl) const {
     return list_for_level(lvl)->first();
@@ -247,11 +239,14 @@ class FreeChunkListVector {
   // Return NULL if no such chunk was found.
   Metachunk* search_chunk_descending(chunklevel_t level, size_t min_committed_words);
 
-  // Returns total size in all lists (regardless of commit state of underlying memory)
+  // Returns total size in all lists (including uncommitted areas)
   size_t word_size() const;
 
-  // Returns total committed size in all lists
-  size_t committed_word_size() const;
+  // Calculates total number of committed words over all chunks (walks chunks).
+  size_t calc_committed_word_size_at_level(chunklevel_t lvl) const;
+
+  // Calculates total number of committed words over all chunks (walks chunks).
+  size_t calc_committed_word_size() const;
 
   // Returns number of chunks in all lists
   int num_chunks() const;

test/hotspot/gtest/metaspace/test_metachunklist.cpp

@@ -112,7 +112,7 @@ TEST_VM(metaspace, freechunklist) {
 
     EXPECT_EQ(lst.num_chunks(), (int)cnt.count());
     EXPECT_EQ(lst.word_size(), cnt.total_size());
-    EXPECT_EQ(lst.committed_word_size(), committed_cnt.total_size());
+    EXPECT_EQ(lst.calc_committed_word_size(), committed_cnt.total_size());
   }
 
   // Drain each list separately, front to back. While draining observe the order
@@ -137,7 +137,7 @@ TEST_VM(metaspace, freechunklist) {
 
       EXPECT_EQ(lst.num_chunks(), (int)cnt.count());
       EXPECT_EQ(lst.word_size(), cnt.total_size());
-      EXPECT_EQ(lst.committed_word_size(), committed_cnt.total_size());
+      EXPECT_EQ(lst.calc_committed_word_size(), committed_cnt.total_size());
 
       context.return_chunk(c);
 

test/hotspot/gtest/metaspace/test_metaspacearena.cpp

@@ -504,7 +504,7 @@ static void test_recover_from_commit_limit_hit() {
   EXPECT_LE(allocated_from_3, Settings::commit_granule_words() * 2);
 
   // We expect the freelist to be empty of committed space...
-  EXPECT_0(context.cm().total_committed_word_size());
+  EXPECT_0(context.cm().calc_committed_word_size());
 
   //msthelper.cm().print_on(tty);
 
@@ -515,7 +515,7 @@ static void test_recover_from_commit_limit_hit() {
 
   // Should have populated the freelist with committed space
   // We expect the freelist to be empty of committed space...
-  EXPECT_GT(context.cm().total_committed_word_size(), (size_t)0);
+  EXPECT_GT(context.cm().calc_committed_word_size(), (size_t)0);
 
   // Repeat allocation from helper3, should now work.
   helper3.allocate_from_arena_with_tests_expect_success(1);