# HG changeset patch # User apetrusenko # Date 1256636544 25200 # Node ID 850627f5fd81e0a39ec1cdc399c7f53771defd60 # Parent 2572dd3d6d7c7f63fc95c18dc39052f0da18e4ae 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 diff -r 2572dd3d6d7c -r 850627f5fd81 src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp --- 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; } diff -r 2572dd3d6d7c -r 850627f5fd81 src/share/vm/gc_implementation/g1/sparsePRT.cpp --- 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); } diff -r 2572dd3d6d7c -r 850627f5fd81 src/share/vm/gc_implementation/g1/sparsePRT.hpp --- 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.