--- old/src/share/vm/gc/g1/g1CollectedHeap.cpp 2016-06-28 14:13:03.861389183 +0200 +++ new/src/share/vm/gc/g1/g1CollectedHeap.cpp 2016-06-28 14:13:03.770386441 +0200 @@ -2418,8 +2418,8 @@ _collection_set.iterate(cl); } -void G1CollectedHeap::collection_set_iterate_from(HeapRegionClosure *cl, uint worker_id, uint active_workers) { - _collection_set.iterate_from(cl, worker_id, active_workers); +void G1CollectedHeap::collection_set_iterate_from(HeapRegionClosure *cl, uint worker_id) { + _collection_set.iterate_from(cl, worker_id, workers()->active_workers()); } HeapRegion* G1CollectedHeap::next_compaction_region(const HeapRegion* from) const { @@ -2987,7 +2987,8 @@ } }; -bool G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) { +bool +G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) { assert_at_safepoint(true /* should_be_vm_thread */); guarantee(!is_gc_active(), "collection is not reentrant"); --- old/src/share/vm/gc/g1/g1CollectedHeap.hpp 2016-06-28 14:13:04.424406143 +0200 +++ new/src/share/vm/gc/g1/g1CollectedHeap.hpp 2016-06-28 14:13:04.332403371 +0200 @@ -1205,10 +1205,10 @@ void collection_set_iterate(HeapRegionClosure* blk); // Iterate over the regions (if any) in the current collection set. Starts the - // iteration over the entire collection set so that a given worker id over the - // set of 0..active_workers-1 are distributed across the set of collection set - // regions. - void collection_set_iterate_from(HeapRegionClosure *blk, uint worker_id, uint active_workers); + // iteration over the entire collection set so that the start regions of a given + // worker id over the set active_workers are evenly spread across the set of + // collection set regions. + void collection_set_iterate_from(HeapRegionClosure *blk, uint worker_id); HeapRegion* next_compaction_region(const HeapRegion* from) const; --- old/src/share/vm/gc/g1/g1CollectionSet.cpp 2016-06-28 14:13:04.957422199 +0200 +++ new/src/share/vm/gc/g1/g1CollectionSet.cpp 2016-06-28 14:13:04.861419307 +0200 @@ -26,7 +26,6 @@ #include "gc/g1/g1CollectedHeap.hpp" #include "gc/g1/g1CollectionSet.hpp" #include "gc/g1/g1CollectorState.hpp" -#include "gc/g1/g1FixedSizeStack.inline.hpp" #include "gc/g1/g1Policy.hpp" #include "gc/g1/heapRegion.inline.hpp" #include "gc/g1/heapRegionRemSet.hpp" @@ -57,9 +56,11 @@ _eden_region_length(0), _survivor_region_length(0), _old_region_length(0), - _collection_set_regions(), _bytes_used_before(0), _recorded_rs_lengths(0), + _collection_set_regions(NULL), + _collection_set_cur_length(0), + _collection_set_max_length(0), // Incremental CSet attributes _inc_build_state(Inactive), _inc_bytes_used_before(0), @@ -70,23 +71,29 @@ } G1CollectionSet::~G1CollectionSet() { + if (_collection_set_regions != NULL) { + FREE_C_HEAP_ARRAY(uint, _collection_set_regions); + } delete _cset_chooser; } void G1CollectionSet::init_region_lengths(uint eden_cset_region_length, uint survivor_cset_region_length) { + assert_at_safepoint(true); + _eden_region_length = eden_cset_region_length; _survivor_region_length = survivor_cset_region_length; - assert((size_t) young_region_length() == _collection_set_regions.length(), - "Young region length %u should match collection set length " SIZE_FORMAT, young_region_length(), _collection_set_regions.length()); + assert((size_t) young_region_length() == _collection_set_cur_length, + "Young region length %u should match collection set length " SIZE_FORMAT, young_region_length(), _collection_set_cur_length); _old_region_length = 0; } void G1CollectionSet::set_max_length(uint max_region_length) { - guarantee(_collection_set_regions.max_length() == 0, "Must only initialize once."); - _collection_set_regions.initialize(max_region_length); + guarantee(_collection_set_regions == NULL, "Must only initialize once."); + _collection_set_max_length = max_region_length; + _collection_set_regions = NEW_C_HEAP_ARRAY(uint, max_region_length, mtGC); } void G1CollectionSet::set_recorded_rs_lengths(size_t rs_lengths) { @@ -95,13 +102,16 @@ // Add the heap region at the head of the non-incremental collection set void G1CollectionSet::add_old_region(HeapRegion* hr) { + assert_at_safepoint(true); + assert(_inc_build_state == Active, "Precondition"); assert(hr->is_old(), "the region should be old"); assert(!hr->in_collection_set(), "should not already be in the CSet"); _g1->register_old_region_with_cset(hr); - _collection_set_regions.par_push(hr->hrm_index()); + _collection_set_regions[_collection_set_cur_length++] = hr->hrm_index(); + assert(_collection_set_cur_length <= _collection_set_max_length, "Collection set now larger than maximum size."); _bytes_used_before += hr->used(); size_t rs_length = hr->rem_set()->occupied(); @@ -111,11 +121,9 @@ // Initialize the per-collection-set information void G1CollectionSet::start_incremental_building() { - assert(_collection_set_regions.length() == 0, "Must be empty before starting a new collection set."); + assert(_collection_set_cur_length == 0, "Collection set must be empty before starting a new collection set."); assert(_inc_build_state == Inactive, "Precondition"); - _collection_set_regions.clear(); - _inc_bytes_used_before = 0; _inc_recorded_rs_lengths = 0; @@ -157,12 +165,18 @@ _inc_predicted_elapsed_time_ms_diffs = 0.0; } -void G1CollectionSet::iterate(HeapRegionClosure* cl) { - iterate_from(cl, 0, 1, true); +void G1CollectionSet::clear() { + assert_at_safepoint(true); + _collection_set_cur_length = 0; +} + +void G1CollectionSet::iterate(HeapRegionClosure* cl, bool may_be_aborted) { + iterate_from(cl, 0, 1, may_be_aborted); } void G1CollectionSet::iterate_from(HeapRegionClosure* cl, uint worker_id, uint total_workers, bool may_be_aborted) { - size_t len = _collection_set_regions.length(); + size_t len = _collection_set_cur_length; + OrderAccess::loadload(); if (len == 0) { return; } @@ -170,10 +184,11 @@ size_t cur_pos = start_pos; do { - HeapRegion* r = G1CollectedHeap::heap()->region_at(_collection_set_regions.get_by_index(cur_pos)); + HeapRegion* r = G1CollectedHeap::heap()->region_at(_collection_set_regions[cur_pos]); bool result = cl->doHeapRegion(r); guarantee(may_be_aborted || !result, "This iteration should not abort."); if (result) { + cl->incomplete(); return; } cur_pos++; @@ -215,10 +230,16 @@ assert(hr->is_young(), "invariant"); assert(_inc_build_state == Active, "Precondition"); - size_t collection_set_length = _collection_set_regions.length(); + size_t collection_set_length = _collection_set_cur_length; assert(collection_set_length <= INT_MAX, "Collection set is too large with %d entries", (int)collection_set_length); hr->set_young_index_in_cset((int)collection_set_length); - _collection_set_regions.par_push(hr->hrm_index()); + + _collection_set_regions[_collection_set_cur_length] = hr->hrm_index(); + // Concurrent readers must observe the store of the value in the array before an + // update to the length field. + OrderAccess::storestore(); + _collection_set_cur_length++; + assert(_collection_set_cur_length <= _collection_set_max_length, "Collection set larger than maximum allowed."); // This routine is used when: // * adding survivor regions to the incremental cset at the end of an @@ -266,11 +287,13 @@ #ifndef PRODUCT bool G1CollectionSet::verify_young_ages() { + assert_at_safepoint(true); + bool ret = true; - size_t length = _collection_set_regions.length(); + size_t length = _collection_set_cur_length; for (size_t i = 0; i < length; i++) { - HeapRegion* curr = G1CollectedHeap::heap()->region_at(_collection_set_regions.get_by_index(i)); + HeapRegion* curr = G1CollectedHeap::heap()->region_at(_collection_set_regions[i]); guarantee(curr->is_young(), "Region must be young but is %s", curr->get_type_str()); @@ -466,15 +489,17 @@ #ifdef ASSERT void G1CollectionSet::verify_young_cset_indices() const { + assert_at_safepoint(true); + ResourceMark rm; uint* heap_region_indices = NEW_RESOURCE_ARRAY(uint, young_region_length()); for (uint i = 0; i < young_region_length(); ++i) { heap_region_indices[i] = (uint)-1; } - size_t length = _collection_set_regions.length(); + size_t length = _collection_set_cur_length; for (size_t i = 0; i < length; i++) { - HeapRegion* hr = G1CollectedHeap::heap()->region_at(_collection_set_regions.get_by_index(i)); + HeapRegion* hr = G1CollectedHeap::heap()->region_at(_collection_set_regions[i]); const int idx = hr->young_index_in_cset(); assert(idx > -1, "Young index must be set for all regions in the incremental collection set but is not for region %u.", hr->hrm_index()); --- old/src/share/vm/gc/g1/g1CollectionSet.hpp 2016-06-28 14:13:05.515439009 +0200 +++ new/src/share/vm/gc/g1/g1CollectionSet.hpp 2016-06-28 14:13:05.426436328 +0200 @@ -26,7 +26,6 @@ #define SHARE_VM_GC_G1_G1COLLECTIONSET_HPP #include "gc/g1/collectionSetChooser.hpp" -#include "gc/g1/g1FixedSizeStack.hpp" #include "memory/allocation.hpp" #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" @@ -49,7 +48,9 @@ uint _old_region_length; // The actual collection set as a set of region indices. - G1FixedSizeStack _collection_set_regions; + uint* _collection_set_regions; + volatile size_t _collection_set_cur_length; + size_t _collection_set_max_length; // The number of bytes in the collection set before the pause. Set from // the incrementally built collection set at the start of an evacuation @@ -127,7 +128,7 @@ uint survivor_region_length() const { return _survivor_region_length; } uint old_region_length() const { return _old_region_length; } - // Incremental collection set Support + // Incremental collection set support // Initialize incremental collection set info. void start_incremental_building(); @@ -137,12 +138,12 @@ void finalize_incremental_building(); // Reset the contents of the collection set. - void clear() { - _collection_set_regions.clear(); - } + void clear(); // Iterate over the collection set, applying the given HeapRegionClosure on all of them. - void iterate(HeapRegionClosure* cl); + // If may_be_aborted is true, iteration may be aborted using the return value of the + // called closure method. + void iterate(HeapRegionClosure* cl, bool may_be_aborted = false); // Iterate over the collection set, applying the given HeapRegionClosure on all of them, // trying to optimally spread out starting position of total_workers workers given the --- old/src/share/vm/gc/g1/g1EvacFailure.cpp 2016-06-28 14:13:06.071455759 +0200 +++ new/src/share/vm/gc/g1/g1EvacFailure.cpp 2016-06-28 14:13:05.983453108 +0200 @@ -251,5 +251,5 @@ void G1ParRemoveSelfForwardPtrsTask::work(uint worker_id) { RemoveSelfForwardPtrHRClosure rsfp_cl(worker_id, &_hrclaimer); - _g1h->collection_set_iterate_from(&rsfp_cl, worker_id, _g1h->workers()->active_workers()); + _g1h->collection_set_iterate_from(&rsfp_cl, worker_id); } --- old/src/share/vm/gc/g1/g1HeapVerifier.cpp 2016-06-28 14:13:06.570470791 +0200 +++ new/src/share/vm/gc/g1/g1HeapVerifier.cpp 2016-06-28 14:13:06.471467809 +0200 @@ -591,13 +591,9 @@ } }; -void G1HeapVerifier::verify_dirty_young_list(G1CollectionSet* collection_set) { - G1VerifyDirtyYoungListClosure cl(this); - collection_set->iterate(&cl); -} - void G1HeapVerifier::verify_dirty_young_regions() { - verify_dirty_young_list(_g1h->collection_set()); + G1VerifyDirtyYoungListClosure cl(this); + _g1h->collection_set()->iterate(&cl); } bool G1HeapVerifier::verify_no_bits_over_tams(const char* bitmap_name, G1CMBitMapRO* bitmap, --- old/src/share/vm/gc/g1/g1HeapVerifier.hpp 2016-06-28 14:13:07.063485642 +0200 +++ new/src/share/vm/gc/g1/g1HeapVerifier.hpp 2016-06-28 14:13:06.976483022 +0200 @@ -108,7 +108,6 @@ void verify_not_dirty_region(HeapRegion* hr) PRODUCT_RETURN; void verify_dirty_region(HeapRegion* hr) PRODUCT_RETURN; - void verify_dirty_young_list(G1CollectionSet* collection_set) PRODUCT_RETURN; void verify_dirty_young_regions() PRODUCT_RETURN; }; --- old/src/share/vm/gc/g1/g1RemSet.cpp 2016-06-28 14:13:07.557500524 +0200 +++ new/src/share/vm/gc/g1/g1RemSet.cpp 2016-06-28 14:13:07.470497903 +0200 @@ -383,7 +383,7 @@ double rs_time_start = os::elapsedTime(); G1ScanRSClosure cl(_scan_state, oops_in_heap_closure, heap_region_codeblobs, worker_i); - _g1->collection_set_iterate_from(&cl, worker_i, _g1->workers()->active_workers()); + _g1->collection_set_iterate_from(&cl, worker_i); double scan_rs_time_sec = (os::elapsedTime() - rs_time_start) - cl.strong_code_root_scan_time_sec(); --- old/src/share/vm/gc/g1/g1YoungRemSetSamplingThread.cpp 2016-06-28 14:13:08.047515285 +0200 +++ new/src/share/vm/gc/g1/g1YoungRemSetSamplingThread.cpp 2016-06-28 14:13:07.960512664 +0200 @@ -107,11 +107,12 @@ SuspendibleThreadSetJoiner sts; G1CollectedHeap* g1h = G1CollectedHeap::heap(); G1Policy* g1p = g1h->g1_policy(); - G1CollectionSet* g1cs = g1h->collection_set(); - if (G1CollectedHeap::heap()->g1_policy()->adaptive_young_list_length()) { + if (g1p->adaptive_young_list_length()) { G1YoungRemSetSamplingClosure cl(&sts); - g1cs->iterate(&cl); + + G1CollectionSet* g1cs = g1h->collection_set(); + g1cs->iterate(&cl, true); if (cl.complete()) { g1p->revise_young_list_target_length_if_necessary(cl.sampled_rs_lengths()); --- old/src/share/vm/gc/g1/heapRegion.hpp 2016-06-28 14:13:08.537530047 +0200 +++ new/src/share/vm/gc/g1/heapRegion.hpp 2016-06-28 14:13:08.450527426 +0200 @@ -734,12 +734,15 @@ // HeapRegionClosure is used for iterating over regions. // Terminates the iteration when the "doHeapRegion" method returns "true". class HeapRegionClosure : public StackObj { + friend class HeapRegionManager; + friend class G1CollectionSet; + bool _complete; + void incomplete() { _complete = false; } + public: HeapRegionClosure(): _complete(true) {} - void incomplete() { _complete = false; } - // Typically called on each region until it returns true. virtual bool doHeapRegion(HeapRegion* r) = 0; --- old/src/share/vm/gc/g1/g1FixedSizeStack.cpp 2016-06-28 14:13:09.054545621 +0200 +++ /dev/null 2016-06-08 17:45:05.423675909 +0200 @@ -1,43 +0,0 @@ -/* - * Copyright (c) 2016, 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/g1FixedSizeStack.inline.hpp" -#include "memory/allocation.inline.hpp" - -G1FixedSizeStackBase::~G1FixedSizeStackBase() { - FREE_C_HEAP_ARRAY(address, _base); -} - -void G1FixedSizeStackBase::initialize(size_t max_length, size_t elem_size) { - _base = NEW_C_HEAP_ARRAY(unsigned char, max_length * elem_size, mtGC); - _max_length = max_length; - _length = 0; -} - -#ifndef PRODUCT -void G1FixedSizeStackBase::verify_index(size_t index) const { - assert(index < length(), "Index " SIZE_FORMAT " out of bounds " SIZE_FORMAT, index, length()); -} -#endif --- old/src/share/vm/gc/g1/g1FixedSizeStack.hpp 2016-06-28 14:13:09.343554327 +0200 +++ /dev/null 2016-06-08 17:45:05.423675909 +0200 @@ -1,105 +0,0 @@ -/* - * Copyright (c) 2016, 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_VM_GC_G1_G1FIXEDSIZESTACK_HPP -#define SHARE_VM_GC_G1_G1FIXEDSIZESTACK_HPP - -#include "memory/allocation.hpp" -#include "utilities/debug.hpp" -#include "utilities/globalDefinitions.hpp" - -// Base class for a fixed size stack, i.e. its backing array is of fixed size. Collects -// methods common to any instances of the array. The backing array is preallocated -// on the C heap. -// -// Pushing elements and access to elements below _length is MT safe. Any other -// operations modifying it are not. -class G1FixedSizeStackBase : public CHeapObj { -protected: - address _base; // The real base address. - size_t _max_length; // The length of the array. - - // Current number of elements pushed. The MSB of this value is also used to - // indicate that a concurrent writer is currently pushing to it. - size_t volatile _length; - - static const size_t LengthMask = ~nth_bit(BitsPerWord - 1); - -protected: - - G1FixedSizeStackBase() : _base(NULL), _max_length(0), _length(0) { } - - // Allocate and initialize the backing array. - void initialize(size_t max_length, size_t elem_size); - - void verify_index(size_t index) const PRODUCT_RETURN; - -public: - virtual ~G1FixedSizeStackBase(); - - size_t max_length() const { return _max_length; } - inline size_t length() const; -}; - -template -class G1FixedSizeStack : public G1FixedSizeStackBase { -private: - T* base() const { return (T*)G1FixedSizeStackBase::_base; } - - // Set the element of the given array at the given index to the - // given value. Assume the index is valid. - void set_by_index_raw(size_t index, T const value) { - this->base()[index] = value; - } - -public: - // Return the element of the array at the given index. Assume - // the index is valid. This is a convenience method that does sanity - // checking on the index. - T get_by_index(size_t index) const { - verify_index(index); - return this->base()[index]; - } - - void clear() { -#ifdef ASSERT - _length = _max_length; - for (size_t i = 0; i < _max_length; i++) { - set_by_index_raw(i, T()); - } -#endif - _length = 0; - } - - G1FixedSizeStack() : G1FixedSizeStackBase() {} - - void initialize(size_t max_length) { - G1FixedSizeStackBase::initialize(max_length, sizeof(T)); - this->clear(); - } - - inline void par_push(T const elem); -}; - -#endif // SHARE_VM_GC_G1_G1FIXEDSIZESTACK_HPP --- old/src/share/vm/gc/g1/g1FixedSizeStack.inline.hpp 2016-06-28 14:13:09.624562792 +0200 +++ /dev/null 2016-06-08 17:45:05.423675909 +0200 @@ -1,56 +0,0 @@ -/* - * Copyright (c) 2016, 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_VM_GC_G1_G1FIXEDSIZESTACK_INLINE_HPP -#define SHARE_VM_GC_G1_G1FIXEDSIZESTACK_INLINE_HPP - -#include "gc/g1/g1FixedSizeStack.hpp" -#include "runtime/atomic.inline.hpp" -#include "runtime/orderAccess.inline.hpp" -#include "utilities/debug.hpp" - -size_t G1FixedSizeStackBase::length() const { - return OrderAccess::load_acquire((volatile intptr_t*)&_length) & LengthMask; -} -// This is required for the use of Atomic::cmpxchg_ptr in par_push(). -STATIC_ASSERT(sizeof(intptr_t) == sizeof(size_t)); - -template -void G1FixedSizeStack::par_push(const T elem) { - // Try to set exclusive access to the length field by setting its upper bit. - size_t cur_length = length(); - while (true) { - size_t old_length = (size_t)Atomic::cmpxchg_ptr((intptr_t)cur_length | nth_bit(BitsPerWord - 1), (volatile intptr_t*)&_length, (intptr_t)cur_length); - if (old_length == cur_length) { - assert((cur_length & nth_bit(BitsPerWord - 1)) == 0, "Topmost bit must not be set in old value."); - break; - } - cur_length = old_length & LengthMask; - } - assert(cur_length < _max_length, "Tried to push more objects than there is space."); - set_by_index_raw(cur_length, elem); - OrderAccess::release_store_ptr((volatile intptr_t*)&_length, cur_length + 1); -} - -#endif /* SHARE_VM_GC_G1_G1FIXEDSIZESTACK_INLINE_HPP */