New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8254739: G1: Optimize evacuation failure for regions with few failed objects #5181
Changes from 17 commits
954b5ee
4d88f42
74bc4d9
05f026a
ded8275
51d19eb
f454925
34aed3f
43a8b59
6132372
44e3562
e779c3a
924fec5
0070826
c4ca77c
ab04f1c
3712037
82c172a
d33f87b
e588cad
3efa90b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
/* | ||
* Copyright (c) 2021, Huawei 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/g1EvacFailureObjectsSet.hpp" | ||
#include "gc/g1/g1CollectedHeap.hpp" | ||
#include "gc/g1/g1SegmentedArray.inline.hpp" | ||
#include "gc/g1/heapRegion.hpp" | ||
#include "gc/g1/heapRegion.inline.hpp" | ||
#include "utilities/quickSort.hpp" | ||
|
||
|
||
const G1SegmentedArrayAllocOptions G1EvacFailureObjectsSet::_alloc_options = | ||
G1SegmentedArrayAllocOptions((uint)sizeof(OffsetInRegion), BufferLength, UINT_MAX, Alignment); | ||
|
||
G1SegmentedArrayBufferList<mtGC> G1EvacFailureObjectsSet::_free_buffer_list; | ||
|
||
#ifdef ASSERT | ||
void G1EvacFailureObjectsSet::assert_is_valid_offset(size_t offset) const { | ||
const uint max_offset = 1u << (HeapRegion::LogOfHRGrainBytes - LogHeapWordSize); | ||
assert(offset < max_offset, "must be, but is " SIZE_FORMAT, offset); | ||
} | ||
#endif | ||
|
||
oop G1EvacFailureObjectsSet::from_offset(OffsetInRegion offset) const { | ||
assert_is_valid_offset(offset); | ||
return cast_to_oop(_bottom + offset); | ||
} | ||
|
||
G1EvacFailureObjectsSet::OffsetInRegion G1EvacFailureObjectsSet::cast_to_offset(oop obj) const { | ||
const HeapWord* o = cast_from_oop<const HeapWord*>(obj); | ||
size_t offset = pointer_delta(o, _bottom); | ||
assert_is_valid_offset(offset); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
assert(obj == from_offset(static_cast<OffsetInRegion>(offset)), "must be"); | ||
return static_cast<OffsetInRegion>(offset); | ||
} | ||
|
||
G1EvacFailureObjectsSet::G1EvacFailureObjectsSet(uint region_idx, HeapWord* bottom) : | ||
DEBUG_ONLY(_region_idx(region_idx) COMMA) | ||
_bottom(bottom), | ||
_offsets("", &_alloc_options, &_free_buffer_list) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fwiw, I recently removed the first parameter of |
||
assert(HeapRegion::LogOfHRGrainBytes < 32, "must be"); | ||
} | ||
|
||
void G1EvacFailureObjectsSet::record(oop obj) { | ||
assert(obj != NULL, "must be"); | ||
assert(_region_idx == G1CollectedHeap::heap()->heap_region_containing(obj)->hrm_index(), "must be"); | ||
OffsetInRegion* e = _offsets.allocate(); | ||
*e = cast_to_offset(obj); | ||
} | ||
|
||
// Helper class to join, sort and iterate over the previously collected segmented | ||
// array of objects that failed evacuation. | ||
class G1EvacFailureObjectsIterator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this is not an |
||
typedef G1EvacFailureObjectsSet::OffsetInRegion OffsetInRegion; | ||
friend class G1SegmentedArray<OffsetInRegion, mtGC>; | ||
friend class G1SegmentedArrayBuffer<mtGC>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to make |
||
|
||
G1EvacFailureObjectsSet* _collector; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably rename |
||
const G1SegmentedArray<OffsetInRegion, mtGC>* _segments; | ||
OffsetInRegion* _offset_array; | ||
uint _array_length; | ||
|
||
static int order_oop(OffsetInRegion a, OffsetInRegion b) { | ||
return static_cast<int>(a-b); | ||
} | ||
|
||
void join_and_sort() { | ||
uint num = _segments->num_allocated_nodes(); | ||
_offset_array = NEW_C_HEAP_ARRAY(OffsetInRegion, num, mtGC); | ||
|
||
_segments->iterate_nodes(*this); | ||
assert(_array_length == num, "must be %u, %u", _array_length, num); | ||
|
||
QuickSort::sort(_offset_array, _array_length, order_oop, true); | ||
} | ||
|
||
void iterate_internal(ObjectClosure* closure) { | ||
for (uint i = 0; i < _array_length; i++) { | ||
_collector->assert_is_valid_offset(_offset_array[i]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assert duplicates the one in |
||
oop cur = _collector->from_offset(_offset_array[i]); | ||
closure->do_object(cur); | ||
} | ||
|
||
FREE_C_HEAP_ARRAY(OffsetInRegion, _offset_array); | ||
} | ||
|
||
// Callback of G1SegmentedArray::iterate_nodes | ||
void visit_buffer(G1SegmentedArrayBuffer<mtGC>* node, uint length) { | ||
node->copy_to(&_offset_array[_array_length]); | ||
_array_length += length; | ||
|
||
// Verify elements in the node | ||
DEBUG_ONLY(node->iterate_elems(*this)); | ||
} | ||
|
||
#ifdef ASSERT | ||
// Callback of G1SegmentedArrayBuffer::iterate_elems | ||
// Verify a single element in a segment node | ||
void visit_elem(void* elem) { | ||
uint* ptr = (uint*)elem; | ||
_collector->assert_is_valid_offset(*ptr); | ||
} | ||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I intentionally removed this Fwiw, in Hotspot code we do not use the A change here needs to be discussed with a wider audience. |
||
|
||
public: | ||
G1EvacFailureObjectsIterator(G1EvacFailureObjectsSet* collector) : | ||
_collector(collector), | ||
_segments(&_collector->_offsets), | ||
_offset_array(nullptr), | ||
_array_length(0) { } | ||
|
||
~G1EvacFailureObjectsIterator() { } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the empty unnecessary destructor. |
||
|
||
void iterate(ObjectClosure* closure) { | ||
join_and_sort(); | ||
iterate_internal(closure); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably move the array allocation and freeing here instead of having this at the start and end of I.e.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Thomas, good catch. |
||
} | ||
}; | ||
|
||
void G1EvacFailureObjectsSet::iterate(ObjectClosure* closure) { | ||
assert_at_safepoint(); | ||
|
||
G1EvacFailureObjectsIterator iterator(this); | ||
iterator.iterate(closure); | ||
|
||
_offsets.drop_all(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
/* | ||
* Copyright (c) 2021, Huawei 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_G1EVACUATIONFAILUREOBJSINHR_HPP | ||
#define SHARE_GC_G1_G1EVACUATIONFAILUREOBJSINHR_HPP | ||
|
||
#include "gc/g1/g1SegmentedArray.hpp" | ||
#include "memory/iterator.hpp" | ||
#include "oops/oop.hpp" | ||
|
||
class G1EvacFailureObjectsIterator; | ||
|
||
// This class collects addresses of objects that failed evacuation in a specific | ||
// heap region. | ||
// Provides sorted iteration of these elements for processing during the remove | ||
// self forwards phase. | ||
class G1EvacFailureObjectsSet { | ||
friend class G1EvacFailureObjectsIterator; | ||
|
||
public: | ||
// Storage type of an object that failed evacuation within a region. Given | ||
// heap region size and possible object locations within a region, it is | ||
// sufficient to use an uint here to save some space instead of full pointers. | ||
typedef uint OffsetInRegion; | ||
|
||
private: | ||
static const uint BufferLength = 256; | ||
static const uint Alignment = 4; | ||
|
||
static const G1SegmentedArrayAllocOptions _alloc_options; | ||
|
||
// This free list is shared among evacuation failure process in all regions. | ||
static G1SegmentedArrayBufferList<mtGC> _free_buffer_list; | ||
|
||
DEBUG_ONLY(const uint _region_idx;) | ||
|
||
// Region bottom | ||
const HeapWord* _bottom; | ||
|
||
// Offsets within region containing objects that failed evacuation. | ||
G1SegmentedArray<OffsetInRegion, mtGC> _offsets; | ||
|
||
void assert_is_valid_offset(size_t offset) const NOT_DEBUG_RETURN; | ||
// Converts between an offset within a region and an oop address. | ||
oop from_offset(OffsetInRegion offset) const; | ||
OffsetInRegion cast_to_offset(oop obj) const; | ||
|
||
public: | ||
G1EvacFailureObjectsSet(uint region_idx, HeapWord* bottom); | ||
~G1EvacFailureObjectsSet() { } | ||
|
||
// Record an object that failed evacuation. | ||
void record(oop obj); | ||
|
||
// Apply the given ObjectClosure to all objects that failed evacuation. Objects | ||
// are passed in increasing address order. | ||
void iterate(ObjectClosure* closure); | ||
}; | ||
|
||
|
||
#endif //SHARE_GC_G1_G1EVACUATIONFAILUREOBJSINHR_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be
to_offset
to matchfrom_offset
(forgot that) - this is not a cast to me (changing the interpretation of a bit pattern) but a transformation (changing the bit pattern for storage savings).So
cast
seems to be the wrong word to me here.