8212911: Unify and micro-optimize handling of non-in-collection set references in oop closures
Reviewed-by: kbarrett, sjohanss
--- a/src/hotspot/share/gc/g1/g1OopClosures.cpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1OopClosures.cpp Wed Oct 31 13:43:57 2018 +0100
@@ -38,7 +38,7 @@
{ }
G1ScanClosureBase::G1ScanClosureBase(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state) :
- _g1h(g1h), _par_scan_state(par_scan_state), _from(NULL)
+ _g1h(g1h), _par_scan_state(par_scan_state)
{ }
void G1CLDScanClosure::do_cld(ClassLoaderData* cld) {
--- a/src/hotspot/share/gc/g1/g1OopClosures.hpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1OopClosures.hpp Wed Oct 31 13:43:57 2018 +0100
@@ -43,7 +43,6 @@
protected:
G1CollectedHeap* _g1h;
G1ParScanThreadState* _par_scan_state;
- HeapRegion* _from;
G1ScanClosureBase(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state);
~G1ScanClosureBase() { }
@@ -56,24 +55,19 @@
public:
virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS; }
- void set_region(HeapRegion* from) { _from = from; }
-
inline void trim_queue_partially();
};
// Used during the Update RS phase to refine remaining cards in the DCQ during garbage collection.
-class G1ScanObjsDuringUpdateRSClosure: public G1ScanClosureBase {
- uint _worker_i;
-
+class G1ScanObjsDuringUpdateRSClosure : public G1ScanClosureBase {
public:
G1ScanObjsDuringUpdateRSClosure(G1CollectedHeap* g1h,
- G1ParScanThreadState* pss,
- uint worker_i) :
- G1ScanClosureBase(g1h, pss), _worker_i(worker_i) { }
+ G1ParScanThreadState* pss) :
+ G1ScanClosureBase(g1h, pss) { }
template <class T> void do_oop_work(T* p);
virtual void do_oop(narrowOop* p) { do_oop_work(p); }
- virtual void do_oop(oop* p) { do_oop_work(p); }
+ virtual void do_oop(oop* p) { do_oop_work(p); }
};
// Used during the Scan RS phase to scan cards from the remembered set during garbage collection.
@@ -90,9 +84,13 @@
// This closure is applied to the fields of the objects that have just been copied during evacuation.
class G1ScanEvacuatedObjClosure : public G1ScanClosureBase {
+ bool _scanning_in_young;
+
public:
G1ScanEvacuatedObjClosure(G1CollectedHeap* g1h, G1ParScanThreadState* par_scan_state) :
- G1ScanClosureBase(g1h, par_scan_state) { }
+ G1ScanClosureBase(g1h, par_scan_state), _scanning_in_young(false) { }
+
+ void set_scanning_in_young(bool scanning_in_young) { _scanning_in_young = scanning_in_young; }
template <class T> void do_oop_work(T* p);
virtual void do_oop(oop* p) { do_oop_work(p); }
--- a/src/hotspot/share/gc/g1/g1OopClosures.inline.hpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1OopClosures.inline.hpp Wed Oct 31 13:43:57 2018 +0100
@@ -82,12 +82,12 @@
const InCSetState state = _g1h->in_cset_state(obj);
if (state.is_in_cset()) {
prefetch_and_push(p, obj);
- } else {
- if (HeapRegion::is_in_same_region(p, obj)) {
+ } else if (!HeapRegion::is_in_same_region(p, obj)) {
+ handle_non_cset_obj_common(state, p, obj);
+ if (_scanning_in_young) {
return;
}
- handle_non_cset_obj_common(state, p, obj);
- _par_scan_state->update_rs(_from, p, obj);
+ _par_scan_state->enqueue_card_if_tracked(p, obj);
}
}
@@ -172,13 +172,9 @@
// Since the source is always from outside the collection set, here we implicitly know
// that this is a cross-region reference too.
prefetch_and_push(p, obj);
- } else {
- HeapRegion* to = _g1h->heap_region_containing(obj);
- if (_from == to) {
- return;
- }
+ } else if (!HeapRegion::is_in_same_region(p, obj)) {
handle_non_cset_obj_common(state, p, obj);
- to->rem_set()->add_reference(p, _worker_i);
+ _par_scan_state->enqueue_card_if_tracked(p, obj);
}
}
@@ -193,10 +189,7 @@
const InCSetState state = _g1h->in_cset_state(obj);
if (state.is_in_cset()) {
prefetch_and_push(p, obj);
- } else {
- if (HeapRegion::is_in_same_region(p, obj)) {
- return;
- }
+ } else if (!HeapRegion::is_in_same_region(p, obj)) {
handle_non_cset_obj_common(state, p, obj);
}
}
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp Wed Oct 31 13:43:57 2018 +0100
@@ -311,8 +311,7 @@
oop* old_p = set_partial_array_mask(old);
do_oop_partial_array(old_p);
} else {
- HeapRegion* const to_region = _g1h->heap_region_containing(obj_ptr);
- _scanner.set_region(to_region);
+ _scanner.set_scanning_in_young(dest_state.is_young());
obj->oop_iterate_backwards(&_scanner);
}
return obj;
@@ -367,7 +366,7 @@
_g1h->preserve_mark_during_evac_failure(_worker_id, old, m);
- _scanner.set_region(r);
+ _scanner.set_scanning_in_young(r->is_young());
old->oop_iterate_backwards(&_scanner);
return old;
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.hpp Wed Oct 31 13:43:57 2018 +0100
@@ -104,17 +104,16 @@
template <class T> void do_oop_ext(T* ref);
template <class T> void push_on_queue(T* ref);
- template <class T> void update_rs(HeapRegion* from, T* p, oop o) {
- assert(!HeapRegion::is_in_same_region(p, o), "Caller should have filtered out cross-region references already.");
- // If the field originates from the to-space, we don't need to include it
- // in the remembered set updates. Also, if we are not tracking the remembered
- // set in the destination region, do not bother either.
- if (!from->is_young() && _g1h->heap_region_containing((HeapWord*)o)->rem_set()->is_tracked()) {
- size_t card_index = ct()->index_for(p);
- // If the card hasn't been added to the buffer, do it.
- if (ct()->mark_card_deferred(card_index)) {
- dirty_card_queue().enqueue((jbyte*)ct()->byte_for_index(card_index));
- }
+ template <class T> void enqueue_card_if_tracked(T* p, oop o) {
+ assert(!HeapRegion::is_in_same_region(p, o), "Should have filtered out cross-region references already.");
+ assert(!_g1h->heap_region_containing(p)->is_young(), "Should have filtered out from-young references already.");
+ if (!_g1h->heap_region_containing((HeapWord*)o)->rem_set()->is_tracked()) {
+ return;
+ }
+ size_t card_index = ct()->index_for(p);
+ // If the card hasn't been added to the buffer, do it.
+ if (ct()->mark_card_deferred(card_index)) {
+ dirty_card_queue().enqueue((jbyte*)ct()->byte_for_index(card_index));
}
}
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.inline.hpp Wed Oct 31 13:43:57 2018 +0100
@@ -61,9 +61,12 @@
RawAccess<IS_NOT_NULL>::oop_store(p, obj);
assert(obj != NULL, "Must be");
- if (!HeapRegion::is_in_same_region(p, obj)) {
- HeapRegion* from = _g1h->heap_region_containing(p);
- update_rs(from, p, obj);
+ if (HeapRegion::is_in_same_region(p, obj)) {
+ return;
+ }
+ HeapRegion* from = _g1h->heap_region_containing(p);
+ if (!from->is_young()) {
+ enqueue_card_if_tracked(p, obj);
}
}
@@ -109,7 +112,9 @@
// so that the heap remains parsable in case of evacuation failure.
to_obj_array->set_length(end);
}
- _scanner.set_region(_g1h->heap_region_containing(to_obj));
+
+ HeapRegion* hr = _g1h->heap_region_containing(to_obj);
+ _scanner.set_scanning_in_young(hr->is_young());
// Process indexes [start,end). It will also process the header
// along with the first chunk (i.e., the chunk with start == 0).
// Note that at this point the length field of to_obj_array is not
--- a/src/hotspot/share/gc/g1/g1RemSet.cpp Wed Oct 31 13:43:57 2018 +0100
+++ b/src/hotspot/share/gc/g1/g1RemSet.cpp Wed Oct 31 13:43:57 2018 +0100
@@ -334,7 +334,7 @@
void G1ScanRSForRegionClosure::scan_card(MemRegion mr, uint region_idx_for_card) {
HeapRegion* const card_region = _g1h->region_at(region_idx_for_card);
- _scan_objs_on_card_cl->set_region(card_region);
+ assert(!card_region->is_young(), "Should not scan card in young region %u", region_idx_for_card);
card_region->oops_on_card_seq_iterate_careful<true>(mr, _scan_objs_on_card_cl);
_scan_objs_on_card_cl->trim_queue_partially();
_cards_scanned++;
@@ -494,7 +494,7 @@
if (G1HotCardCache::default_use_cache()) {
G1EvacPhaseTimesTracker x(p, pss, G1GCPhaseTimes::ScanHCC, worker_i);
- G1ScanObjsDuringUpdateRSClosure scan_hcc_cl(_g1h, pss, worker_i);
+ G1ScanObjsDuringUpdateRSClosure scan_hcc_cl(_g1h, pss);
G1RefineCardClosure refine_card_cl(_g1h, &scan_hcc_cl);
_g1h->iterate_hcc_closure(&refine_card_cl, worker_i);
}
@@ -503,7 +503,7 @@
{
G1EvacPhaseTimesTracker x(p, pss, G1GCPhaseTimes::UpdateRS, worker_i);
- G1ScanObjsDuringUpdateRSClosure update_rs_cl(_g1h, pss, worker_i);
+ G1ScanObjsDuringUpdateRSClosure update_rs_cl(_g1h, pss);
G1RefineCardClosure refine_card_cl(_g1h, &update_rs_cl);
_g1h->iterate_dirty_card_closure(&refine_card_cl, worker_i);
@@ -729,7 +729,7 @@
assert(!dirty_region.is_empty(), "sanity");
HeapRegion* const card_region = _g1h->region_at(card_region_idx);
- update_rs_cl->set_region(card_region);
+ assert(!card_region->is_young(), "Should not scan card in young region %u", card_region_idx);
bool card_processed = card_region->oops_on_card_seq_iterate_careful<true>(dirty_region, update_rs_cl);
assert(card_processed, "must be");
return true;