--- old/src/hotspot/share/gc/g1/g1BarrierSet.cpp 2019-03-08 19:15:51.922657210 -0500 +++ new/src/hotspot/share/gc/g1/g1BarrierSet.cpp 2019-03-08 19:15:51.594639831 -0500 @@ -58,7 +58,8 @@ _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() + _dirty_card_queue_set(), + _shared_dirty_card_queue(&_dirty_card_queue_set) {} void G1BarrierSet::enqueue(oop pre_val) { --- old/src/hotspot/share/gc/g1/g1BarrierSet.hpp 2019-03-08 19:15:52.886708290 -0500 +++ new/src/hotspot/share/gc/g1/g1BarrierSet.hpp 2019-03-08 19:15:52.518688791 -0500 @@ -27,6 +27,7 @@ #include "gc/g1/g1DirtyCardQueue.hpp" #include "gc/g1/g1SATBMarkQueueSet.hpp" +#include "gc/g1/g1SharedDirtyCardQueue.hpp" #include "gc/shared/cardTableBarrierSet.hpp" class CardTable; @@ -42,6 +43,7 @@ BufferNode::Allocator _dirty_card_queue_buffer_allocator; G1SATBMarkQueueSet _satb_mark_queue_set; G1DirtyCardQueueSet _dirty_card_queue_set; + G1SharedDirtyCardQueue _shared_dirty_card_queue; static G1BarrierSet* g1_barrier_set() { return barrier_set_cast(BarrierSet::barrier_set()); @@ -91,6 +93,10 @@ return g1_barrier_set()->_dirty_card_queue_set; } + static G1SharedDirtyCardQueue& shared_dirty_card_queue() { + return g1_barrier_set()->_shared_dirty_card_queue; + } + // Callbacks for runtime accesses. template class AccessBarrier: public ModRefBarrierSet::AccessBarrier { --- old/src/hotspot/share/gc/g1/g1CollectedHeap.cpp 2019-03-08 19:15:53.858759793 -0500 +++ new/src/hotspot/share/gc/g1/g1CollectedHeap.cpp 2019-03-08 19:15:53.470739235 -0500 @@ -1682,12 +1682,10 @@ // later, based on the concurrent refinement object. G1BarrierSet::dirty_card_queue_set().initialize(DirtyCardQ_CBL_mon, &bs->dirty_card_queue_buffer_allocator(), - Shared_DirtyCardQ_lock, true); // init_free_ids dirty_card_queue_set().initialize(DirtyCardQ_CBL_mon, - &bs->dirty_card_queue_buffer_allocator(), - Shared_DirtyCardQ_lock); + &bs->dirty_card_queue_buffer_allocator()); // Create the hot card cache. _hot_card_cache = new G1HotCardCache(this); --- old/src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp 2019-03-08 19:15:54.862812992 -0500 +++ new/src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp 2019-03-08 19:15:54.478792645 -0500 @@ -56,21 +56,18 @@ } }; -G1DirtyCardQueue::G1DirtyCardQueue(G1DirtyCardQueueSet* qset, bool permanent) : +G1DirtyCardQueue::G1DirtyCardQueue(G1DirtyCardQueueSet* qset) : // Dirty card queues are always active, so we create them with their // active field set to true. - PtrQueue(qset, permanent, true /* active */) + PtrQueue(qset, true /* active */) { } G1DirtyCardQueue::~G1DirtyCardQueue() { - if (!is_permanent()) { - flush(); - } + flush(); } G1DirtyCardQueueSet::G1DirtyCardQueueSet(bool notify_when_complete) : PtrQueueSet(notify_when_complete), - _shared_dirty_card_queue(this, true /* permanent */), _free_ids(NULL), _processed_buffers_mut(0), _processed_buffers_rs_thread(0), @@ -90,10 +87,8 @@ void G1DirtyCardQueueSet::initialize(Monitor* cbl_mon, BufferNode::Allocator* allocator, - Mutex* lock, bool init_free_ids) { PtrQueueSet::initialize(cbl_mon, allocator); - _shared_dirty_card_queue.set_lock(lock); if (init_free_ids) { _free_ids = new G1FreeIdSet(0, num_par_ids()); } @@ -217,13 +212,7 @@ } closure; Threads::threads_do(&closure); - shared_dirty_card_queue()->reset(); -} - -void G1DirtyCardQueueSet::concatenate_log(G1DirtyCardQueue& dcq) { - if (!dcq.is_empty()) { - dcq.flush(); - } + G1BarrierSet::shared_dirty_card_queue().reset(); } void G1DirtyCardQueueSet::concatenate_logs() { @@ -234,16 +223,16 @@ size_t old_limit = max_completed_buffers(); set_max_completed_buffers(MaxCompletedBuffersUnlimited); - class ConcatenateThreadLogClosure : public ThreadClosure { - G1DirtyCardQueueSet* _qset; - public: - ConcatenateThreadLogClosure(G1DirtyCardQueueSet* qset) : _qset(qset) {} + struct ConcatenateThreadLogClosure : public ThreadClosure { virtual void do_thread(Thread* t) { - _qset->concatenate_log(G1ThreadLocalData::dirty_card_queue(t)); + G1DirtyCardQueue& dcq = G1ThreadLocalData::dirty_card_queue(t); + if (!dcq.is_empty()) { + dcq.flush(); + } } - } closure(this); + } closure; Threads::threads_do(&closure); - concatenate_log(_shared_dirty_card_queue); + G1BarrierSet::shared_dirty_card_queue().flush(); set_max_completed_buffers(old_limit); } --- old/src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp 2019-03-08 19:15:55.854865556 -0500 +++ new/src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp 2019-03-08 19:15:55.470845209 -0500 @@ -45,7 +45,7 @@ // A ptrQueue whose elements are "oops", pointers to object heads. class G1DirtyCardQueue: public PtrQueue { public: - G1DirtyCardQueue(G1DirtyCardQueueSet* qset, bool permanent = false); + G1DirtyCardQueue(G1DirtyCardQueueSet* qset); // Flush before destroying; queue may be used to capture pending work while // doing something else, with auto-flush on completion. @@ -70,8 +70,6 @@ class G1DirtyCardQueueSet: public PtrQueueSet { - G1DirtyCardQueue _shared_dirty_card_queue; - // Apply the closure to the elements of "node" from it's index to // buffer_size. If all closure applications return true, then // returns true. Stops processing after the first closure @@ -113,15 +111,12 @@ // Current buffer node used for parallel iteration. BufferNode* volatile _cur_par_buffer_node; - void concatenate_log(G1DirtyCardQueue& dcq); - public: G1DirtyCardQueueSet(bool notify_when_complete = true); ~G1DirtyCardQueueSet(); void initialize(Monitor* cbl_mon, BufferNode::Allocator* allocator, - Mutex* lock, bool init_free_ids = false); // The number of parallel ids that can be claimed to allow collector or @@ -144,10 +139,6 @@ // by reset_for_par_iteration. void par_apply_closure_to_all_completed_buffers(G1CardTableEntryClosure* cl); - G1DirtyCardQueue* shared_dirty_card_queue() { - return &_shared_dirty_card_queue; - } - // If a full collection is happening, reset partial logs, and ignore // completed ones: the full collection will make them all irrelevant. void abandon_logs(); --- old/src/hotspot/share/gc/g1/g1RemSet.cpp 2019-03-08 19:15:56.830917271 -0500 +++ new/src/hotspot/share/gc/g1/g1RemSet.cpp 2019-03-08 19:15:56.446896924 -0500 @@ -35,6 +35,7 @@ #include "gc/g1/g1OopClosures.inline.hpp" #include "gc/g1/g1RootClosures.hpp" #include "gc/g1/g1RemSet.hpp" +#include "gc/g1/g1SharedDirtyCardQueue.hpp" #include "gc/g1/heapRegion.inline.hpp" #include "gc/g1/heapRegionManager.inline.hpp" #include "gc/g1/heapRegionRemSet.hpp" @@ -523,9 +524,7 @@ } void G1RemSet::prepare_for_oops_into_collection_set_do() { - G1DirtyCardQueueSet& dcqs = G1BarrierSet::dirty_card_queue_set(); - dcqs.concatenate_logs(); - + G1BarrierSet::dirty_card_queue_set().concatenate_logs(); _scan_state->reset(); } @@ -664,29 +663,30 @@ assert(!dirty_region.is_empty(), "sanity"); G1ConcurrentRefineOopClosure conc_refine_cl(_g1h, worker_i); - - bool card_processed = - r->oops_on_card_seq_iterate_careful(dirty_region, &conc_refine_cl); + if (r->oops_on_card_seq_iterate_careful(dirty_region, &conc_refine_cl)) { + _num_conc_refined_cards++; // Unsynchronized update, only used for logging. + return; + } // If unable to process the card then we encountered an unparsable - // part of the heap (e.g. a partially allocated object) while - // processing a stale card. Despite the card being stale, redirty - // and re-enqueue, because we've already cleaned the card. Without - // this we could incorrectly discard a non-stale card. - if (!card_processed) { - // The card might have gotten re-dirtied and re-enqueued while we - // worked. (In fact, it's pretty likely.) - if (*card_ptr != G1CardTable::dirty_card_val()) { - *card_ptr = G1CardTable::dirty_card_val(); - MutexLockerEx x(Shared_DirtyCardQ_lock, - Mutex::_no_safepoint_check_flag); - G1DirtyCardQueue* sdcq = - G1BarrierSet::dirty_card_queue_set().shared_dirty_card_queue(); - sdcq->enqueue(card_ptr); - } - } else { - _num_conc_refined_cards++; // Unsynchronized update, only used for logging. + // part of the heap (e.g. a partially allocated object, so only + // temporarily a problem) while processing a stale card. Despite + // the card being stale, we can't simply ignore it, because we've + // already marked the card cleaned, so taken responsibility for + // ensuring the card gets scanned. + // + // However, the card might have gotten re-dirtied and re-enqueued + // while we worked. (In fact, it's pretty likely.) + if (*card_ptr == G1CardTable::dirty_card_val()) { + return; } + + // Re-dirty the card and enqueue in the *shared* queue. Can't use + // the thread-local queue, because that might be the queue that is + // being processed by us; we could be a Java thread conscripted to + // perform refinement on our queue's current buffer. + *card_ptr = G1CardTable::dirty_card_val(); + G1BarrierSet::shared_dirty_card_queue().enqueue(card_ptr); } bool G1RemSet::refine_card_during_gc(jbyte* card_ptr, --- old/src/hotspot/share/gc/shared/ptrQueue.cpp 2019-03-08 19:15:57.806968986 -0500 +++ new/src/hotspot/share/gc/shared/ptrQueue.cpp 2019-03-08 19:15:57.426948851 -0500 @@ -36,18 +36,16 @@ #include -PtrQueue::PtrQueue(PtrQueueSet* qset, bool permanent, bool active) : +PtrQueue::PtrQueue(PtrQueueSet* qset, bool active) : _qset(qset), _active(active), - _permanent(permanent), _index(0), _capacity_in_bytes(0), - _buf(NULL), - _lock(NULL) + _buf(NULL) {} PtrQueue::~PtrQueue() { - assert(_permanent || (_buf == NULL), "queue must be flushed before delete"); + assert(_buf == NULL, "queue must be flushed before delete"); } void PtrQueue::flush_impl() { @@ -271,23 +269,13 @@ return; } - if (_lock) { - assert(_lock->owned_by_self(), "Required."); - - BufferNode* node = BufferNode::make_node_from_buffer(_buf, index()); - _buf = NULL; // clear shared _buf field - - qset()->enqueue_completed_buffer(node); - assert(_buf == NULL, "multiple enqueuers appear to be racing"); - } else { - BufferNode* node = BufferNode::make_node_from_buffer(_buf, index()); - if (qset()->process_or_enqueue_completed_buffer(node)) { - // Recycle the buffer. No allocation. - assert(_buf == BufferNode::make_buffer_from_node(node), "invariant"); - assert(capacity() == qset()->buffer_size(), "invariant"); - reset(); - return; - } + BufferNode* node = BufferNode::make_node_from_buffer(_buf, index()); + if (qset()->process_or_enqueue_completed_buffer(node)) { + // Recycle the buffer. No allocation. + assert(_buf == BufferNode::make_buffer_from_node(node), "invariant"); + assert(capacity() == qset()->buffer_size(), "invariant"); + reset(); + return; } } // Set capacity in case this is the first allocation. --- old/src/hotspot/share/gc/shared/ptrQueue.hpp 2019-03-08 19:15:58.767019853 -0500 +++ new/src/hotspot/share/gc/shared/ptrQueue.hpp 2019-03-08 19:15:58.386999718 -0500 @@ -54,11 +54,6 @@ // Whether updates should be logged. bool _active; - // If true, the queue is permanent, and doesn't need to deallocate - // its buffer in the destructor (since that obtains a lock which may not - // be legally locked by then. - const bool _permanent; - // The (byte) index at which an object was last enqueued. Starts at // capacity_in_bytes (indicating an empty buffer) and goes towards zero. // Value is always pointer-size aligned. @@ -111,27 +106,20 @@ return byte_index_to_index(capacity_in_bytes()); } - // If there is a lock associated with this buffer, this is that lock. - Mutex* _lock; - PtrQueueSet* qset() { return _qset; } - bool is_permanent() const { return _permanent; } // Process queue entries and release resources. void flush_impl(); // Initialize this queue to contain a null buffer, and be part of the // given PtrQueueSet. - PtrQueue(PtrQueueSet* qset, bool permanent = false, bool active = false); + PtrQueue(PtrQueueSet* qset, bool active = false); - // Requires queue flushed or permanent. + // Requires queue flushed. ~PtrQueue(); public: - // Associate a lock with a ptr queue. - void set_lock(Mutex* lock) { _lock = lock; } - // Forcibly set empty. void reset() { if (_buf != NULL) { --- old/src/hotspot/share/gc/shared/satbMarkQueue.cpp 2019-03-08 19:15:59.771073052 -0500 +++ new/src/hotspot/share/gc/shared/satbMarkQueue.cpp 2019-03-08 19:15:59.387052705 -0500 @@ -35,14 +35,14 @@ #include "runtime/threadSMR.hpp" #include "runtime/vmThread.hpp" -SATBMarkQueue::SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent) : +SATBMarkQueue::SATBMarkQueue(SATBMarkQueueSet* qset) : // SATB queues are only active during marking cycles. We create // them with their active field set to false. If a thread is // created during a cycle and its SATB queue needs to be activated // before the thread starts running, we'll need to set its active // field to true. This must be done in the collector-specific // BarrierSet thread attachment protocol. - PtrQueue(qset, permanent, false /* active */) + PtrQueue(qset, false /* active */) { } void SATBMarkQueue::flush() { --- old/src/hotspot/share/gc/shared/satbMarkQueue.hpp 2019-03-08 19:16:00.815128370 -0500 +++ new/src/hotspot/share/gc/shared/satbMarkQueue.hpp 2019-03-08 19:16:00.415107175 -0500 @@ -55,7 +55,7 @@ inline void apply_filter(Filter filter_out); public: - SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent = false); + SATBMarkQueue(SATBMarkQueueSet* qset); // Process queue entries and free resources. void flush(); --- /dev/null 2019-03-05 14:29:01.184137278 -0500 +++ new/src/hotspot/share/gc/g1/g1SharedDirtyCardQueue.cpp 2019-03-08 19:16:01.431161010 -0500 @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2019, 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. + * + */ + +#include "precompiled.hpp" +#include "gc/g1/g1DirtyCardQueue.hpp" +#include "gc/g1/g1SharedDirtyCardQueue.hpp" +#include "gc/shared/ptrQueue.hpp" +#include "runtime/mutex.hpp" +#include "runtime/mutexLocker.hpp" + +G1SharedDirtyCardQueue::G1SharedDirtyCardQueue(G1DirtyCardQueueSet* qset) : + _qset(qset), + _buffer(NULL), + _index(0) +{} + +G1SharedDirtyCardQueue::~G1SharedDirtyCardQueue() { + flush(); +} + +void G1SharedDirtyCardQueue::enqueue(void* card_ptr) { + MutexLockerEx ml(Shared_DirtyCardQ_lock, Mutex::_no_safepoint_check_flag); + if (_index == 0) { + flush(); + _buffer = _qset->allocate_buffer(); + _index = _qset->buffer_size(); + assert(_index != 0, "invariant"); + } + _buffer[--_index] = card_ptr; +} + +void G1SharedDirtyCardQueue::flush() { + if (_buffer != NULL) { + BufferNode* node = BufferNode::make_node_from_buffer(_buffer, _index); + _buffer = NULL; + _index = 0; + if (node->index() == _qset->buffer_size()) { + _qset->deallocate_buffer(node); + } else { + _qset->enqueue_completed_buffer(node); + } + } + assert(_index == 0, "invariant"); +} + +void G1SharedDirtyCardQueue::reset() { + if (_buffer == NULL) { + _index = 0; + } else { + _index = _qset->buffer_size(); + } +} --- /dev/null 2019-03-05 14:29:01.184137278 -0500 +++ new/src/hotspot/share/gc/g1/g1SharedDirtyCardQueue.hpp 2019-03-08 19:16:02.395212089 -0500 @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2019, 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. + * + */ + +#ifndef SHARE_GC_G1_G1SHAREDDIRTYCARDQUEUE_HPP +#define SHARE_GC_G1_G1SHAREDDIRTYCARDQUEUE_HPP + +#include "utilities/globalDefinitions.hpp" + +class G1DirtyCardQueueSet; + +class G1SharedDirtyCardQueue { + G1DirtyCardQueueSet* const _qset; + void** _buffer; + size_t _index; + + // Noncopyable + G1SharedDirtyCardQueue(const G1SharedDirtyCardQueue&); + G1SharedDirtyCardQueue& operator=(const G1SharedDirtyCardQueue&); + +public: + G1SharedDirtyCardQueue(G1DirtyCardQueueSet* qset); + ~G1SharedDirtyCardQueue(); // flushes the queue. + + void enqueue(void* card_ptr); + + // Flush any pending entries to the qset and remove the buffer. + // Not thread-safe. + void flush(); + + // Discard any pending entries. + // Not thread-safe. + void reset(); +}; + +#endif // SHARE_GC_G1_G1SHAREDDIRTYCARDQUEUE_HPP