diff --git a/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp b/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp index 0a7354b17be..43bc1fcd53e 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp @@ -125,15 +125,6 @@ void ShenandoahDegenGC::op_degenerated() { // we can do the most aggressive degen cycle, which includes processing references and // class unloading, unless those features are explicitly disabled. - if (heap->is_concurrent_old_mark_in_progress()) { - // We have come straight into a degenerated cycle without running a concurrent cycle - // first and the SATB barrier is enabled to support concurrent old marking. The SATB buffer - // may hold a mix of old and young pointers. The old pointers need to be transferred - // to the old generation mark queues and the young pointers are _not_ part of this - // snapshot, so they must be dropped here. - heap->transfer_old_pointers_from_satb(); - } - // Note that we can only do this for "outside-cycle" degens, otherwise we would risk // changing the cycle parameters mid-cycle during concurrent -> degenerated handover. heap->set_unload_classes(_generation->heuristics()->can_unload_classes() && @@ -156,9 +147,20 @@ void ShenandoahDegenGC::op_degenerated() { if (_generation->is_concurrent_mark_in_progress()) { // We want to allow old generation marking to be punctuated by young collections // (even if they have degenerated). If this is a global cycle, we'd have cancelled - // the entire old gc before coming into this switch. + // the entire old gc before coming into this switch. Note that cancel_marking on + // the generation does NOT abandon incomplete SATB buffers as cancel_concurrent_mark does. + // We need to separate out the old pointers which is done below. _generation->cancel_marking(); } + + if (heap->is_concurrent_mark_in_progress()) { + // If either old or young marking is in progress, the SATB barrier will be enabled. + // The SATB buffer may hold a mix of old and young pointers. The old pointers need to be + // transferred to the old generation mark queues and the young pointers are NOT part + // of this snapshot, so they must be dropped here. It is safe to drop them here because + // we will rescan the roots on this safepoint. + heap->transfer_old_pointers_from_satb(); + } } if (_degen_point == ShenandoahDegenPoint::_degenerated_roots) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp index 1ac7e22c41a..484804afddc 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp @@ -65,7 +65,7 @@ class ShenandoahOldGeneration : public ShenandoahGeneration { // We leave the SATB barrier on for the entirety of the old generation // marking phase. In some cases, this can cause a write to a perfectly // reachable oop to enqueue a pointer that later becomes garbage (because - // it points at an object in the collection set, for example). There are + // it points at an object that is later chosen for the collection set). There are // also cases where the referent of a weak reference ends up in the SATB // and is later collected. In these cases the oop in the SATB buffer becomes // invalid and the _next_ cycle will crash during its marking phase. To diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp index 61ea96999d0..2ff38dfa34a 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp @@ -87,6 +87,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure { _loc(nullptr), _generation(nullptr) { if (options._verify_marked == ShenandoahVerifier::_verify_marked_complete_except_references || + options._verify_marked == ShenandoahVerifier::_verify_marked_complete_satb_empty || options._verify_marked == ShenandoahVerifier::_verify_marked_disable) { set_ref_discoverer_internal(new ShenandoahIgnoreReferenceDiscoverer()); } @@ -257,6 +258,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure { "Must be marked in complete bitmap"); break; case ShenandoahVerifier::_verify_marked_complete_except_references: + case ShenandoahVerifier::_verify_marked_complete_satb_empty: check(ShenandoahAsserts::_safe_all, obj, _heap->complete_marking_context()->is_marked_or_old(obj), "Must be marked in complete bitmap, except j.l.r.Reference referents"); break; @@ -597,6 +599,20 @@ class ShenandoahVerifierReachableTask : public WorkerTask { } }; +class ShenandoahVerifyNoIncompleteSatbBuffers : public ThreadClosure { +public: + virtual void do_thread(Thread* thread) { + SATBMarkQueue& queue = ShenandoahThreadLocalData::satb_mark_queue(thread); + if (!is_empty(queue)) { + fatal("All SATB buffers should have been flushed during mark"); + } + } +private: + bool is_empty(SATBMarkQueue& queue) { + return queue.buffer() == nullptr || queue.index() == queue.capacity(); + } +}; + class ShenandoahVerifierMarkedRegionTask : public WorkerTask { private: const char* _label; @@ -622,6 +638,10 @@ class ShenandoahVerifierMarkedRegionTask : public WorkerTask { _claimed(0), _processed(0), _generation(nullptr) { + if (_options._verify_marked == ShenandoahVerifier::_verify_marked_complete_satb_empty) { + Threads::change_thread_claim_token(); + } + if (_heap->mode()->is_generational()) { _generation = _heap->active_generation(); assert(_generation != nullptr, "Expected active generation in this mode."); @@ -633,6 +653,11 @@ class ShenandoahVerifierMarkedRegionTask : public WorkerTask { } virtual void work(uint worker_id) { + if (_options._verify_marked == ShenandoahVerifier::_verify_marked_complete_satb_empty) { + ShenandoahVerifyNoIncompleteSatbBuffers verify_satb; + Threads::possibly_parallel_threads_do(true, &verify_satb); + } + ShenandoahVerifierStack stack; ShenandoahVerifyOopClosure cl(&stack, _bitmap, _ld, ShenandoahMessageBuffer("%s, Marked", _label), @@ -945,7 +970,10 @@ void ShenandoahVerifier::verify_at_safepoint(const char* label, // version size_t count_marked = 0; - if (ShenandoahVerifyLevel >= 4 && (marked == _verify_marked_complete || marked == _verify_marked_complete_except_references)) { + if (ShenandoahVerifyLevel >= 4 && + (marked == _verify_marked_complete || + marked == _verify_marked_complete_except_references || + marked == _verify_marked_complete_satb_empty)) { guarantee(_heap->marking_context()->is_complete(), "Marking context should be complete"); ShenandoahVerifierMarkedRegionTask task(_verification_bit_map, ld, label, options); _heap->workers()->run_task(&task); @@ -1031,7 +1059,7 @@ void ShenandoahVerifier::verify_after_concmark() { "After Mark", _verify_remembered_disable, // do not verify remembered set _verify_forwarded_none, // no forwarded references - _verify_marked_complete_except_references, + _verify_marked_complete_satb_empty, // bitmaps as precise as we can get, except dangling j.l.r.Refs _verify_cset_none, // no references to cset anymore _verify_liveness_complete, // liveness data must be complete here diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.hpp b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.hpp index 67b35644bf5..6fbdd8515ed 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.hpp @@ -88,7 +88,12 @@ class ShenandoahVerifier : public CHeapObj { // Objects should be marked in "complete" bitmap, except j.l.r.Reference referents, which // may be dangling after marking but before conc-weakrefs-processing. - _verify_marked_complete_except_references + _verify_marked_complete_except_references, + + // Objects should be marked in "complete" bitmap, except j.l.r.Reference referents, which + // may be dangling after marking but before conc-weakrefs-processing. All SATB buffers must + // be empty. + _verify_marked_complete_satb_empty, } VerifyMarked; typedef enum {