# HG changeset patch # User kbarrett # Date 1568062488 14400 # Node ID baa4dd528de0d5bd9713211f0791c541214acf09 # Parent b553ad95acf0714232ebac48c9886a7c302268fe 8221361: Eliminate two-phase initialization for PtrQueueSet classes Summary: Move allocator and CBL monitor init to constructor. Reviewed-by: tschatzl, shade diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1BarrierSet.cpp --- a/src/hotspot/share/gc/g1/g1BarrierSet.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1BarrierSet.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -57,8 +57,8 @@ BarrierSet::FakeRtti(BarrierSet::G1BarrierSet)), _satb_mark_queue_buffer_allocator("SATB Buffer Allocator", G1SATBBufferSize), _dirty_card_queue_buffer_allocator("DC Buffer Allocator", G1UpdateBufferSize), - _satb_mark_queue_set(), - _dirty_card_queue_set(), + _satb_mark_queue_set(&_satb_mark_queue_buffer_allocator), + _dirty_card_queue_set(DirtyCardQ_CBL_mon, &_dirty_card_queue_buffer_allocator), _shared_dirty_card_queue(&_dirty_card_queue_set) {} @@ -159,11 +159,3 @@ G1ThreadLocalData::satb_mark_queue(thread).flush(); G1ThreadLocalData::dirty_card_queue(thread).flush(); } - -BufferNode::Allocator& G1BarrierSet::satb_mark_queue_buffer_allocator() { - return _satb_mark_queue_buffer_allocator; -} - -BufferNode::Allocator& G1BarrierSet::dirty_card_queue_buffer_allocator() { - return _dirty_card_queue_buffer_allocator; -} diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1BarrierSet.hpp --- a/src/hotspot/share/gc/g1/g1BarrierSet.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1BarrierSet.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -82,9 +82,6 @@ virtual void on_thread_attach(Thread* thread); virtual void on_thread_detach(Thread* thread); - BufferNode::Allocator& satb_mark_queue_buffer_allocator(); - BufferNode::Allocator& dirty_card_queue_buffer_allocator(); - static G1SATBMarkQueueSet& satb_mark_queue_set() { return g1_barrier_set()->_satb_mark_queue_set; } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1CollectedHeap.cpp --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -1678,15 +1678,11 @@ BarrierSet::set_barrier_set(bs); _card_table = ct; - G1BarrierSet::satb_mark_queue_set().initialize(this, - &bs->satb_mark_queue_buffer_allocator(), - G1SATBProcessCompletedThreshold, - G1SATBBufferEnqueueingThresholdPercent); - - // process_cards_threshold and max_cards are updated - // later, based on the concurrent refinement object. - G1BarrierSet::dirty_card_queue_set().initialize(DirtyCardQ_CBL_mon, - &bs->dirty_card_queue_buffer_allocator()); + { + G1SATBMarkQueueSet& satbqs = bs->satb_mark_queue_set(); + satbqs.set_process_completed_buffers_threshold(G1SATBProcessCompletedThreshold); + satbqs.set_buffer_enqueue_threshold_percentage(G1SATBBufferEnqueueingThresholdPercent); + } // Create the hot card cache. _hot_card_cache = new G1HotCardCache(this); diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp --- a/src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -62,9 +62,10 @@ } } -G1DirtyCardQueueSet::G1DirtyCardQueueSet() : - PtrQueueSet(), - _cbl_mon(NULL), +G1DirtyCardQueueSet::G1DirtyCardQueueSet(Monitor* cbl_mon, + BufferNode::Allocator* allocator) : + PtrQueueSet(allocator), + _cbl_mon(cbl_mon), _completed_buffers_head(NULL), _completed_buffers_tail(NULL), _num_cards(0), @@ -88,13 +89,6 @@ return (uint)os::initial_active_processor_count(); } -void G1DirtyCardQueueSet::initialize(Monitor* cbl_mon, - BufferNode::Allocator* allocator) { - PtrQueueSet::initialize(allocator); - assert(_cbl_mon == NULL, "Init order issue?"); - _cbl_mon = cbl_mon; -} - void G1DirtyCardQueueSet::handle_zero_index_for_thread(Thread* t) { G1ThreadLocalData::dirty_card_queue(t).handle_zero_index(); } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp --- a/src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -103,11 +103,9 @@ jint _processed_buffers_rs_thread; public: - G1DirtyCardQueueSet(); + G1DirtyCardQueueSet(Monitor* cbl_mon, BufferNode::Allocator* allocator); ~G1DirtyCardQueueSet(); - void initialize(Monitor* cbl_mon, BufferNode::Allocator* allocator); - // The number of parallel ids that can be claimed to allow collector or // mutator threads to do card-processing work. static uint num_par_ids(); diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1RedirtyCardsQueue.cpp --- a/src/hotspot/share/gc/g1/g1RedirtyCardsQueue.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1RedirtyCardsQueue.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -31,12 +31,10 @@ // G1RedirtyCardsQueueBase::LocalQSet G1RedirtyCardsQueueBase::LocalQSet::LocalQSet(G1RedirtyCardsQueueSet* shared_qset) : - PtrQueueSet(), + PtrQueueSet(shared_qset->allocator()), _shared_qset(shared_qset), _buffers() -{ - PtrQueueSet::initialize(_shared_qset->allocator()); -} +{} G1RedirtyCardsQueueBase::LocalQSet::~LocalQSet() { assert(_buffers._head == NULL, "unflushed qset"); @@ -86,14 +84,12 @@ // G1RedirtyCardsQueueSet G1RedirtyCardsQueueSet::G1RedirtyCardsQueueSet(BufferNode::Allocator* allocator) : - PtrQueueSet(), + PtrQueueSet(allocator), _list(), _entry_count(0), _tail(NULL) DEBUG_ONLY(COMMA _collecting(true)) -{ - initialize(allocator); -} +{} G1RedirtyCardsQueueSet::~G1RedirtyCardsQueueSet() { verify_empty(); diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp --- a/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -32,17 +32,9 @@ #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" -G1SATBMarkQueueSet::G1SATBMarkQueueSet() : _g1h(NULL) {} - -void G1SATBMarkQueueSet::initialize(G1CollectedHeap* g1h, - BufferNode::Allocator* allocator, - size_t process_completed_buffers_threshold, - uint buffer_enqueue_threshold_percentage) { - SATBMarkQueueSet::initialize(allocator, - process_completed_buffers_threshold, - buffer_enqueue_threshold_percentage); - _g1h = g1h; -} +G1SATBMarkQueueSet::G1SATBMarkQueueSet(BufferNode::Allocator* allocator) : + SATBMarkQueueSet(allocator) +{} void G1SATBMarkQueueSet::handle_zero_index_for_thread(Thread* t) { G1ThreadLocalData::satb_mark_queue(t).handle_zero_index(); @@ -112,7 +104,7 @@ G1CollectedHeap* _g1h; public: - G1SATBMarkQueueFilterFn(G1CollectedHeap* g1h) : _g1h(g1h) {} + G1SATBMarkQueueFilterFn() : _g1h(G1CollectedHeap::heap()) {} // Return true if entry should be filtered out (removed), false if // it should be retained. @@ -122,6 +114,5 @@ }; void G1SATBMarkQueueSet::filter(SATBMarkQueue* queue) { - assert(_g1h != NULL, "SATB queue set not initialized"); - apply_filter(G1SATBMarkQueueFilterFn(_g1h), queue); + apply_filter(G1SATBMarkQueueFilterFn(), queue); } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/g1/g1SATBMarkQueueSet.hpp --- a/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -35,12 +35,7 @@ G1CollectedHeap* _g1h; public: - G1SATBMarkQueueSet(); - - void initialize(G1CollectedHeap* g1h, - BufferNode::Allocator* allocator, - size_t process_completed_buffers_threshold, - uint buffer_enqueue_threshold_percentage); + G1SATBMarkQueueSet(BufferNode::Allocator* allocator); static void handle_zero_index_for_thread(Thread* t); virtual SATBMarkQueue& satb_queue_for_thread(Thread* const t) const; diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shared/ptrQueue.cpp --- a/src/hotspot/share/gc/shared/ptrQueue.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shared/ptrQueue.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -40,7 +40,7 @@ _qset(qset), _active(active), _index(0), - _capacity_in_bytes(0), + _capacity_in_bytes(index_to_byte_index(qset->buffer_size())), _buf(NULL) {} @@ -80,13 +80,6 @@ if (_buf != NULL) { handle_completed_buffer(); } else { - // Bootstrapping kludge; lazily initialize capacity. The initial - // thread's queues are constructed before the second phase of the - // two-phase initialization of the associated qsets. As a result, - // we can't initialize _capacity_in_bytes in the queue constructor. - if (_capacity_in_bytes == 0) { - _capacity_in_bytes = index_to_byte_index(qset()->buffer_size()); - } allocate_buffer(); } } @@ -250,18 +243,13 @@ return removed; } -PtrQueueSet::PtrQueueSet() : - _allocator(NULL), +PtrQueueSet::PtrQueueSet(BufferNode::Allocator* allocator) : + _allocator(allocator), _all_active(false) {} PtrQueueSet::~PtrQueueSet() {} -void PtrQueueSet::initialize(BufferNode::Allocator* allocator) { - assert(allocator != NULL, "Init order issue?"); - _allocator = allocator; -} - void** PtrQueueSet::allocate_buffer() { BufferNode* node = _allocator->allocate(); return BufferNode::make_buffer_from_node(node); diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shared/ptrQueue.hpp --- a/src/hotspot/share/gc/shared/ptrQueue.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shared/ptrQueue.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -303,13 +303,9 @@ bool _all_active; // Create an empty ptr queue set. - PtrQueueSet(); + PtrQueueSet(BufferNode::Allocator* allocator); ~PtrQueueSet(); - // Because of init-order concerns, we can't pass these as constructor - // arguments. - void initialize(BufferNode::Allocator* allocator); - public: // Return the associated BufferNode allocator. diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shared/satbMarkQueue.cpp --- a/src/hotspot/share/gc/shared/satbMarkQueue.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shared/satbMarkQueue.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -108,8 +108,8 @@ #endif // PRODUCT -SATBMarkQueueSet::SATBMarkQueueSet() : - PtrQueueSet(), +SATBMarkQueueSet::SATBMarkQueueSet(BufferNode::Allocator* allocator) : + PtrQueueSet(allocator), _list(), _count_and_process_flag(0), _process_completed_buffers_threshold(SIZE_MAX), @@ -153,27 +153,21 @@ } while (value != old); } -// Scale requested threshold to align with count field. If scaling -// overflows, just use max value. Set process flag field to make -// comparison in increment_count exact. -static size_t scale_threshold(size_t value) { +void SATBMarkQueueSet::set_process_completed_buffers_threshold(size_t value) { + // Scale requested threshold to align with count field. If scaling + // overflows, just use max value. Set process flag field to make + // comparison in increment_count exact. size_t scaled_value = value << 1; if ((scaled_value >> 1) != value) { scaled_value = SIZE_MAX; } - return scaled_value | 1; + _process_completed_buffers_threshold = scaled_value | 1; } -void SATBMarkQueueSet::initialize(BufferNode::Allocator* allocator, - size_t process_completed_buffers_threshold, - uint buffer_enqueue_threshold_percentage) { - PtrQueueSet::initialize(allocator); - _process_completed_buffers_threshold = - scale_threshold(process_completed_buffers_threshold); - assert(buffer_size() != 0, "buffer size not initialized"); +void SATBMarkQueueSet::set_buffer_enqueue_threshold_percentage(uint value) { // Minimum threshold of 1 ensures enqueuing of completely full buffers. size_t size = buffer_size(); - size_t enqueue_qty = (size * buffer_enqueue_threshold_percentage) / 100; + size_t enqueue_qty = (size * value) / 100; _buffer_enqueue_threshold = MAX2(size - enqueue_qty, (size_t)1); } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shared/satbMarkQueue.hpp --- a/src/hotspot/share/gc/shared/satbMarkQueue.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shared/satbMarkQueue.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -112,7 +112,7 @@ #endif // ASSERT protected: - SATBMarkQueueSet(); + SATBMarkQueueSet(BufferNode::Allocator* allocator); ~SATBMarkQueueSet(); template @@ -120,10 +120,6 @@ queue->apply_filter(filter); } - void initialize(BufferNode::Allocator* allocator, - size_t process_completed_buffers_threshold, - uint buffer_enqueue_threshold_percentage); - public: virtual SATBMarkQueue& satb_queue_for_thread(Thread* const t) const = 0; @@ -133,7 +129,11 @@ // set itself, has an active value same as expected_active. void set_active_all_threads(bool active, bool expected_active); + void set_process_completed_buffers_threshold(size_t value); + size_t buffer_enqueue_threshold() const { return _buffer_enqueue_threshold; } + void set_buffer_enqueue_threshold_percentage(uint value); + virtual void filter(SATBMarkQueue* queue) = 0; // If there exists some completed buffer, pop and process it, and diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -75,7 +75,8 @@ NULL /* barrier_set_nmethod */, BarrierSet::FakeRtti(BarrierSet::ShenandoahBarrierSet)), _heap(heap), - _satb_mark_queue_set() + _satb_mark_queue_buffer_allocator("SATB Buffer Allocator", ShenandoahSATBBufferSize), + _satb_mark_queue_set(&_satb_mark_queue_buffer_allocator) { } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -41,6 +41,7 @@ private: ShenandoahHeap* _heap; + BufferNode::Allocator _satb_mark_queue_buffer_allocator; ShenandoahSATBMarkQueueSet _satb_mark_queue_set; public: diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -343,11 +343,13 @@ Copy::fill_to_bytes(_liveness_cache[worker], _num_regions * sizeof(jushort)); } - // The call below uses stuff (the SATB* things) that are in G1, but probably - // belong into a shared location. - ShenandoahBarrierSet::satb_mark_queue_set().initialize(this, - 20 /* G1SATBProcessCompletedThreshold */, - 60 /* G1SATBBufferEnqueueingThresholdPercent */); + // There should probably be Shenandoah-specific options for these, + // just as there are G1-specific options. + { + ShenandoahSATBMarkQueueSet& satbqs = ShenandoahBarrierSet::satb_mark_queue_set(); + satbqs.set_process_completed_buffers_threshold(20); // G1SATBProcessCompletedThreshold + satbqs.set_buffer_enqueue_threshold_percentage(60); // G1SATBBufferEnqueueingThresholdPercent + } _monitoring_support = new ShenandoahMonitoringSupport(this); _phase_timings = new ShenandoahPhaseTimings(); diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.cpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.cpp Mon Sep 09 16:54:48 2019 -0400 @@ -27,20 +27,10 @@ #include "gc/shenandoah/shenandoahSATBMarkQueueSet.hpp" #include "gc/shenandoah/shenandoahThreadLocalData.hpp" -ShenandoahSATBMarkQueueSet::ShenandoahSATBMarkQueueSet() : - _heap(NULL), - _satb_mark_queue_buffer_allocator("SATB Buffer Allocator", ShenandoahSATBBufferSize) +ShenandoahSATBMarkQueueSet::ShenandoahSATBMarkQueueSet(BufferNode::Allocator* allocator) : + SATBMarkQueueSet(allocator) {} -void ShenandoahSATBMarkQueueSet::initialize(ShenandoahHeap* const heap, - int process_completed_threshold, - uint buffer_enqueue_threshold_percentage) { - SATBMarkQueueSet::initialize(&_satb_mark_queue_buffer_allocator, - process_completed_threshold, - buffer_enqueue_threshold_percentage); - _heap = heap; -} - SATBMarkQueue& ShenandoahSATBMarkQueueSet::satb_queue_for_thread(Thread* const t) const { return ShenandoahThreadLocalData::satb_mark_queue(t); } @@ -60,11 +50,11 @@ }; void ShenandoahSATBMarkQueueSet::filter(SATBMarkQueue* queue) { - assert(_heap != NULL, "SATB queue set not initialized"); - if (_heap->has_forwarded_objects()) { - apply_filter(ShenandoahSATBMarkQueueFilterFn(_heap), queue); + ShenandoahHeap* heap = ShenandoahHeap::heap(); + if (heap->has_forwarded_objects()) { + apply_filter(ShenandoahSATBMarkQueueFilterFn(heap), queue); } else { - apply_filter(ShenandoahSATBMarkQueueFilterFn(_heap), queue); + apply_filter(ShenandoahSATBMarkQueueFilterFn(heap), queue); } } diff -r b553ad95acf0 -r baa4dd528de0 src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp Mon Sep 09 12:42:01 2019 -0700 +++ b/src/hotspot/share/gc/shenandoah/shenandoahSATBMarkQueueSet.hpp Mon Sep 09 16:54:48 2019 -0400 @@ -37,15 +37,8 @@ }; class ShenandoahSATBMarkQueueSet : public SATBMarkQueueSet { -private: - ShenandoahHeap* _heap; - BufferNode::Allocator _satb_mark_queue_buffer_allocator; public: - ShenandoahSATBMarkQueueSet(); - - void initialize(ShenandoahHeap* const heap, - int process_completed_threshold, - uint buffer_enqueue_threshold_percentage); + ShenandoahSATBMarkQueueSet(BufferNode::Allocator* allocator); virtual SATBMarkQueue& satb_queue_for_thread(Thread* const t) const; virtual void filter(SATBMarkQueue* queue);