diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index 561aba5ebd7af45594aaa5c2d1b49cfbc9de1376..2fa28d9f3cb306d92ceb5ae1002574243d0d54c0 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -1483,9 +1483,7 @@ bool G1CollectedHeap::do_collection(bool explicit_gc, // Discard all rset updates JavaThread::dirty_card_queue_set().abandon_logs(); - assert(!G1DeferredRSUpdate - || (G1DeferredRSUpdate && - (dirty_card_queue_set().completed_buffers_num() == 0)), "Should not be any"); + assert(dirty_card_queue_set().completed_buffers_num() == 0, "DCQS should be empty"); _young_list->reset_sampled_info(); // At this point there should be no regions in the @@ -2100,15 +2098,13 @@ jint G1CollectedHeap::initialize() { concurrent_g1_refine()->red_zone(), Shared_DirtyCardQ_lock); - if (G1DeferredRSUpdate) { - dirty_card_queue_set().initialize(NULL, // Should never be called by the Java code - DirtyCardQ_CBL_mon, - DirtyCardQ_FL_lock, - -1, // never trigger processing - -1, // no limit on length - Shared_DirtyCardQ_lock, - &JavaThread::dirty_card_queue_set()); - } + dirty_card_queue_set().initialize(NULL, // Should never be called by the Java code + DirtyCardQ_CBL_mon, + DirtyCardQ_FL_lock, + -1, // never trigger processing + -1, // no limit on length + Shared_DirtyCardQ_lock, + &JavaThread::dirty_card_queue_set()); // Initialize the card queue set used to hold cards containing // references into the collection set. @@ -5303,7 +5299,6 @@ class G1RedirtyLoggedCardsTask : public AbstractGangTask { }; void G1CollectedHeap::redirty_logged_cards() { - guarantee(G1DeferredRSUpdate, "Must only be called when using deferred RS updates."); double redirty_logged_cards_start = os::elapsedTime(); uint n_workers = (G1CollectedHeap::use_parallel_gc_threads() ? @@ -5963,9 +5958,7 @@ void G1CollectedHeap::evacuate_collection_set(EvacuationInfo& evacuation_info) { // RSets. enqueue_discovered_references(n_workers); - if (G1DeferredRSUpdate) { - redirty_logged_cards(); - } + redirty_logged_cards(); COMPILER2_PRESENT(DerivedPointerTable::update_pointers()); } diff --git a/src/share/vm/gc_implementation/g1/g1EvacFailure.hpp b/src/share/vm/gc_implementation/g1/g1EvacFailure.hpp index 72d1ca179f89d7493330f7d0fbe533a83061ec4d..e628340105f08550d9f1e5f4bd80b09e9205be62 100644 --- a/src/share/vm/gc_implementation/g1/g1EvacFailure.hpp +++ b/src/share/vm/gc_implementation/g1/g1EvacFailure.hpp @@ -176,15 +176,17 @@ public: class RemoveSelfForwardPtrHRClosure: public HeapRegionClosure { G1CollectedHeap* _g1h; ConcurrentMark* _cm; - OopsInHeapRegionClosure *_update_rset_cl; uint _worker_id; + DirtyCardQueue _dcq; + UpdateRSetDeferred _update_rset_cl; + public: RemoveSelfForwardPtrHRClosure(G1CollectedHeap* g1h, - OopsInHeapRegionClosure* update_rset_cl, uint worker_id) : - _g1h(g1h), _update_rset_cl(update_rset_cl), - _worker_id(worker_id), _cm(_g1h->concurrent_mark()) { } + _g1h(g1h), _dcq(&g1h->dirty_card_queue_set()), _update_rset_cl(g1h, &_dcq), + _worker_id(worker_id), _cm(_g1h->concurrent_mark()) { + } bool doHeapRegion(HeapRegion *hr) { bool during_initial_mark = _g1h->g1_policy()->during_initial_mark_pause(); @@ -195,7 +197,7 @@ public: if (hr->claimHeapRegion(HeapRegion::ParEvacFailureClaimValue)) { if (hr->evacuation_failed()) { - RemoveSelfForwardPtrObjClosure rspc(_g1h, _cm, hr, _update_rset_cl, + RemoveSelfForwardPtrObjClosure rspc(_g1h, _cm, hr, &_update_rset_cl, during_initial_mark, during_conc_mark, _worker_id); @@ -214,7 +216,7 @@ public: // whenever this might be required in the future. hr->rem_set()->reset_for_par_iteration(); hr->reset_bot(); - _update_rset_cl->set_region(hr); + _update_rset_cl.set_region(hr); hr->object_iterate(&rspc); hr->rem_set()->clean_strong_code_roots(hr); @@ -238,16 +240,7 @@ public: _g1h(g1h) { } void work(uint worker_id) { - UpdateRSetImmediate immediate_update(_g1h->g1_rem_set()); - DirtyCardQueue dcq(&_g1h->dirty_card_queue_set()); - UpdateRSetDeferred deferred_update(_g1h, &dcq); - - OopsInHeapRegionClosure *update_rset_cl = &deferred_update; - if (!G1DeferredRSUpdate) { - update_rset_cl = &immediate_update; - } - - RemoveSelfForwardPtrHRClosure rsfp_cl(_g1h, update_rset_cl, worker_id); + RemoveSelfForwardPtrHRClosure rsfp_cl(_g1h, worker_id); HeapRegion* hr = _g1h->start_cset_region_for_worker(worker_id); _g1h->collection_set_iterate_from(hr, &rsfp_cl); diff --git a/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp b/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp index 8e5b29e9ca100f49d2765f21c00f24f24ad96016..a133510348ed32cc2e6c39a93465d718c50a411e 100644 --- a/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp +++ b/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp @@ -236,10 +236,8 @@ void G1GCPhaseTimes::note_gc_end() { _last_gc_worker_times_ms.verify(); _last_gc_worker_other_times_ms.verify(); - if (G1DeferredRSUpdate) { - _last_redirty_logged_cards_time_ms.verify(); - _last_redirty_logged_cards_processed_cards.verify(); - } + _last_redirty_logged_cards_time_ms.verify(); + _last_redirty_logged_cards_processed_cards.verify(); } void G1GCPhaseTimes::note_string_dedup_fixup_start() { @@ -351,12 +349,10 @@ void G1GCPhaseTimes::print(double pause_time_sec) { _recorded_non_young_cset_choice_time_ms)); print_stats(2, "Ref Proc", _cur_ref_proc_time_ms); print_stats(2, "Ref Enq", _cur_ref_enq_time_ms); - if (G1DeferredRSUpdate) { - print_stats(2, "Redirty Cards", _recorded_redirty_logged_cards_time_ms); - if (G1Log::finest()) { - _last_redirty_logged_cards_time_ms.print(3, "Parallel Redirty"); - _last_redirty_logged_cards_processed_cards.print(3, "Redirtied Cards"); - } + print_stats(2, "Redirty Cards", _recorded_redirty_logged_cards_time_ms); + if (G1Log::finest()) { + _last_redirty_logged_cards_time_ms.print(3, "Parallel Redirty"); + _last_redirty_logged_cards_processed_cards.print(3, "Redirtied Cards"); } if (G1ReclaimDeadHumongousObjectsAtYoungGC) { print_stats(2, "Humongous Reclaim", _cur_fast_reclaim_humongous_time_ms); diff --git a/src/share/vm/gc_implementation/g1/g1ParScanThreadState.hpp b/src/share/vm/gc_implementation/g1/g1ParScanThreadState.hpp index 3f2dc69eb16c263aefde2a5f00a75dd110c827c9..c1262a68882ea16f75295385f947800c1a7b7c24 100644 --- a/src/share/vm/gc_implementation/g1/g1ParScanThreadState.hpp +++ b/src/share/vm/gc_implementation/g1/g1ParScanThreadState.hpp @@ -82,20 +82,6 @@ class G1ParScanThreadState : public StackObj { DirtyCardQueue& dirty_card_queue() { return _dcq; } G1SATBCardTableModRefBS* ctbs() { return _ct_bs; } - template inline void immediate_rs_update(HeapRegion* from, T* p, int tid); - - template void deferred_rs_update(HeapRegion* from, T* p, int tid) { - // If the new value of the field points to the same region or - // is the to-space, we don't need to include it in the Rset updates. - if (!from->is_in_reserved(oopDesc::load_decode_heap_oop(p)) && !from->is_survivor()) { - size_t card_index = ctbs()->index_for(p); - // If the card hasn't been added to the buffer, do it. - if (ctbs()->mark_card_deferred(card_index)) { - dirty_card_queue().enqueue((jbyte*)ctbs()->byte_for_index(card_index)); - } - } - } - public: G1ParScanThreadState(G1CollectedHeap* g1h, uint queue_num, ReferenceProcessor* rp); ~G1ParScanThreadState(); @@ -115,8 +101,17 @@ class G1ParScanThreadState : public StackObj { _refs->push(ref); } - template inline void update_rs(HeapRegion* from, T* p, int tid); - + template void update_rs(HeapRegion* from, T* p, int tid) { + // If the new value of the field points to the same region or + // is the to-space, we don't need to include it in the Rset updates. + if (!from->is_in_reserved(oopDesc::load_decode_heap_oop(p)) && !from->is_survivor()) { + size_t card_index = ctbs()->index_for(p); + // If the card hasn't been added to the buffer, do it. + if (ctbs()->mark_card_deferred(card_index)) { + dirty_card_queue().enqueue((jbyte*)ctbs()->byte_for_index(card_index)); + } + } + } public: void set_evac_failure_closure(OopsInHeapRegionClosure* evac_failure_cl) { diff --git a/src/share/vm/gc_implementation/g1/g1ParScanThreadState.inline.hpp b/src/share/vm/gc_implementation/g1/g1ParScanThreadState.inline.hpp index 75517fb3f9d426f1185f9f83dc42b88a9bb9b098..3fb1829f20d0f05617296fcccf74f2c3f10fe732 100644 --- a/src/share/vm/gc_implementation/g1/g1ParScanThreadState.inline.hpp +++ b/src/share/vm/gc_implementation/g1/g1ParScanThreadState.inline.hpp @@ -29,20 +29,6 @@ #include "gc_implementation/g1/g1RemSet.inline.hpp" #include "oops/oop.inline.hpp" -template inline void G1ParScanThreadState::immediate_rs_update(HeapRegion* from, T* p, int tid) { - if (!from->is_survivor()) { - _g1_rem->par_write_ref(from, p, tid); - } -} - -template void G1ParScanThreadState::update_rs(HeapRegion* from, T* p, int tid) { - if (G1DeferredRSUpdate) { - deferred_rs_update(from, p, tid); - } else { - immediate_rs_update(from, p, tid); - } -} - template void G1ParScanThreadState::do_oop_evac(T* p, HeapRegion* from) { assert(!oopDesc::is_null(oopDesc::load_decode_heap_oop(p)), "Reference should not be NULL here as such are never pushed to the task queue."); diff --git a/src/share/vm/gc_implementation/g1/g1RemSet.cpp b/src/share/vm/gc_implementation/g1/g1RemSet.cpp index 42376213c609e11998ca91711c5e344752aed79a..b28fd2376dde1242f1c37cb8a593fd9e76cae973 100644 --- a/src/share/vm/gc_implementation/g1/g1RemSet.cpp +++ b/src/share/vm/gc_implementation/g1/g1RemSet.cpp @@ -338,12 +338,8 @@ void G1RemSet::oops_into_collection_set_do(OopsInHeapRegionClosure* oc, // are just discarded (there's no need to update the RSets of regions // that were in the collection set - after the pause these regions // are wholly 'free' of live objects. In the event of an evacuation - // failure the cards/buffers in this queue set are: - // * passed to the DirtyCardQueueSet that is used to manage deferred - // RSet updates, or - // * scanned for references that point into the collection set - // and the RSet of the corresponding region in the collection set - // is updated immediately. + // failure the cards/buffers in this queue set are passed to the + // DirtyCardQueueSet that is used to manage RSet updates DirtyCardQueue into_cset_dcq(&_g1->into_cset_dirty_card_queue_set()); assert((ParallelGCThreads > 0) || worker_i == 0, "invariant"); @@ -372,7 +368,6 @@ void G1RemSet::oops_into_collection_set_do(OopsInHeapRegionClosure* oc, void G1RemSet::prepare_for_oops_into_collection_set_do() { cleanupHRRS(); - ConcurrentG1Refine* cg1r = _g1->concurrent_g1_refine(); _g1->set_refine_cte_cl_concurrency(false); DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set(); dcqs.concatenate_logs(); @@ -385,66 +380,6 @@ void G1RemSet::prepare_for_oops_into_collection_set_do() { _total_cards_scanned = 0; } - -// This closure, applied to a DirtyCardQueueSet, is used to immediately -// update the RSets for the regions in the CSet. For each card it iterates -// through the oops which coincide with that card. It scans the reference -// fields in each oop; when it finds an oop that points into the collection -// set, the RSet for the region containing the referenced object is updated. -class UpdateRSetCardTableEntryIntoCSetClosure: public CardTableEntryClosure { - G1CollectedHeap* _g1; - CardTableModRefBS* _ct_bs; -public: - UpdateRSetCardTableEntryIntoCSetClosure(G1CollectedHeap* g1, - CardTableModRefBS* bs): - _g1(g1), _ct_bs(bs) - { } - - bool do_card_ptr(jbyte* card_ptr, uint worker_i) { - // Construct the region representing the card. - HeapWord* start = _ct_bs->addr_for(card_ptr); - // And find the region containing it. - HeapRegion* r = _g1->heap_region_containing(start); - - // Scan oops in the card looking for references into the collection set - // Don't use addr_for(card_ptr + 1) which can ask for - // a card beyond the heap. This is not safe without a perm - // gen. - HeapWord* end = start + CardTableModRefBS::card_size_in_words; - MemRegion scanRegion(start, end); - - UpdateRSetImmediate update_rs_cl(_g1->g1_rem_set()); - FilterIntoCSClosure update_rs_cset_oop_cl(NULL, _g1, &update_rs_cl); - FilterOutOfRegionClosure filter_then_update_rs_cset_oop_cl(r, &update_rs_cset_oop_cl); - - // We can pass false as the "filter_young" parameter here as: - // * we should be in a STW pause, - // * the DCQS to which this closure is applied is used to hold - // references that point into the collection set from the prior - // RSet updating, - // * the post-write barrier shouldn't be logging updates to young - // regions (but there is a situation where this can happen - see - // the comment in G1RemSet::refine_card() below - - // that should not be applicable here), and - // * during actual RSet updating, the filtering of cards in young - // regions in HeapRegion::oops_on_card_seq_iterate_careful is - // employed. - // As a result, when this closure is applied to "refs into cset" - // DCQS, we shouldn't see any cards in young regions. - update_rs_cl.set_region(r); - HeapWord* stop_point = - r->oops_on_card_seq_iterate_careful(scanRegion, - &filter_then_update_rs_cset_oop_cl, - false /* filter_young */, - NULL /* card_ptr */); - - // Since this is performed in the event of an evacuation failure, we - // we shouldn't see a non-null stop point - assert(stop_point == NULL, "saw an unallocated region"); - return true; - } -}; - void G1RemSet::cleanup_after_oops_into_collection_set_do() { guarantee( _cards_scanned != NULL, "invariant" ); _total_cards_scanned = 0; @@ -465,25 +400,10 @@ void G1RemSet::cleanup_after_oops_into_collection_set_do() { double restore_remembered_set_start = os::elapsedTime(); // Restore remembered sets for the regions pointing into the collection set. - if (G1DeferredRSUpdate) { - // If deferred RS updates are enabled then we just need to transfer - // the completed buffers from (a) the DirtyCardQueueSet used to hold - // cards that contain references that point into the collection set - // to (b) the DCQS used to hold the deferred RS updates - _g1->dirty_card_queue_set().merge_bufferlists(&into_cset_dcqs); - } else { - - CardTableModRefBS* bs = (CardTableModRefBS*)_g1->barrier_set(); - UpdateRSetCardTableEntryIntoCSetClosure update_rs_cset_immediate(_g1, bs); - - int n_completed_buffers = 0; - while (into_cset_dcqs.apply_closure_to_completed_buffer(&update_rs_cset_immediate, - 0, 0, true)) { - n_completed_buffers++; - } - assert(n_completed_buffers == into_cset_n_buffers, "missed some buffers"); - } - + // We just need to transfer the completed buffers from the DirtyCardQueueSet + // used to hold cards that contain references that point into the collection set + // to the DCQS used to hold the deferred RS updates. + _g1->dirty_card_queue_set().merge_bufferlists(&into_cset_dcqs); _g1->g1_policy()->phase_times()->record_evac_fail_restore_remsets((os::elapsedTime() - restore_remembered_set_start) * 1000.0); } diff --git a/src/share/vm/gc_implementation/g1/g1RemSet.hpp b/src/share/vm/gc_implementation/g1/g1RemSet.hpp index 35279a52e201f6e5977b3c7a378c309c37149edb..5a629fad2d06f91384338f59eee6f8e8b2eef55d 100644 --- a/src/share/vm/gc_implementation/g1/g1RemSet.hpp +++ b/src/share/vm/gc_implementation/g1/g1RemSet.hpp @@ -193,18 +193,4 @@ public: bool apply_to_weak_ref_discovered_field() { return true; } }; -class UpdateRSetImmediate: public OopsInHeapRegionClosure { -private: - G1RemSet* _g1_rem_set; - - template void do_oop_work(T* p); -public: - UpdateRSetImmediate(G1RemSet* rs) : - _g1_rem_set(rs) {} - - virtual void do_oop(narrowOop* p) { do_oop_work(p); } - virtual void do_oop( oop* p) { do_oop_work(p); } -}; - - #endif // SHARE_VM_GC_IMPLEMENTATION_G1_G1REMSET_HPP diff --git a/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp b/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp index 949614b28fcf64d71e7b105375f5af3b615ec87c..1afef2fb50111f9a9093f467082f0a91a83baf27 100644 --- a/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp +++ b/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp @@ -79,13 +79,4 @@ inline void UpdateRSOopClosure::do_oop_work(T* p) { _rs->par_write_ref(_from, p, _worker_i); } -template -inline void UpdateRSetImmediate::do_oop_work(T* p) { - assert(_from->is_in_reserved(p), "paranoia"); - T heap_oop = oopDesc::load_heap_oop(p); - if (!oopDesc::is_null(heap_oop) && !_from->is_survivor()) { - _g1_rem_set->par_write_ref(_from, p, 0); - } -} - #endif // SHARE_VM_GC_IMPLEMENTATION_G1_G1REMSET_INLINE_HPP diff --git a/src/share/vm/gc_implementation/g1/g1_globals.hpp b/src/share/vm/gc_implementation/g1/g1_globals.hpp index 8a3ea9d75687cb1aa59cb02e910758e441c89a8c..a2f93f48968528945f3614793f1c5d91c13bb0e3 100644 --- a/src/share/vm/gc_implementation/g1/g1_globals.hpp +++ b/src/share/vm/gc_implementation/g1/g1_globals.hpp @@ -108,9 +108,6 @@ develop(bool, G1RSBarrierRegionFilter, true, \ "If true, generate region filtering code in RS barrier") \ \ - develop(bool, G1DeferredRSUpdate, true, \ - "If true, use deferred RS updates") \ - \ develop(bool, G1RSLogCheckCardTable, false, \ "If true, verify that no dirty cards remain after RS log " \ "processing.") \ diff --git a/test/gc/g1/TestDeferredRSUpdate.java b/test/gc/g1/TestDeferredRSUpdate.java deleted file mode 100644 index f00967d1df45de99fbf16674eccd1af8dd08827e..0000000000000000000000000000000000000000 --- a/test/gc/g1/TestDeferredRSUpdate.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/* - * @test TestDeferredRSUpdate - * @bug 8040977 8052170 - * @summary Ensure that running with -XX:-G1DeferredRSUpdate does not crash the VM - * @key gc - * @library /testlibrary - */ - -import com.oracle.java.testlibrary.ProcessTools; -import com.oracle.java.testlibrary.OutputAnalyzer; - -public class TestDeferredRSUpdate { - public static void main(String[] args) throws Exception { - GCTest.main(args); - - ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-XX:+UseG1GC", - "-Xmx10M", - "-XX:+PrintGCDetails", - // G1DeferredRSUpdate is a develop option, but we cannot limit execution of this test to only debug VMs. - "-XX:+IgnoreUnrecognizedVMOptions", - "-XX:-G1DeferredRSUpdate", - GCTest.class.getName()); - - OutputAnalyzer output = new OutputAnalyzer(pb.start()); - output.shouldHaveExitValue(0); - } - - static class GCTest { - private static Object[] garbage = new Object[32]; - - public static void main(String [] args) { - System.out.println("Creating garbage"); - // Create 128MB of garbage. This should result in at least one minor GC, with - // some objects copied to old gen. As references from old to young are installed, - // the crash due to the use before initialize occurs. - Object prev = null; - Object prevPrev = null; - for (int i = 0; i < 1024; i++) { - Object[] next = new Object[32 * 1024]; - next[0] = prev; - next[1] = prevPrev; - - Object[] cur = (Object[]) garbage[i % garbage.length]; - if (cur != null) { - cur[0] = null; - cur[1] = null; - } - garbage[i % garbage.length] = next; - - prevPrev = prev; - prev = next; - } - System.out.println("Done"); - } - } -}