Mercurial > hg > openjdk > hsx16

changeset 966:850627f5fd81

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6870843: G1: G1 GC memory leak Summary: The fix addresses two memory leaks in G1 code: (1) _evac_failure_scan_stack - a resource object allocated on the C heap was not freed; (2) RSHashTable were linked into deleted list which was only cleared at full GC. Reviewed-by: tonyp, iveresov
author apetrusenko
date Tue, 27 Oct 2009 02:42:24 -0700
parents 2572dd3d6d7c
children 9f7d1f6201ab
files src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp src/share/vm/gc_implementation/g1/sparsePRT.cpp src/share/vm/gc_implementation/g1/sparsePRT.hpp
diffstat 3 files changed, 8 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Wed Sep 30 14:50:51 2009 -0400
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Oct 27 02:42:24 2009 -0700
@@ -3133,7 +3133,7 @@
          _evac_failure_scan_stack->length() == 0,
          "Postcondition");
   assert(!_drain_in_progress, "Postcondition");
-  // Don't have to delete, since the scan stack is a resource object.
+  delete _evac_failure_scan_stack;
   _evac_failure_scan_stack = NULL;
 }
 
--- a/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Wed Sep 30 14:50:51 2009 -0400
+++ b/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Tue Oct 27 02:42:24 2009 -0700
@@ -135,7 +135,6 @@
   _occupied_entries(0), _occupied_cards(0),
   _entries(NEW_C_HEAP_ARRAY(SparsePRTEntry, capacity)),
   _buckets(NEW_C_HEAP_ARRAY(int, capacity)),
-  _next_deleted(NULL), _deleted(false),
   _free_list(NullEntry), _free_region(0)
 {
   clear();
@@ -296,40 +295,6 @@
   assert(e2->num_valid_cards() > 0, "Postcondition.");
 }
 
-RSHashTable* RSHashTable::_head_deleted_list = NULL;
-
-void RSHashTable::add_to_deleted_list(RSHashTable* rsht) {
-  assert(!rsht->deleted(), "Should delete only once.");
-  rsht->set_deleted(true);
-  RSHashTable* hd = _head_deleted_list;
-  while (true) {
-    rsht->_next_deleted = hd;
-    RSHashTable* res =
-      (RSHashTable*)
-      Atomic::cmpxchg_ptr(rsht, &_head_deleted_list, hd);
-    if (res == hd) return;
-    else hd = res;
-  }
-}
-
-RSHashTable* RSHashTable::get_from_deleted_list() {
-  RSHashTable* hd = _head_deleted_list;
-  while (hd != NULL) {
-    RSHashTable* next = hd->next_deleted();
-    RSHashTable* res =
-      (RSHashTable*)
-      Atomic::cmpxchg_ptr(next, &_head_deleted_list, hd);
-    if (res == hd) {
-      hd->set_next_deleted(NULL);
-      hd->set_deleted(false);
-      return hd;
-    } else {
-      hd = res;
-    }
-  }
-  return NULL;
-}
-
 CardIdx_t /* RSHashTable:: */ RSHashTableIter::find_first_card_in_list() {
   CardIdx_t res;
   while (_bl_ind != RSHashTable::NullEntry) {
@@ -442,15 +407,6 @@
     sprt->cleanup();
     sprt = get_from_expanded_list();
   }
-  // Now delete all deleted RSHashTables.
-  RSHashTable* rsht = RSHashTable::get_from_deleted_list();
-  while (rsht != NULL) {
-#if SPARSE_PRT_VERBOSE
-    gclog_or_tty->print_cr("About to delete RSHT " PTR_FORMAT ".", rsht);
-#endif
-    delete rsht;
-    rsht = RSHashTable::get_from_deleted_list();
-  }
 }
 
 
@@ -511,8 +467,10 @@
 }
 
 void SparsePRT::cleanup() {
-  // Make sure that the current and next tables agree.  (Another mechanism
-  // takes care of deleting now-unused tables.)
+  // Make sure that the current and next tables agree.
+  if (_cur != _next) {
+    delete _cur;
+  }
   _cur = _next;
   set_expanded(false);
 }
@@ -535,7 +493,8 @@
       _next->add_entry(e);
     }
   }
-  if (last != _cur)
-    RSHashTable::add_to_deleted_list(last);
+  if (last != _cur) {
+    delete last;
+  }
   add_to_expanded_list(this);
 }
--- a/src/share/vm/gc_implementation/g1/sparsePRT.hpp	Wed Sep 30 14:50:51 2009 -0400
+++ b/src/share/vm/gc_implementation/g1/sparsePRT.hpp	Tue Oct 27 02:42:24 2009 -0700
@@ -102,13 +102,6 @@
   int  _free_region;
   int  _free_list;
 
-  static RSHashTable* _head_deleted_list;
-  RSHashTable* _next_deleted;
-  RSHashTable* next_deleted() { return _next_deleted; }
-  void set_next_deleted(RSHashTable* rsht) { _next_deleted = rsht; }
-  bool _deleted;
-  void set_deleted(bool b) { _deleted = b; }
-
   // Requires that the caller hold a lock preventing parallel modifying
   // operations, and that the the table be less than completely full.  If
   // an entry for "region_ind" is already in the table, finds it and
@@ -154,14 +147,10 @@
   size_t occupied_entries() const { return _occupied_entries; }
   size_t occupied_cards() const   { return _occupied_cards;   }
   size_t mem_size() const;
-  bool deleted() { return _deleted; }
 
   SparsePRTEntry* entry(int i) const { return &_entries[i]; }
 
   void print();
-
-  static void add_to_deleted_list(RSHashTable* rsht);
-  static RSHashTable* get_from_deleted_list();
 };
 
 // ValueObj because will be embedded in HRRS iterator.