7039089: G1: changeset for 7037276 broke heap verification, and related cleanups

Tue, 26 Apr 2011 21:17:24 -0700

author
ysr
date
Tue, 26 Apr 2011 21:17:24 -0700
changeset 2825
1f4413413144
parent 2824
c303b3532d4a
child 2826
86ebb26bcdeb

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

src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/cardTableRS.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/genCollectedHeap.hpp file | annotate | diff | comparison | revisions
src/share/vm/memory/sharedHeap.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/sharedHeap.hpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/vmThread.cpp file | annotate | diff | comparison | revisions
     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();

mercurial