Date: Sat, 30 Mar 2024 07:12:06 +0000 Subject: 8149343: assert(rp->num_q() == no_of_gc_workers) failed: sanity --- .../gc_implementation/g1/g1CollectedHeap.cpp | 20 +++++++++++-------- .../share/vm/memory/referenceProcessor.cpp | 9 +++++++-- .../share/vm/memory/referenceProcessor.hpp | 4 +++- .../TestDynamicNumberOfGCThreads.java | 8 ++++++++ 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index 84d5d4d8..5b156f99 100644 --- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -5462,7 +5462,7 @@ public: _workers(workers), _active_workers(n_workers) { - assert(n_workers > 0, "shouldn't call this otherwise"); + g1h->ref_processor_stw()->set_active_mt_degree(n_workers); } // Executes the given task using concurrent marking worker threads. @@ -5595,7 +5595,9 @@ public: _queues(task_queues), _terminator(workers, _queues), _n_workers(workers) - { } + { + g1h->ref_processor_cm()->set_active_mt_degree(workers); + } void work(uint worker_id) { ResourceMark rm; @@ -5760,8 +5762,10 @@ void G1CollectedHeap::process_discovered_references(uint no_of_gc_workers) { _gc_tracer_stw->gc_id()); } else { // Parallel reference processing - assert(rp->num_q() == no_of_gc_workers, "sanity"); - assert(no_of_gc_workers <= rp->max_num_q(), "sanity"); + assert(no_of_gc_workers <= rp->max_num_q(), + err_msg( + "Mismatch between the number of GC workers %u and the maximum number of Reference process queues %u", + no_of_gc_workers, rp->max_num_q())); G1STWRefProcTaskExecutor par_task_executor(this, workers(), _task_queues, no_of_gc_workers); stats = rp->process_discovered_references(&is_alive, @@ -5796,10 +5800,10 @@ void G1CollectedHeap::enqueue_discovered_references(uint no_of_gc_workers) { } else { // Parallel reference enqueueing - assert(no_of_gc_workers == workers()->active_workers(), - "Need to reset active workers"); - assert(rp->num_q() == no_of_gc_workers, "sanity"); - assert(no_of_gc_workers <= rp->max_num_q(), "sanity"); + assert(no_of_gc_workers <= rp->max_num_q(), + err_msg( + "Mismatch between the number of GC workers %u and the maximum number of Reference process queues %u", + no_of_gc_workers, rp->max_num_q())); G1STWRefProcTaskExecutor par_task_executor(this, workers(), _task_queues, no_of_gc_workers); rp->enqueue_discovered_references(&par_task_executor); diff --git a/hotspot/src/share/vm/memory/referenceProcessor.cpp b/hotspot/src/share/vm/memory/referenceProcessor.cpp index b916e696..823fd49c 100644 --- a/hotspot/src/share/vm/memory/referenceProcessor.cpp +++ b/hotspot/src/share/vm/memory/referenceProcessor.cpp @@ -136,7 +136,7 @@ void ReferenceProcessor::verify_no_references_recorded() { guarantee(!_discovering_refs, "Discovering refs?"); for (uint i = 0; i < _max_num_q * number_of_subclasses_of_ref(); i++) { guarantee(_discovered_refs[i].is_empty(), - "Found non-empty discovered list"); + err_msg("Found non-empty discovered list at %u", i)); } } #endif @@ -780,6 +780,11 @@ private: bool _clear_referent; }; +void ReferenceProcessor::set_active_mt_degree(uint v) { + _num_q = v; + _next_id = 0; +} + // Balances reference queues. // Move entries from all queues[0, 1, ..., _max_num_q-1] to // queues[0, 1, ..., _num_q-1] because only the first _num_q @@ -862,7 +867,7 @@ void ReferenceProcessor::balance_queues(DiscoveredList ref_lists[]) } #ifdef ASSERT size_t balanced_total_refs = 0; - for (uint i = 0; i < _max_num_q; ++i) { + for (uint i = 0; i < _num_q; ++i) { balanced_total_refs += ref_lists[i].length(); if (TraceReferenceGC && PrintGCDetails) { gclog_or_tty->print("%d ", ref_lists[i].length()); diff --git a/hotspot/src/share/vm/memory/referenceProcessor.hpp b/hotspot/src/share/vm/memory/referenceProcessor.hpp index 470503ee..da148a6c 100644 --- a/hotspot/src/share/vm/memory/referenceProcessor.hpp +++ b/hotspot/src/share/vm/memory/referenceProcessor.hpp @@ -270,7 +270,7 @@ class ReferenceProcessor : public CHeapObj { uint num_q() { return _num_q; } uint max_num_q() { return _max_num_q; } - void set_active_mt_degree(uint v) { _num_q = v; } + void set_active_mt_degree(uint v); DiscoveredList* discovered_refs() { return _discovered_refs; } @@ -385,9 +385,11 @@ class ReferenceProcessor : public CHeapObj { // round-robin mod _num_q (not: _not_ mode _max_num_q) uint next_id() { uint id = _next_id; + assert(!_discovery_is_mt, "Round robin should only be used in serial discovery"); if (++_next_id == _num_q) { _next_id = 0; } + assert(_next_id < _num_q, err_msg("_next_id %u _num_q %u _max_num_q %u", _next_id, _num_q, _max_num_q)); return id; } DiscoveredList* get_discovered_list(ReferenceType rt); diff --git a/hotspot/test/gc/ergonomics/TestDynamicNumberOfGCThreads.java b/hotspot/test/gc/ergonomics/TestDynamicNumberOfGCThreads.java index f4a6625a..2005a67e 100644 --- a/hotspot/test/gc/ergonomics/TestDynamicNumberOfGCThreads.java +++ b/hotspot/test/gc/ergonomics/TestDynamicNumberOfGCThreads.java @@ -63,6 +63,14 @@ public class TestDynamicNumberOfGCThreads { System.arraycopy(baseArgs, 0, finalArgs, extraArgs.length, baseArgs.length); pb_enabled = ProcessTools.createJavaProcessBuilder(finalArgs); verifyDynamicNumberOfGCThreads(new OutputAnalyzer(pb_enabled.start())); + + // Turn on parallel reference processing + String[] parRefProcArg = {"-XX:+ParallelRefProcEnabled", "-XX:-ShowMessageBoxOnError"}; + String[] parRefArgs = new String[baseArgs.length + parRefProcArg.length]; + System.arraycopy(parRefProcArg, 0, parRefArgs, 0, parRefProcArg.length); + System.arraycopy(baseArgs, 0, parRefArgs, parRefProcArg.length, baseArgs.length); + pb_enabled = ProcessTools.createJavaProcessBuilder(parRefArgs); + verifyDynamicNumberOfGCThreads(new OutputAnalyzer(pb_enabled.start())); } static class GCTest { -- 2.17.1