changeset 20218:828056cf311f

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 Mon, 21 Jul 2014 09:40:19 +0200
parents 6b52700a896b
children f40816c5e359
files src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp src/share/vm/gc_implementation/g1/sparsePRT.cpp
diffstat 5 files changed, 28 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp	Mon Jul 21 09:40:19 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp	Mon Jul 21 09:40:19 2014 +0200
@@ -86,7 +86,7 @@
 }
 
 size_t G1CodeRootChunkManager::static_mem_size() {
-  return sizeof(this);
+  return sizeof(G1CodeRootChunkManager);
 }
 
 
@@ -118,7 +118,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();
 }
 
@@ -215,8 +215,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
@@ -226,6 +230,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;
 
@@ -233,6 +240,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/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp	Mon Jul 21 09:40:19 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp	Mon Jul 21 09:40:19 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/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Mon Jul 21 09:40:19 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp	Mon Jul 21 09:40:19 2014 +0200
@@ -169,7 +169,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()".
@@ -735,7 +735,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;
 }
 
@@ -1249,6 +1249,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/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Mon Jul 21 09:40:19 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp	Mon Jul 21 09:40:19 2014 +0200
@@ -339,14 +339,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/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Mon Jul 21 09:40:19 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Mon Jul 21 09:40:19 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) {