Tue, 26 Apr 2011 21:17:24 -0700
7039089: G1: changeset for 7037276 broke heap verification, and related cleanups
Summary: In G1 heap verification, we no longer scan perm to G1-collected heap refs as part of process_strong_roots() but rather in a separate explicit oop iteration over the perm gen. This preserves the original perm card-marks. Added a new assertion in younger_refs_iterate() to catch a simple subcase where the user may have forgotten a prior save_marks() call, as happened in the case of G1's attempt to iterate perm to G1 refs when verifying the heap before exit. The assert was deliberately weakened for ParNew+CMS and will be fixed for that combination in a future CR. Also made some (non-G1) cleanups related to code and comments obsoleted by the migration of Symbols to the native heap.
Reviewed-by: iveresov, jmasa, tonyp
1.1 --- a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Tue Apr 26 11:46:34 2011 -0700 1.2 +++ b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Tue Apr 26 21:17:24 2011 -0700 1.3 @@ -1963,10 +1963,21 @@ 1.4 // Iteration support, mostly delegated from a CMS generation 1.5 1.6 void CompactibleFreeListSpace::save_marks() { 1.7 - // mark the "end" of the used space at the time of this call; 1.8 + assert(Thread::current()->is_VM_thread(), 1.9 + "Global variable should only be set when single-threaded"); 1.10 + // Mark the "end" of the used space at the time of this call; 1.11 // note, however, that promoted objects from this point 1.12 // on are tracked in the _promoInfo below. 1.13 set_saved_mark_word(unallocated_block()); 1.14 +#ifdef ASSERT 1.15 + // Check the sanity of save_marks() etc. 1.16 + MemRegion ur = used_region(); 1.17 + MemRegion urasm = used_region_at_save_marks(); 1.18 + assert(ur.contains(urasm), 1.19 + err_msg(" Error at save_marks(): [" PTR_FORMAT "," PTR_FORMAT ")" 1.20 + " should contain [" PTR_FORMAT "," PTR_FORMAT ")", 1.21 + ur.start(), ur.end(), urasm.start(), urasm.end())); 1.22 +#endif 1.23 // inform allocator that promotions should be tracked. 1.24 assert(_promoInfo.noPromotions(), "_promoInfo inconsistency"); 1.25 _promoInfo.startTrackingPromotions();
2.1 --- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Tue Apr 26 11:46:34 2011 -0700 2.2 +++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Tue Apr 26 21:17:24 2011 -0700 2.3 @@ -3189,10 +3189,9 @@ 2.4 } 2.5 2.6 void CMSCollector::setup_cms_unloading_and_verification_state() { 2.7 - const bool should_verify = VerifyBeforeGC || VerifyAfterGC || VerifyDuringGC 2.8 + const bool should_verify = VerifyBeforeGC || VerifyAfterGC || VerifyDuringGC 2.9 || VerifyBeforeExit; 2.10 - const int rso = SharedHeap::SO_Symbols | SharedHeap::SO_Strings 2.11 - | SharedHeap::SO_CodeCache; 2.12 + const int rso = SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; 2.13 2.14 if (should_unload_classes()) { // Should unload classes this cycle 2.15 remove_root_scanning_option(rso); // Shrink the root set appropriately
3.1 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Tue Apr 26 11:46:34 2011 -0700 3.2 +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Tue Apr 26 21:17:24 2011 -0700 3.3 @@ -2805,17 +2805,26 @@ 3.4 bool silent, 3.5 bool use_prev_marking) { 3.6 if (SafepointSynchronize::is_at_safepoint() || ! UseTLAB) { 3.7 - if (!silent) { gclog_or_tty->print("roots "); } 3.8 + if (!silent) { gclog_or_tty->print("Roots (excluding permgen) "); } 3.9 VerifyRootsClosure rootsCl(use_prev_marking); 3.10 CodeBlobToOopClosure blobsCl(&rootsCl, /*do_marking=*/ false); 3.11 - process_strong_roots(true, // activate StrongRootsScope 3.12 - false, 3.13 - SharedHeap::SO_AllClasses, 3.14 + // We apply the relevant closures to all the oops in the 3.15 + // system dictionary, the string table and the code cache. 3.16 + const int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; 3.17 + process_strong_roots(true, // activate StrongRootsScope 3.18 + true, // we set "collecting perm gen" to true, 3.19 + // so we don't reset the dirty cards in the perm gen. 3.20 + SharedHeap::ScanningOption(so), // roots scanning options 3.21 &rootsCl, 3.22 &blobsCl, 3.23 &rootsCl); 3.24 + // Since we used "collecting_perm_gen" == true above, we will not have 3.25 + // checked the refs from perm into the G1-collected heap. We check those 3.26 + // references explicitly below. Whether the relevant cards are dirty 3.27 + // is checked further below in the rem set verification. 3.28 + if (!silent) { gclog_or_tty->print("Permgen roots "); } 3.29 + perm_gen()->oop_iterate(&rootsCl); 3.30 bool failures = rootsCl.failures(); 3.31 - rem_set()->invalidate(perm_gen()->used_region(), false); 3.32 if (!silent) { gclog_or_tty->print("HeapRegionSets "); } 3.33 verify_region_sets(); 3.34 if (!silent) { gclog_or_tty->print("HeapRegions "); }
4.1 --- a/src/share/vm/memory/cardTableRS.cpp Tue Apr 26 11:46:34 2011 -0700 4.2 +++ b/src/share/vm/memory/cardTableRS.cpp Tue Apr 26 21:17:24 2011 -0700 4.3 @@ -250,7 +250,34 @@ 4.4 cl->gen_boundary()); 4.5 ClearNoncleanCardWrapper clear_cl(dcto_cl, this); 4.6 4.7 - _ct_bs->non_clean_card_iterate_possibly_parallel(sp, sp->used_region_at_save_marks(), 4.8 + const MemRegion urasm = sp->used_region_at_save_marks(); 4.9 +#ifdef ASSERT 4.10 + // Convert the assertion check to a warning if we are running 4.11 + // CMS+ParNew until related bug is fixed. 4.12 + MemRegion ur = sp->used_region(); 4.13 + assert(ur.contains(urasm) || (UseConcMarkSweepGC && UseParNewGC), 4.14 + err_msg("Did you forget to call save_marks()? " 4.15 + "[" PTR_FORMAT ", " PTR_FORMAT ") is not contained in " 4.16 + "[" PTR_FORMAT ", " PTR_FORMAT ")", 4.17 + urasm.start(), urasm.end(), ur.start(), ur.end())); 4.18 + // In the case of CMS+ParNew, issue a warning 4.19 + if (!ur.contains(urasm)) { 4.20 + assert(UseConcMarkSweepGC && UseParNewGC, "Tautology: see assert above"); 4.21 + warning("CMS+ParNew: Did you forget to call save_marks()? " 4.22 + "[" PTR_FORMAT ", " PTR_FORMAT ") is not contained in " 4.23 + "[" PTR_FORMAT ", " PTR_FORMAT ")", 4.24 + urasm.start(), urasm.end(), ur.start(), ur.end()); 4.25 + MemRegion ur2 = sp->used_region(); 4.26 + MemRegion urasm2 = sp->used_region_at_save_marks(); 4.27 + if (!ur.equals(ur2)) { 4.28 + warning("CMS+ParNew: Flickering used_region()!!"); 4.29 + } 4.30 + if (!urasm.equals(urasm2)) { 4.31 + warning("CMS+ParNew: Flickering used_region_at_save_marks()!!"); 4.32 + } 4.33 + } 4.34 +#endif 4.35 + _ct_bs->non_clean_card_iterate_possibly_parallel(sp, urasm, 4.36 dcto_cl, &clear_cl); 4.37 } 4.38
5.1 --- a/src/share/vm/memory/genCollectedHeap.hpp Tue Apr 26 11:46:34 2011 -0700 5.2 +++ b/src/share/vm/memory/genCollectedHeap.hpp Tue Apr 26 21:17:24 2011 -0700 5.3 @@ -427,13 +427,13 @@ 5.4 // explicitly mark reachable objects in younger generations, to avoid 5.5 // excess storage retention.) If "collecting_perm_gen" is false, then 5.6 // roots that may only contain references to permGen objects are not 5.7 - // scanned. The "so" argument determines which of the roots 5.8 + // scanned; instead, the older_gens closure is applied to all outgoing 5.9 + // references in the perm gen. The "so" argument determines which of the roots 5.10 // the closure is applied to: 5.11 // "SO_None" does none; 5.12 // "SO_AllClasses" applies the closure to all entries in the SystemDictionary; 5.13 // "SO_SystemClasses" to all the "system" classes and loaders; 5.14 - // "SO_Symbols_and_Strings" applies the closure to all entries in 5.15 - // SymbolsTable and StringTable. 5.16 + // "SO_Strings" applies the closure to all entries in the StringTable. 5.17 void gen_process_strong_roots(int level, 5.18 bool younger_gens_as_roots, 5.19 // The remaining arguments are in an order
6.1 --- a/src/share/vm/memory/sharedHeap.cpp Tue Apr 26 11:46:34 2011 -0700 6.2 +++ b/src/share/vm/memory/sharedHeap.cpp Tue Apr 26 21:17:24 2011 -0700 6.3 @@ -46,7 +46,6 @@ 6.4 SH_PS_Management_oops_do, 6.5 SH_PS_SystemDictionary_oops_do, 6.6 SH_PS_jvmti_oops_do, 6.7 - SH_PS_SymbolTable_oops_do, 6.8 SH_PS_StringTable_oops_do, 6.9 SH_PS_CodeCache_oops_do, 6.10 // Leave this one last. 6.11 @@ -161,13 +160,9 @@ 6.12 if (!_process_strong_tasks->is_task_claimed(SH_PS_SystemDictionary_oops_do)) { 6.13 if (so & SO_AllClasses) { 6.14 SystemDictionary::oops_do(roots); 6.15 - } else 6.16 - if (so & SO_SystemClasses) { 6.17 - SystemDictionary::always_strong_oops_do(roots); 6.18 - } 6.19 - } 6.20 - 6.21 - if (!_process_strong_tasks->is_task_claimed(SH_PS_SymbolTable_oops_do)) { 6.22 + } else if (so & SO_SystemClasses) { 6.23 + SystemDictionary::always_strong_oops_do(roots); 6.24 + } 6.25 } 6.26 6.27 if (!_process_strong_tasks->is_task_claimed(SH_PS_StringTable_oops_do)) {
7.1 --- a/src/share/vm/memory/sharedHeap.hpp Tue Apr 26 11:46:34 2011 -0700 7.2 +++ b/src/share/vm/memory/sharedHeap.hpp Tue Apr 26 21:17:24 2011 -0700 7.3 @@ -192,9 +192,8 @@ 7.4 SO_None = 0x0, 7.5 SO_AllClasses = 0x1, 7.6 SO_SystemClasses = 0x2, 7.7 - SO_Symbols = 0x4, 7.8 - SO_Strings = 0x8, 7.9 - SO_CodeCache = 0x10 7.10 + SO_Strings = 0x4, 7.11 + SO_CodeCache = 0x8 7.12 }; 7.13 7.14 FlexibleWorkGang* workers() const { return _workers; } 7.15 @@ -208,14 +207,13 @@ 7.16 7.17 // Invoke the "do_oop" method the closure "roots" on all root locations. 7.18 // If "collecting_perm_gen" is false, then roots that may only contain 7.19 - // references to permGen objects are not scanned. If true, the 7.20 - // "perm_gen" closure is applied to all older-to-younger refs in the 7.21 + // references to permGen objects are not scanned; instead, in that case, 7.22 + // the "perm_blk" closure is applied to all outgoing refs in the 7.23 // permanent generation. The "so" argument determines which of roots 7.24 // the closure is applied to: 7.25 // "SO_None" does none; 7.26 // "SO_AllClasses" applies the closure to all entries in the SystemDictionary; 7.27 // "SO_SystemClasses" to all the "system" classes and loaders; 7.28 - // "SO_Symbols" applies the closure to all entries in SymbolsTable; 7.29 // "SO_Strings" applies the closure to all entries in StringTable; 7.30 // "SO_CodeCache" applies the closure to all elements of the CodeCache. 7.31 void process_strong_roots(bool activate_scope,
8.1 --- a/src/share/vm/runtime/vmThread.cpp Tue Apr 26 11:46:34 2011 -0700 8.2 +++ b/src/share/vm/runtime/vmThread.cpp Tue Apr 26 21:17:24 2011 -0700 8.3 @@ -291,7 +291,9 @@ 8.4 // Among other things, this ensures that Eden top is correct. 8.5 Universe::heap()->prepare_for_verify(); 8.6 os::check_heap(); 8.7 - Universe::verify(true, true); // Silent verification to not polute normal output 8.8 + // Silent verification so as not to pollute normal output, 8.9 + // unless we really asked for it. 8.10 + Universe::verify(true, !(PrintGCDetails || Verbose)); 8.11 } 8.12 8.13 CompileBroker::set_should_block();