From 2d2ad866669c3ba9c1c02332d99c2fc92f742b68 Mon Sep 17 00:00:00 2001 From: johnc Date: Mon, 29 Aug 2011 10:13:06 -0700 Subject: [PATCH] 7080389: G1: refactor marking code in evacuation pause copy closures Summary: Refactor code marking code in the evacuation pause copy closures so that an evacuated object is only marked by the thread that successfully copies it. Reviewed-by: stefank, brutisso, tonyp --- .../gc_implementation/g1/g1CollectedHeap.cpp | 82 ++++++++++++++++--- .../gc_implementation/g1/g1CollectedHeap.hpp | 16 ++-- .../vm/gc_implementation/g1/g1OopClosures.hpp | 10 +-- .../g1/g1_specialized_oop_closures.hpp | 2 +- 4 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index 48b017dc6..52b85d011 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -4069,6 +4069,23 @@ bool GCLabBitMapClosure::do_bit(size_t offset) { } #endif // PRODUCT +G1ParGCAllocBuffer::G1ParGCAllocBuffer(size_t gclab_word_size) : + ParGCAllocBuffer(gclab_word_size), + _should_mark_objects(false), + _bitmap(G1CollectedHeap::heap()->reserved_region().start(), gclab_word_size), + _retired(false) +{ + //_should_mark_objects is set to true when G1ParCopyHelper needs to + // mark the forwarded location of an evacuated object. + // We set _should_mark_objects to true if marking is active, i.e. when we + // need to propagate a mark, or during an initial mark pause, i.e. when we + // need to mark objects immediately reachable by the roots. + if (G1CollectedHeap::heap()->mark_in_progress() || + G1CollectedHeap::heap()->g1_policy()->during_initial_mark_pause()) { + _should_mark_objects = true; + } +} + G1ParScanThreadState::G1ParScanThreadState(G1CollectedHeap* g1h, int queue_num) : _g1h(g1h), _refs(g1h->task_queue(queue_num)), @@ -4184,12 +4201,14 @@ void G1ParScanThreadState::trim_queue() { G1ParClosureSuper::G1ParClosureSuper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state) : _g1(g1), _g1_rem(_g1->g1_rem_set()), _cm(_g1->concurrent_mark()), - _par_scan_state(par_scan_state) { } + _par_scan_state(par_scan_state), + _during_initial_mark(_g1->g1_policy()->during_initial_mark_pause()), + _mark_in_progress(_g1->mark_in_progress()) { } -template void G1ParCopyHelper::mark_forwardee(T* p) { - // This is called _after_ do_oop_work has been called, hence after - // the object has been relocated to its new location and *p points - // to its new location. +template void G1ParCopyHelper::mark_object(T* p) { + // This is called from do_oop_work for objects that are not + // in the collection set. Objects in the collection set + // are marked after they have been evacuated. T heap_oop = oopDesc::load_heap_oop(p); if (!oopDesc::is_null(heap_oop)) { @@ -4201,7 +4220,7 @@ template void G1ParCopyHelper::mark_forwardee(T* p) { } } -oop G1ParCopyHelper::copy_to_survivor_space(oop old) { +oop G1ParCopyHelper::copy_to_survivor_space(oop old, bool should_mark_copy) { size_t word_sz = old->size(); HeapRegion* from_region = _g1->heap_region_containing_raw(old); // +1 to make the -1 indexes valid... @@ -4257,8 +4276,8 @@ oop G1ParCopyHelper::copy_to_survivor_space(oop old) { obj->set_mark(m); } - // preserve "next" mark bit - if (_g1->mark_in_progress() && !_g1->is_obj_ill(old)) { + // Mark the evacuated object or propagate "next" mark bit + if (should_mark_copy) { if (!use_local_bitmaps || !_par_scan_state->alloc_buffer(alloc_purpose)->mark(obj_ptr)) { // if we couldn't mark it on the local bitmap (this happens when @@ -4266,11 +4285,12 @@ oop G1ParCopyHelper::copy_to_survivor_space(oop old) { // the bullet and do the standard parallel mark _cm->markAndGrayObjectIfNecessary(obj); } -#if 1 + if (_g1->isMarkedNext(old)) { + // Unmark the object's old location so that marking + // doesn't think the old object is alive. _cm->nextMarkBitMap()->parClear((HeapWord*)old); } -#endif } size_t* surv_young_words = _par_scan_state->surviving_young_words(); @@ -4293,26 +4313,62 @@ oop G1ParCopyHelper::copy_to_survivor_space(oop old) { return obj; } -template +template template -void G1ParCopyClosure +void G1ParCopyClosure ::do_oop_work(T* p) { oop obj = oopDesc::load_decode_heap_oop(p); assert(barrier != G1BarrierRS || obj != NULL, "Precondition: G1BarrierRS implies obj is nonNull"); + // Marking: + // If the object is in the collection set, then the thread + // that copies the object should mark, or propagate the + // mark to, the evacuated object. + // If the object is not in the collection set then we + // should call the mark_object() method depending on the + // value of the template parameter do_mark_object (which will + // be true for root scanning closures during an initial mark + // pause). + // The mark_object() method first checks whether the object + // is marked and, if not, attempts to mark the object. + // here the null check is implicit in the cset_fast_test() test if (_g1->in_cset_fast_test(obj)) { if (obj->is_forwarded()) { oopDesc::encode_store_heap_oop(p, obj->forwardee()); + // If we are a root scanning closure during an initial + // mark pause (i.e. do_mark_object will be true) then + // we also need to handle marking of roots in the + // event of an evacuation failure. In the event of an + // evacuation failure, the object is forwarded to itself + // and not copied so let's mark it here. + if (do_mark_object && obj->forwardee() == obj) { + mark_object(p); + } } else { - oop copy_oop = copy_to_survivor_space(obj); + // We need to mark the copied object if we're a root scanning + // closure during an initial mark pause (i.e. do_mark_object + // will be true), or the object is already marked and we need + // to propagate the mark to the evacuated copy. + bool should_mark_copy = do_mark_object || + _during_initial_mark || + (_mark_in_progress && !_g1->is_obj_ill(obj)); + + oop copy_oop = copy_to_survivor_space(obj, should_mark_copy); oopDesc::encode_store_heap_oop(p, copy_oop); } // When scanning the RS, we only care about objs in CS. if (barrier == G1BarrierRS) { _par_scan_state->update_rs(_from, p, _par_scan_state->queue_num()); } + } else { + // The object is not in collection set. If we're a root scanning + // closure during an initial mark pause (i.e. do_mark_object will + // be true) then attempt to mark the object. + if (do_mark_object) { + mark_object(p); + } } if (barrier == G1BarrierEvac && obj != NULL) { diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp index 154fc8c7a..650e43887 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp @@ -1715,26 +1715,22 @@ public: class G1ParGCAllocBuffer: public ParGCAllocBuffer { private: bool _retired; - bool _during_marking; + bool _should_mark_objects; GCLabBitMap _bitmap; public: - G1ParGCAllocBuffer(size_t gclab_word_size) : - ParGCAllocBuffer(gclab_word_size), - _during_marking(G1CollectedHeap::heap()->mark_in_progress()), - _bitmap(G1CollectedHeap::heap()->reserved_region().start(), gclab_word_size), - _retired(false) - { } + G1ParGCAllocBuffer(size_t gclab_word_size); inline bool mark(HeapWord* addr) { guarantee(use_local_bitmaps, "invariant"); - assert(_during_marking, "invariant"); + assert(_should_mark_objects, "invariant"); return _bitmap.mark(addr); } inline void set_buf(HeapWord* buf) { - if (use_local_bitmaps && _during_marking) + if (use_local_bitmaps && _should_mark_objects) { _bitmap.set_buffer(buf); + } ParGCAllocBuffer::set_buf(buf); _retired = false; } @@ -1742,7 +1738,7 @@ public: inline void retire(bool end_of_gc, bool retain) { if (_retired) return; - if (use_local_bitmaps && _during_marking) { + if (use_local_bitmaps && _should_mark_objects) { _bitmap.retire(); } ParGCAllocBuffer::retire(end_of_gc, retain); diff --git a/src/share/vm/gc_implementation/g1/g1OopClosures.hpp b/src/share/vm/gc_implementation/g1/g1OopClosures.hpp index 8eaaa9d1e..d3f1ce24c 100644 --- a/src/share/vm/gc_implementation/g1/g1OopClosures.hpp +++ b/src/share/vm/gc_implementation/g1/g1OopClosures.hpp @@ -50,6 +50,8 @@ protected: G1RemSet* _g1_rem; ConcurrentMark* _cm; G1ParScanThreadState* _par_scan_state; + bool _during_initial_mark; + bool _mark_in_progress; public: G1ParClosureSuper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state); bool apply_to_weak_ref_discovered_field() { return true; } @@ -102,8 +104,8 @@ public: class G1ParCopyHelper : public G1ParClosureSuper { G1ParScanClosure *_scanner; protected: - template void mark_forwardee(T* p); - oop copy_to_survivor_space(oop obj); + template void mark_object(T* p); + oop copy_to_survivor_space(oop obj, bool should_mark_copy); public: G1ParCopyHelper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state, G1ParScanClosure *scanner) : @@ -111,7 +113,7 @@ public: }; template + bool do_mark_object> class G1ParCopyClosure : public G1ParCopyHelper { G1ParScanClosure _scanner; template void do_oop_work(T* p); @@ -120,8 +122,6 @@ public: _scanner(g1, par_scan_state), G1ParCopyHelper(g1, par_scan_state, &_scanner) { } template void do_oop_nv(T* p) { do_oop_work(p); - if (do_mark_forwardee) - mark_forwardee(p); } virtual void do_oop(oop* p) { do_oop_nv(p); } virtual void do_oop(narrowOop* p) { do_oop_nv(p); } diff --git a/src/share/vm/gc_implementation/g1/g1_specialized_oop_closures.hpp b/src/share/vm/gc_implementation/g1/g1_specialized_oop_closures.hpp index 5fcd66f1c..fea3e076d 100644 --- a/src/share/vm/gc_implementation/g1/g1_specialized_oop_closures.hpp +++ b/src/share/vm/gc_implementation/g1/g1_specialized_oop_closures.hpp @@ -36,7 +36,7 @@ enum G1Barrier { }; template + bool do_mark_object> class G1ParCopyClosure; class G1ParScanClosure; class G1ParPushHeapRSClosure; -- GitLab