--- old/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp 2013-06-19 12:28:43.859414957 -0700 +++ new/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp 2013-06-19 12:28:43.623860123 -0700 @@ -3349,7 +3349,7 @@ if (!silent) { gclog_or_tty->print("Roots "); } VerifyRootsClosure rootsCl(vo); G1VerifyCodeRootOopClosure codeRootsCl(this, &rootsCl, vo); - G1VerifyCodeRootBlobClosure blobsCl(&codeRootsCl, /*do_marking=*/ false); + G1VerifyCodeRootBlobClosure blobsCl(&codeRootsCl, false /* do_marking */); VerifyKlassClosure klassCl(this, &rootsCl); // We apply the relevant closures to all the oops in the @@ -3877,11 +3877,9 @@ append_secondary_free_list_if_not_empty_with_lock(); } - assert(check_young_list_well_formed(), - "young list should be well formed"); - + assert(check_young_list_well_formed(), "young list should be well formed"); assert(check_heap_region_claim_values(HeapRegion::InitialClaimValue), - "sanity check"); + "sanity check"); // Don't dynamically change the number of GC threads this early. A value of // 0 is used to indicate serial work. When parallel work is done, @@ -4985,9 +4983,10 @@ G1ParPushHeapRSClosure push_heap_rs_cl(_g1h, &pss); - // Don't scan the code cache as part of strong root scanning. The code - // roots that point into a region are scanned when we scan the RSet - // of that region. + // Don't scan the scavengable methods in the code cache as part + // of strong root scanning. The code roots that point into a + // region in the collection set are scanned when we scan the + // region's RSet. int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings; pss.start_strong_roots(); @@ -5099,7 +5098,7 @@ // the entire code cache, we need to mark the oops in the // strong code root lists for the regions that are not in // the collection set. - // Note all threads partipate in this set of root tasks. + // Note all threads participate in this set of root tasks. double mark_strong_code_roots_ms = 0.0; if (g1_policy()->during_initial_mark_pause() && !(so & SO_CodeCache)) { double mark_strong_roots_start = os::elapsedTime(); @@ -6551,13 +6550,14 @@ template void do_oop_work(T* p) { T heap_oop = oopDesc::load_heap_oop(p); if (!oopDesc::is_null(heap_oop)) { - HeapRegion* hr = _g1h->heap_region_containing(heap_oop); - assert(!hr->isHumongous(), "nmethod oop in numongous?"); - - // Note this may push duplicates but that is OK since - // when we scan the nmethods during GC we "mark" them - // as visited. - hr->push_strong_code_root(_nm); + oop obj = oopDesc::decode_heap_oop_not_null(heap_oop); + HeapRegion* hr = _g1h->heap_region_containing(obj); + assert(!hr->isHumongous(), "code root in humongous region?"); + + // HeapRegion::add_strong_code_root() avoids adding duplicate + // entries but having duplicates is OK since we "mark" nmethods + // as visited when we scan the strong code root lists during the GC. + hr->add_strong_code_root(_nm); assert(hr->strong_code_root_list()->contains(_nm), "push failed?"); } } @@ -6577,8 +6577,9 @@ template void do_oop_work(T* p) { T heap_oop = oopDesc::load_heap_oop(p); if (!oopDesc::is_null(heap_oop)) { - HeapRegion* hr = _g1h->heap_region_containing(heap_oop); - assert(!hr->isHumongous(), "nmethod oop in numongous?"); + oop obj = oopDesc::decode_heap_oop_not_null(heap_oop); + HeapRegion* hr = _g1h->heap_region_containing(obj); + assert(!hr->isHumongous(), "code root in humongous region?"); hr->remove_strong_code_root(_nm); assert(!hr->strong_code_root_list()->contains(_nm), "remove failed?"); } @@ -6593,17 +6594,13 @@ }; void G1CollectedHeap::register_nmethod(nmethod* nm) { - assert(nm != NULL, "sanity"); - if (nm == NULL) return; - + guarantee(nm != NULL, "sanity"); RegisterNMethodOopClosure reg_cl(this, nm); nm->oops_do(®_cl); } void G1CollectedHeap::unregister_nmethod(nmethod* nm) { - assert(nm != NULL, "sanity"); - if (nm == NULL) return; - + guarantee(nm != NULL, "sanity"); UnregisterNMethodOopClosure reg_cl(this, nm); nm->oops_do(®_cl); } @@ -6625,18 +6622,20 @@ g1_policy()->phase_times()->record_strong_code_root_migration_time(migration_time_ms); } +// Mark all the code roots that point into regions *not* in the +// collection set. +// +// Note we do not want to use a "marking" CodeBlobToOopClosure while +// walking the the code roots lists of regions not in the collection +// set. Suppose we have an nmethod (M) that points to objects in two +// separate regions - one in the collection set (R1) and one not (R2). +// Using a "marking" CodeBlobToOopClosure here would result in "marking" +// nmethod M when walking the code roots for R1. When we come to scan +// the code roots for R2, we would see that M is already marked and it +// would be skipped and the objects in R2 that are referenced from M +// would not be evacuated. + class MarkStrongCodeRootCodeBlobClosure: public CodeBlobClosure { - // Note when we're marking the oops in the strong code roots lists - // for regions not in the collection set, we do not want to use a - // "marking" CodeBlobToOopClosure. We don't want to get in the following - // situation if marking sees an nmethod whose oops span a couple - // of regions (one in the collection set and the other not). - // - // A "marking" CodeBlobToOopClosure would end up "marking" the nmethod - // by walking the code root list for region not in the collection set. - // When we come to scan the code root list for the region in the - // collection, we would skip the nmethod because it's already been - // "marked" - potentially missing some roots. class MarkStrongCodeRootOopClosure: public OopClosure { ConcurrentMark* _cm; @@ -6657,7 +6656,9 @@ public: MarkStrongCodeRootOopClosure(ConcurrentMark* cm, HeapRegion* hr, uint worker_id) : - _cm(cm), _hr(hr), _worker_id(worker_id) {} + _cm(cm), _hr(hr), _worker_id(worker_id) { + assert(!_hr->in_collection_set(), "sanity"); + } void do_oop(narrowOop* p) { do_oop_work(p); } void do_oop(oop* p) { do_oop_work(p); } --- old/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp 2013-06-19 12:28:45.124391728 -0700 +++ new/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp 2013-06-19 12:28:44.896250383 -0700 @@ -1639,11 +1639,13 @@ virtual void unregister_nmethod(nmethod* nm); // Migrate the nmethods in the code root lists of the regions - // in the collection set to regions in to-space + // in the collection set to regions in to-space. In the event + // of an evacuation failure, nmethods that reference objects + // that were not successfullly evacuated are not migrated. void migrate_strong_code_roots(); - // During an initial mark pause, mark the strong code roots that - // point into the heap. + // During an initial mark pause, mark all the code roots that + // point into regions *not* in the collection set. void mark_strong_code_roots(uint worker_id); // Rebuild the stong code root lists for each region --- old/src/share/vm/gc_implementation/g1/g1RemSet.cpp 2013-06-19 12:28:46.287079568 -0700 +++ new/src/share/vm/gc_implementation/g1/g1RemSet.cpp 2013-06-19 12:28:46.062564921 -0700 @@ -112,9 +112,9 @@ CardTableModRefBS *_ct_bs; double _strong_code_root_scan_time_sec; - int _worker_i; - int _block_size; - bool _try_claimed; + int _worker_i; + int _block_size; + bool _try_claimed; public: ScanRSClosure(OopsInHeapRegionClosure* oc, --- old/src/share/vm/gc_implementation/g1/g1RemSet.hpp 2013-06-19 12:28:47.425049843 -0700 +++ new/src/share/vm/gc_implementation/g1/g1RemSet.hpp 2013-06-19 12:28:47.201074447 -0700 @@ -94,7 +94,7 @@ // of the worker thread calling this function can be helpful in // partitioning the work to be done. It should be the same as // the "i" passed to the calling thread's work(i) function. - // In the sequential case this param will be ingored. + // In the sequential case this param will be ignored. void oops_into_collection_set_do(OopsInHeapRegionClosure* blk, CodeBlobToOopClosure* code_root_cl, int worker_i); --- old/src/share/vm/gc_implementation/g1/g1_globals.hpp 2013-06-19 12:28:48.553629714 -0700 +++ new/src/share/vm/gc_implementation/g1/g1_globals.hpp 2013-06-19 12:28:48.328652671 -0700 @@ -336,8 +336,7 @@ "remembered set when verifying the heap during a full GC.") \ \ diagnostic(bool, G1VerifyHeapRegionCodeRoots, false, \ - "If true, performs verification of the code root lists " \ - "attached to each heap region.") + "Verify the code root lists attached to each heap region.") G1_FLAGS(DECLARE_DEVELOPER_FLAG, DECLARE_PD_DEVELOPER_FLAG, DECLARE_PRODUCT_FLAG, DECLARE_PD_PRODUCT_FLAG, DECLARE_DIAGNOSTIC_FLAG, DECLARE_EXPERIMENTAL_FLAG, DECLARE_NOTPRODUCT_FLAG, DECLARE_MANAGEABLE_FLAG, DECLARE_PRODUCT_RW_FLAG) --- old/src/share/vm/gc_implementation/g1/heapRegion.cpp 2013-06-19 12:28:49.686388576 -0700 +++ new/src/share/vm/gc_implementation/g1/heapRegion.cpp 2013-06-19 12:28:49.462542707 -0700 @@ -233,7 +233,13 @@ _offsets.resize(HeapRegion::GrainWords); init_top_at_mark_start(); - _strong_code_root_list->clear(); + + if (_strong_code_root_list != NULL) { + delete _strong_code_root_list; + } + _strong_code_root_list = new (ResourceObj::C_HEAP, mtGC) + GrowableArray(10, 0, NULL, true); + if (clear_space) clear(SpaceDecorator::Mangle); } @@ -366,8 +372,6 @@ _strong_code_root_list(NULL) { _orig_end = mr.end(); - _strong_code_root_list = new (ResourceObj::C_HEAP, mtGC) - GrowableArray(10, 0, NULL, true); // Note that initialize() will set the start of the unmarked area of the // region. hr_clear(false /*par*/, false /*clear_space*/); @@ -597,7 +601,7 @@ // Code roots support -void HeapRegion::push_strong_code_root(nmethod* nm) { +void HeapRegion::add_strong_code_root(nmethod* nm) { assert(nm != NULL, "sanity"); // Search for the code blob from the RHS to avoid // duplicate entries as much as possible @@ -655,7 +659,7 @@ } else { // The reference points into a promotion or to-space region HeapRegion* to = _g1h->heap_region_containing(obj); - to->push_strong_code_root(_nm); + to->add_strong_code_root(_nm); } } } @@ -694,7 +698,7 @@ while (to_be_retained.is_nonempty()) { nmethod* nm = to_be_retained.pop(); assert(nm != NULL, "sanity"); - push_strong_code_root(nm); + add_strong_code_root(nm); } } @@ -1218,5 +1222,3 @@ _offsets.zero_bottom_entry(); _offsets.initialize_threshold(); } - -template class GrowableArray; --- old/src/share/vm/gc_implementation/g1/heapRegion.hpp 2013-06-19 12:28:50.849451701 -0700 +++ new/src/share/vm/gc_implementation/g1/heapRegion.hpp 2013-06-19 12:28:50.618247896 -0700 @@ -803,24 +803,25 @@ // Routines for managing the list of code roots that point into // this heap region. - void push_strong_code_root(nmethod* nm); + void add_strong_code_root(nmethod* nm); void remove_strong_code_root(nmethod* nm); GrowableArray* strong_code_root_list() { return _strong_code_root_list; } - // During a collection, migrate strong code roots attached to - // this region to the new regions the regions they ppoint into. + // During a collection, migrate successfully evacuated strong + // code roots attached to this region to the new regions that + // they point into. Unsuccessfully evacuated code roots are + // not migrated. void migrate_strong_code_roots(); - // Applied blk->do_code_blob() to each of the entries in + // Applies blk->do_code_blob() to each of the entries in // the strong code roots list; void strong_code_roots_do(CodeBlobClosure* blk) const; // Verify that the entries on the strong code root list are live and - // include at lease one pointer into this region. - // Returns the number of failures. + // include at least one pointer into this region. void verify_strong_code_roots(VerifyOption vo, bool* failures) const; void print() const;