changeset 24108:dfc2242fc6b0

8040792: G1: Memory usage calculation uses sizeof(this) instead of sizeof(classname) Summary: A few locations in the code use sizeof(this) which returns the size of the pointer instead of sizeof(classname) which returns the size of the sum of its members. This change fixes these errors and adds a few tests. Reviewed-by: mgerdin, brutisso
author tschatzl
date Tue, 22 Apr 2014 11:10:12 +0200
parents 74cda485bff8
children e378f2bfaaba
files hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp hotspot/src/share/vm/gc_implementation/g1/sparsePRT.cpp
diffstat 5 files changed, 28 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp	Thu Apr 17 18:47:15 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp	Tue Apr 22 11:10:12 2014 +0200
@@ -84,7 +84,7 @@
 }
 
 size_t G1CodeRootChunkManager::static_mem_size() {
-  return sizeof(this);
+  return sizeof(G1CodeRootChunkManager);
 }
 
 
@@ -116,7 +116,7 @@
   _default_chunk_manager.purge_chunks(keep_ratio);
 }
 
-size_t G1CodeRootSet::static_mem_size() {
+size_t G1CodeRootSet::free_chunks_static_mem_size() {
   return _default_chunk_manager.static_mem_size();
 }
 
@@ -213,8 +213,12 @@
   }
 }
 
+size_t G1CodeRootSet::static_mem_size() {
+  return sizeof(G1CodeRootSet);
+}
+
 size_t G1CodeRootSet::mem_size() {
-  return sizeof(this) + _list.count() * _list.size();
+  return G1CodeRootSet::static_mem_size() + _list.count() * _list.size();
 }
 
 #ifndef PRODUCT
@@ -224,6 +228,9 @@
 
   assert(mgr.num_chunks_handed_out() == 0, "Must not have handed out chunks yet");
 
+  assert(G1CodeRootChunkManager::static_mem_size() > sizeof(void*),
+         err_msg("The chunk manager's static memory usage seems too small, is only "SIZE_FORMAT" bytes.", G1CodeRootChunkManager::static_mem_size()));
+
   // The number of chunks that we allocate for purge testing.
   size_t const num_chunks = 10;
 
@@ -231,6 +238,9 @@
     G1CodeRootSet set1(&mgr);
     assert(set1.is_empty(), "Code root set must be initially empty but is not.");
 
+    assert(G1CodeRootSet::static_mem_size() > sizeof(void*),
+           err_msg("The code root set's static memory usage seems too small, is only "SIZE_FORMAT" bytes", G1CodeRootSet::static_mem_size()));
+
     set1.add((nmethod*)1);
     assert(mgr.num_chunks_handed_out() == 1,
            err_msg("Must have allocated and handed out one chunk, but handed out "
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp	Thu Apr 17 18:47:15 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp	Tue Apr 22 11:10:12 2014 +0200
@@ -147,7 +147,7 @@
   void initialize();
   void purge_chunks(size_t keep_ratio);
 
-  size_t static_mem_size();
+  static size_t static_mem_size();
   size_t fl_mem_size();
 
 #ifndef PRODUCT
@@ -186,7 +186,7 @@
 
   static void purge_chunks(size_t keep_ratio);
 
-  static size_t static_mem_size();
+  static size_t free_chunks_static_mem_size();
   static size_t free_chunks_mem_size();
 
   // Search for the code blob from the recently allocated ones to find duplicates more quickly, as this
@@ -207,6 +207,8 @@
   // Length in elements
   size_t length() const { return _length; }
 
+  // Static data memory size in bytes of this set.
+  static size_t static_mem_size();
   // Memory size in bytes taken by this set.
   size_t mem_size();
 
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Thu Apr 17 18:47:15 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Tue Apr 22 11:10:12 2014 +0200
@@ -167,7 +167,7 @@
 
   // Mem size in bytes.
   size_t mem_size() const {
-    return sizeof(this) + _bm.size_in_words() * HeapWordSize;
+    return sizeof(PerRegionTable) + _bm.size_in_words() * HeapWordSize;
   }
 
   // Requires "from" to be in "hr()".
@@ -733,7 +733,7 @@
   sum += (sizeof(PerRegionTable*) * _max_fine_entries);
   sum += (_coarse_map.size_in_words() * HeapWordSize);
   sum += (_sparse_table.mem_size());
-  sum += sizeof(*this) - sizeof(_sparse_table); // Avoid double counting above.
+  sum += sizeof(OtherRegionsTable) - sizeof(_sparse_table); // Avoid double counting above.
   return sum;
 }
 
@@ -1248,6 +1248,11 @@
 #ifndef PRODUCT
 void PerRegionTable::test_fl_mem_size() {
   PerRegionTable* dummy = alloc(NULL);
+
+  size_t min_prt_size = sizeof(void*) + dummy->bm()->size_in_words() * HeapWordSize;
+  assert(dummy->mem_size() > min_prt_size,
+         err_msg("PerRegionTable memory usage is suspiciously small, only has "SIZE_FORMAT" bytes. "
+                 "Should be at least "SIZE_FORMAT" bytes.", dummy->mem_size(), min_prt_size));
   free(dummy);
   guarantee(dummy->mem_size() == fl_mem_size(), "fl_mem_size() does not return the correct element size");
   // try to reset the state
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Thu Apr 17 18:47:15 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Tue Apr 22 11:10:12 2014 +0200
@@ -335,14 +335,14 @@
     return _other_regions.mem_size()
       // This correction is necessary because the above includes the second
       // part.
-      + (sizeof(this) - sizeof(OtherRegionsTable))
+      + (sizeof(HeapRegionRemSet) - sizeof(OtherRegionsTable))
       + strong_code_roots_mem_size();
   }
 
   // Returns the memory occupancy of all static data structures associated
   // with remembered sets.
   static size_t static_mem_size() {
-    return OtherRegionsTable::static_mem_size() + G1CodeRootSet::static_mem_size();
+    return OtherRegionsTable::static_mem_size() + G1CodeRootSet::free_chunks_static_mem_size();
   }
 
   // Returns the memory occupancy of all free_list data structures associated
--- a/hotspot/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Thu Apr 17 18:47:15 2014 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Tue Apr 22 11:10:12 2014 +0200
@@ -370,7 +370,7 @@
 }
 
 size_t RSHashTable::mem_size() const {
-  return sizeof(this) +
+  return sizeof(RSHashTable) +
     capacity() * (SparsePRTEntry::size() + sizeof(int));
 }
 
@@ -472,7 +472,7 @@
 size_t SparsePRT::mem_size() const {
   // We ignore "_cur" here, because it either = _next, or else it is
   // on the deleted list.
-  return sizeof(this) + _next->mem_size();
+  return sizeof(SparsePRT) + _next->mem_size();
 }
 
 bool SparsePRT::add_card(RegionIdx_t region_id, CardIdx_t card_index) {