--- old/src/share/vm/gc/g1/dirtyCardQueue.cpp 2015-10-30 19:31:39.055586851 -0400 +++ new/src/share/vm/gc/g1/dirtyCardQueue.cpp 2015-10-30 19:31:38.967586414 -0400 @@ -32,6 +32,18 @@ #include "runtime/safepoint.hpp" #include "runtime/thread.inline.hpp" +DirtyCardQueue::DirtyCardQueue(DirtyCardQueueSet* qset, bool permanent) : + // Dirty card queues are always active, so we create them with their + // active field set to true. + PtrQueue(qset, permanent, true /* active */) +{ } + +DirtyCardQueue::~DirtyCardQueue() { + if (!is_permanent()) { + flush(); + } +} + bool DirtyCardQueue::apply_closure(CardTableEntryClosure* cl, bool consume, uint worker_i) { @@ -51,13 +63,13 @@ bool consume, uint worker_i) { if (cl == NULL) return true; - for (size_t i = index; i < sz; i += oopSize) { - int ind = byte_index_to_index((int)i); - jbyte* card_ptr = (jbyte*)buf[ind]; + size_t limit = byte_index_to_index(sz); + for (size_t i = byte_index_to_index(index); i < limit; ++i) { + jbyte* card_ptr = static_cast(buf[i]); if (card_ptr != NULL) { // Set the entry to null, so we don't do it again (via the test // above) if we reconsider this buffer. - if (consume) buf[ind] = NULL; + if (consume) buf[i] = NULL; if (!cl->do_card_ptr(card_ptr, worker_i)) return false; } } @@ -83,13 +95,19 @@ return (uint)os::processor_count(); } -void DirtyCardQueueSet::initialize(CardTableEntryClosure* cl, Monitor* cbl_mon, Mutex* fl_lock, +void DirtyCardQueueSet::initialize(CardTableEntryClosure* cl, + Monitor* cbl_mon, + Mutex* fl_lock, int process_completed_threshold, int max_completed_queue, - Mutex* lock, PtrQueueSet* fl_owner) { + Mutex* lock, + DirtyCardQueueSet* fl_owner) { _mut_process_closure = cl; - PtrQueueSet::initialize(cbl_mon, fl_lock, process_completed_threshold, - max_completed_queue, fl_owner); + PtrQueueSet::initialize(cbl_mon, + fl_lock, + process_completed_threshold, + max_completed_queue, + fl_owner); set_buffer_size(G1UpdateBufferSize); _shared_dirty_card_queue.set_lock(lock); _free_ids = new FreeIdSet((int) num_par_ids(), _cbl_mon); @@ -103,7 +121,7 @@ bool consume, uint worker_i) { assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint."); - for(JavaThread* t = Threads::first(); t; t = t->next()) { + for (JavaThread* t = Threads::first(); t; t = t->next()) { bool b = t->dirty_card_queue().apply_closure(cl, consume); guarantee(b, "Should not be interrupted."); } @@ -178,7 +196,7 @@ _n_completed_buffers--; assert(_n_completed_buffers >= 0, "Invariant"); } - debug_only(assert_completed_buffer_list_len_correct_locked()); + DEBUG_ONLY(assert_completed_buffer_list_len_correct_locked()); return nd; } @@ -259,7 +277,7 @@ } _n_completed_buffers = 0; _completed_buffers_tail = NULL; - debug_only(assert_completed_buffer_list_len_correct_locked()); + DEBUG_ONLY(assert_completed_buffer_list_len_correct_locked()); } while (buffers_to_delete != NULL) { BufferNode* nd = buffers_to_delete; @@ -291,10 +309,11 @@ for (JavaThread* t = Threads::first(); t; t = t->next()) { DirtyCardQueue& dcq = t->dirty_card_queue(); if (dcq.size() != 0) { - void **buf = t->dirty_card_queue().get_buf(); + void** buf = dcq.get_buf(); // We must NULL out the unused entries, then enqueue. - for (size_t i = 0; i < t->dirty_card_queue().get_index(); i += oopSize) { - buf[PtrQueue::byte_index_to_index((int)i)] = NULL; + size_t limit = dcq.byte_index_to_index(dcq.get_index()); + for (size_t i = 0; i < limit; ++i) { + buf[i] = NULL; } enqueue_complete_buffer(dcq.get_buf(), dcq.get_index()); dcq.reinitialize(); --- old/src/share/vm/gc/g1/dirtyCardQueue.hpp 2015-10-30 19:31:39.571589409 -0400 +++ new/src/share/vm/gc/g1/dirtyCardQueue.hpp 2015-10-30 19:31:39.483588973 -0400 @@ -29,6 +29,7 @@ #include "memory/allocation.hpp" class FreeIdSet; +class DirtyCardQueueSet; // A closure class for processing card table entries. Note that we don't // require these closure objects to be stack-allocated. @@ -42,14 +43,11 @@ // A ptrQueue whose elements are "oops", pointers to object heads. class DirtyCardQueue: public PtrQueue { public: - DirtyCardQueue(PtrQueueSet* qset_, bool perm = false) : - // Dirty card queues are always active, so we create them with their - // active field set to true. - PtrQueue(qset_, perm, true /* active */) { } + DirtyCardQueue(DirtyCardQueueSet* qset, bool permanent = false); // Flush before destroying; queue may be used to capture pending work while // doing something else, with auto-flush on completion. - ~DirtyCardQueue() { if (!is_permanent()) flush(); } + ~DirtyCardQueue(); // Process queue entries and release resources. void flush() { flush_impl(); } @@ -72,7 +70,6 @@ bool consume = true, uint worker_i = 0); void **get_buf() { return _buf;} - void set_buf(void **buf) {_buf = buf;} size_t get_index() { return _index;} void reinitialize() { _buf = 0; _sz = 0; _index = 0;} }; @@ -101,10 +98,13 @@ public: DirtyCardQueueSet(bool notify_when_complete = true); - void initialize(CardTableEntryClosure* cl, Monitor* cbl_mon, Mutex* fl_lock, + void initialize(CardTableEntryClosure* cl, + Monitor* cbl_mon, + Mutex* fl_lock, int process_completed_threshold, int max_completed_queue, - Mutex* lock, PtrQueueSet* fl_owner = NULL); + Mutex* lock, + DirtyCardQueueSet* fl_owner = NULL); // The number of parallel ids that can be claimed to allow collector or // mutator threads to do card-processing work. --- old/src/share/vm/gc/g1/ptrQueue.cpp 2015-10-30 19:31:40.079591928 -0400 +++ new/src/share/vm/gc/g1/ptrQueue.cpp 2015-10-30 19:31:39.991591492 -0400 @@ -30,24 +30,25 @@ #include "runtime/mutexLocker.hpp" #include "runtime/thread.inline.hpp" -PtrQueue::PtrQueue(PtrQueueSet* qset, bool perm, bool active) : +PtrQueue::PtrQueue(PtrQueueSet* qset, bool permanent, bool active) : _qset(qset), _buf(NULL), _index(0), _sz(0), _active(active), - _perm(perm), _lock(NULL) + _permanent(permanent), _lock(NULL) {} PtrQueue::~PtrQueue() { - assert(_perm || (_buf == NULL), "queue must be flushed before delete"); + assert(_permanent || (_buf == NULL), "queue must be flushed before delete"); } void PtrQueue::flush_impl() { - if (!_perm && _buf != NULL) { + if (!_permanent && _buf != NULL) { if (_index == _sz) { // No work to do. qset()->deallocate_buffer(_buf); } else { // We must NULL out the unused entries, then enqueue. - for (size_t i = 0; i < _index; i += oopSize) { - _buf[byte_index_to_index((int)i)] = NULL; + size_t limit = byte_index_to_index(_index); + for (size_t i = 0; i < limit; ++i) { + _buf[i] = NULL; } qset()->enqueue_complete_buffer(_buf); } @@ -66,8 +67,8 @@ } assert(_index > 0, "postcondition"); - _index -= oopSize; - _buf[byte_index_to_index((int)_index)] = ptr; + _index -= sizeof(void*); + _buf[byte_index_to_index(_index)] = ptr; assert(_index <= _sz, "Invariant."); } @@ -100,6 +101,26 @@ _fl_owner = this; } +PtrQueueSet::~PtrQueueSet() { + // There are presently only a couple (derived) instances ever + // created, and they are permanent, so no harm currently done by + // doing nothing here. +} + +void PtrQueueSet::initialize(Monitor* cbl_mon, + Mutex* fl_lock, + int process_completed_threshold, + int max_completed_queue, + PtrQueueSet *fl_owner) { + _max_completed_queue = max_completed_queue; + _process_completed_threshold = process_completed_threshold; + _completed_queue_padding = 0; + assert(cbl_mon != NULL && fl_lock != NULL, "Init order issue?"); + _cbl_mon = cbl_mon; + _fl_lock = fl_lock; + _fl_owner = (fl_owner != NULL) ? fl_owner : this; +} + void** PtrQueueSet::allocate_buffer() { assert(_sz > 0, "Didn't set a buffer size."); MutexLockerEx x(_fl_owner->_fl_lock, Mutex::_no_safepoint_check_flag); @@ -233,7 +254,7 @@ if (_notify_when_complete) _cbl_mon->notify(); } - debug_only(assert_completed_buffer_list_len_correct_locked()); + DEBUG_ONLY(assert_completed_buffer_list_len_correct_locked()); } int PtrQueueSet::completed_buffers_list_length() { @@ -258,7 +279,7 @@ void PtrQueueSet::set_buffer_size(size_t sz) { assert(_sz == 0 && sz > 0, "Should be called only once."); - _sz = sz * oopSize; + _sz = sz * sizeof(void*); } // Merge lists of buffers. Notify the processing threads. --- old/src/share/vm/gc/g1/ptrQueue.hpp 2015-10-30 19:31:40.583594428 -0400 +++ new/src/share/vm/gc/g1/ptrQueue.hpp 2015-10-30 19:31:40.499594011 -0400 @@ -40,44 +40,49 @@ class PtrQueue VALUE_OBJ_CLASS_SPEC { friend class VMStructs; -protected: + // Noncopyable - not defined. + PtrQueue(const PtrQueue&); + PtrQueue& operator=(const PtrQueue&); + // The ptr queue set to which this queue belongs. - PtrQueueSet* _qset; + PtrQueueSet* const _qset; // 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; + +protected: // The buffer. void** _buf; - // The index at which an object was last enqueued. Starts at "_sz" + // The (byte) index at which an object was last enqueued. Starts at "_sz" // (indicating an empty buffer) and goes towards zero. size_t _index; - // The size of the buffer. + // The (byte) size of the buffer. size_t _sz; - // 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. - bool _perm; - // If there is a lock associated with this buffer, this is that lock. Mutex* _lock; PtrQueueSet* qset() { return _qset; } - bool is_permanent() const { return _perm; } + bool is_permanent() const { return _permanent; } // Process queue entries and release resources, if not permanent. void flush_impl(); -public: // Initialize this queue to contain a null buffer, and be part of the // given PtrQueueSet. - PtrQueue(PtrQueueSet* qset, bool perm = false, bool active = false); + PtrQueue(PtrQueueSet* qset, bool permanent = false, bool active = false); // Requires queue flushed or permanent. ~PtrQueue(); +public: + // Associate a lock with a ptr queue. void set_lock(Mutex* lock) { _lock = lock; } @@ -129,13 +134,9 @@ bool is_active() { return _active; } - static int byte_index_to_index(int ind) { - assert((ind % oopSize) == 0, "Invariant."); - return ind / oopSize; - } - - static int index_to_byte_index(int byte_ind) { - return byte_ind * oopSize; + static size_t byte_index_to_index(size_t ind) { + assert((ind % sizeof(void*)) == 0, "Invariant."); + return ind / sizeof(void*); } // To support compiler. @@ -246,26 +247,21 @@ return false; } -public: // Create an empty ptr queue set. PtrQueueSet(bool notify_when_complete = false); + ~PtrQueueSet(); // Because of init-order concerns, we can't pass these as constructor // arguments. - void initialize(Monitor* cbl_mon, Mutex* fl_lock, + void initialize(Monitor* cbl_mon, + Mutex* fl_lock, int process_completed_threshold, int max_completed_queue, - PtrQueueSet *fl_owner = NULL) { - _max_completed_queue = max_completed_queue; - _process_completed_threshold = process_completed_threshold; - _completed_queue_padding = 0; - assert(cbl_mon != NULL && fl_lock != NULL, "Init order issue?"); - _cbl_mon = cbl_mon; - _fl_lock = fl_lock; - _fl_owner = (fl_owner != NULL) ? fl_owner : this; - } + PtrQueueSet *fl_owner = NULL); + +public: - // Return an empty oop array of size _sz (required to be non-zero). + // Return an empty array of size _sz (required to be non-zero). void** allocate_buffer(); // Return an empty buffer to the free list. The "buf" argument is --- old/src/share/vm/gc/g1/satbQueue.cpp 2015-10-30 19:31:41.103597006 -0400 +++ new/src/share/vm/gc/g1/satbQueue.cpp 2015-10-30 19:31:41.011596550 -0400 @@ -33,6 +33,15 @@ #include "runtime/thread.hpp" #include "runtime/vmThread.hpp" +ObjPtrQueue::ObjPtrQueue(SATBMarkQueueSet* qset, bool permanent) : + // 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 is done in JavaThread::initialize_queues(). + PtrQueue(qset, permanent, false /* active */) +{ } + void ObjPtrQueue::flush() { // Filter now to possibly save work later. If filtering empties the // buffer then flush_impl can deallocate the buffer. @@ -99,7 +108,6 @@ void ObjPtrQueue::filter() { G1CollectedHeap* g1h = G1CollectedHeap::heap(); void** buf = _buf; - size_t sz = _sz; if (buf == NULL) { // nothing to do @@ -107,43 +115,37 @@ } // Used for sanity checking at the end of the loop. - debug_only(size_t entries = 0; size_t retained = 0;) - - size_t i = sz; - size_t new_index = sz; + DEBUG_ONLY(size_t entries = 0; size_t retained = 0;) - while (i > _index) { - assert(i > 0, "we should have at least one more entry to process"); - i -= oopSize; - debug_only(entries += 1;) - void** p = &buf[byte_index_to_index((int) i)]; - void* entry = *p; + assert(_index <= _sz, "invariant"); + void** limit = &buf[byte_index_to_index(_index)]; + void** src = &buf[byte_index_to_index(_sz)]; + void** dst = src; + + while (limit < src) { + DEBUG_ONLY(entries += 1;) + --src; + void* entry = *src; // NULL the entry so that unused parts of the buffer contain NULLs // at the end. If we are going to retain it we will copy it to its // final place. If we have retained all entries we have visited so // far, we'll just end up copying it to the same place. - *p = NULL; + *src = NULL; if (requires_marking(entry, g1h) && !g1h->isMarkedNext((oop)entry)) { - assert(new_index > 0, "we should not have already filled up the buffer"); - new_index -= oopSize; - assert(new_index >= i, - "new_index should never be below i, as we always compact 'up'"); - void** new_p = &buf[byte_index_to_index((int) new_index)]; - assert(new_p >= p, "the destination location should never be below " - "the source as we always compact 'up'"); - assert(*new_p == NULL, - "we should have already cleared the destination location"); - *new_p = entry; - debug_only(retained += 1;) + --dst; + assert(*dst == NULL, "filtering destination should be clear"); + *dst = entry; + DEBUG_ONLY(retained += 1;); } } + size_t new_index = pointer_delta(dst, buf, 1); #ifdef ASSERT - size_t entries_calc = (sz - _index) / oopSize; + size_t entries_calc = (_sz - _index) / sizeof(void*); assert(entries == entries_calc, "the number of entries we counted " "should match the number of entries we calculated"); - size_t retained_calc = (sz - new_index) / oopSize; + size_t retained_calc = (_sz - new_index) / sizeof(void*); assert(retained == retained_calc, "the number of retained entries we counted " "should match the number of retained entries we calculated"); #endif // ASSERT @@ -171,8 +173,8 @@ filter(); size_t sz = _sz; - size_t all_entries = sz / oopSize; - size_t retained_entries = (sz - _index) / oopSize; + size_t all_entries = sz / sizeof(void*); + size_t retained_entries = (sz - _index) / sizeof(void*); size_t perc = retained_entries * 100 / all_entries; bool should_enqueue = perc > (size_t) G1SATBBufferEnqueueingThresholdPercent; return should_enqueue; @@ -185,8 +187,8 @@ assert(_index % sizeof(void*) == 0, "invariant"); assert(_sz % sizeof(void*) == 0, "invariant"); assert(_index <= _sz, "invariant"); - cl->do_buffer(_buf + byte_index_to_index((int)_index), - byte_index_to_index((int)(_sz - _index))); + cl->do_buffer(_buf + byte_index_to_index(_index), + byte_index_to_index(_sz - _index)); _index = _sz; } } @@ -299,7 +301,7 @@ // Filtering can result in non-full completed buffers; see // should_enqueue_buffer. assert(_sz % sizeof(void*) == 0, "invariant"); - size_t limit = ObjPtrQueue::byte_index_to_index((int)_sz); + size_t limit = ObjPtrQueue::byte_index_to_index(_sz); for (size_t i = 0; i < limit; ++i) { if (buf[i] != NULL) { // Found the end of the block of NULLs; process the remainder. --- old/src/share/vm/gc/g1/satbQueue.hpp 2015-10-30 19:31:41.615599545 -0400 +++ new/src/share/vm/gc/g1/satbQueue.hpp 2015-10-30 19:31:41.527599109 -0400 @@ -50,13 +50,7 @@ void filter(); public: - ObjPtrQueue(PtrQueueSet* qset, bool perm = false) : - // 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 is done in JavaThread::initialize_queues(). - PtrQueue(qset, perm, false /* active */) { } + ObjPtrQueue(SATBMarkQueueSet* qset, bool permanent = false); // Process queue entries and free resources. void flush();