Skip to content
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

Closed
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
954b5ee
speed up iterate evac failure objs in one region.
Hamlin-Li Aug 18, 2021
4d88f42
fix: reset state after iteration
Hamlin-Li Aug 18, 2021
74bc4d9
Use a segmented array rather than a linked list to record evacuation …
Hamlin-Li Aug 25, 2021
05f026a
Fix compilation issues on some platform.
Hamlin-Li Aug 25, 2021
ded8275
Fix test failures; Fix compilation failures on some platforms
Hamlin-Li Aug 26, 2021
51d19eb
Fix test failures; Fix compilation failures on some platforms
Hamlin-Li Aug 26, 2021
f454925
Fix compilation error on windows
Hamlin-Li Aug 26, 2021
34aed3f
Fix compilation error on windows
Hamlin-Li Aug 26, 2021
43a8b59
Merge branch 'master' into speedup-iterate-evac-failure-objs-in-one-r…
Hamlin-Li Oct 25, 2021
6132372
Fix wrong merge
Hamlin-Li Oct 25, 2021
44e3562
Use refactored G1SegmentedArray rather than home-made Array+Node
Hamlin-Li Oct 25, 2021
e779c3a
Add asserts, comments
Hamlin-Li Oct 26, 2021
924fec5
Rename from g1EvacuationFailureObjsInHR to g1EvacFailureObjsInHR
Hamlin-Li Oct 26, 2021
0070826
Refactor as Thomas suggested
Hamlin-Li Oct 29, 2021
c4ca77c
Fix compilation error
Hamlin-Li Oct 29, 2021
ab04f1c
Fix compilation error
Hamlin-Li Oct 29, 2021
3712037
Merge branch 'openjdk:master' into speedup-iterate-evac-failure-objs-…
Hamlin-Li Oct 29, 2021
82c172a
Refine code based on Thomas' suggestion
Hamlin-Li Oct 29, 2021
d33f87b
Merge branch 'openjdk:master' into speedup-iterate-evac-failure-objs-…
Hamlin-Li Nov 3, 2021
e588cad
Move allocation/deallocation in one place
Hamlin-Li Nov 3, 2021
3efa90b
Fix typo
Hamlin-Li Nov 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -114,47 +114,46 @@ class RemoveSelfForwardPtrObjClosure: public ObjectClosure {
HeapWord* obj_addr = cast_from_oop<HeapWord*>(obj);
assert(_hr->is_in(obj_addr), "sanity");

if (obj->is_forwarded() && obj->forwardee() == obj) {
// The object failed to move.

zap_dead_objects(_last_forwarded_object_end, obj_addr);
// We consider all objects that we find self-forwarded to be
// live. What we'll do is that we'll update the prev marking
// info so that they are all under PTAMS and explicitly marked.
if (!_cm->is_marked_in_prev_bitmap(obj)) {
_cm->mark_in_prev_bitmap(obj);
}
if (_during_concurrent_start) {
// For the next marking info we'll only mark the
// self-forwarded objects explicitly if we are during
// concurrent start (since, normally, we only mark objects pointed
// to by roots if we succeed in copying them). By marking all
// self-forwarded objects we ensure that we mark any that are
// still pointed to be roots. During concurrent marking, and
// after concurrent start, we don't need to mark any objects
// explicitly and all objects in the CSet are considered
// (implicitly) live. So, we won't mark them explicitly and
// we'll leave them over NTAMS.
_cm->mark_in_next_bitmap(_worker_id, _hr, obj);
}
size_t obj_size = obj->size();

_marked_bytes += (obj_size * HeapWordSize);
PreservedMarks::init_forwarded_mark(obj);

// During evacuation failure we do not record inter-region
// references referencing regions that need a remembered set
// update originating from young regions (including eden) that
// failed evacuation. Make up for that omission now by rescanning
// these failed objects.
if (_is_young) {
obj->oop_iterate(_log_buffer_cl);
}

HeapWord* obj_end = obj_addr + obj_size;
_last_forwarded_object_end = obj_end;
_hr->cross_threshold(obj_addr, obj_end);
// The object failed to move.
Hamlin-Li marked this conversation as resolved.
Show resolved Hide resolved
assert(obj->is_forwarded() && obj->forwardee() == obj, "sanity");

zap_dead_objects(_last_forwarded_object_end, obj_addr);
// We consider all objects that we find self-forwarded to be
// live. What we'll do is that we'll update the prev marking
// info so that they are all under PTAMS and explicitly marked.
if (!_cm->is_marked_in_prev_bitmap(obj)) {
_cm->mark_in_prev_bitmap(obj);
}
if (_during_concurrent_start) {
// For the next marking info we'll only mark the
// self-forwarded objects explicitly if we are during
// concurrent start (since, normally, we only mark objects pointed
// to by roots if we succeed in copying them). By marking all
// self-forwarded objects we ensure that we mark any that are
// still pointed to be roots. During concurrent marking, and
// after concurrent start, we don't need to mark any objects
// explicitly and all objects in the CSet are considered
// (implicitly) live. So, we won't mark them explicitly and
// we'll leave them over NTAMS.
_cm->mark_in_next_bitmap(_worker_id, _hr, obj);
}
size_t obj_size = obj->size();

_marked_bytes += (obj_size * HeapWordSize);
PreservedMarks::init_forwarded_mark(obj);

// During evacuation failure we do not record inter-region
// references referencing regions that need a remembered set
// update originating from young regions (including eden) that
// failed evacuation. Make up for that omission now by rescanning
// these failed objects.
if (_is_young) {
obj->oop_iterate(_log_buffer_cl);
}

HeapWord* obj_end = obj_addr + obj_size;
_last_forwarded_object_end = obj_end;
_hr->cross_threshold(obj_addr, obj_end);
}

// Fill the memory area from start to end with filler objects, and update the BOT
@@ -223,7 +222,8 @@ class RemoveSelfForwardPtrHRClosure: public HeapRegionClosure {
&_log_buffer_cl,
during_concurrent_start,
_worker_id);
hr->object_iterate(&rspc);
// Iterates evac failure objs which are recorded during evacuation.
hr->iterate_evac_failure_objs(&rspc);
// Need to zap the remainder area of the processed region.
rspc.zap_remainder();

@@ -0,0 +1,121 @@
/*
* 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 "g1EvacuationFailureObjsInHR.hpp"
#include "utilities/quickSort.hpp"


static intptr_t order_oop(oop a, oop b) {
return static_cast<intptr_t>(a-b);
}

void G1EvacuationFailureObjsInHR::compact() {
assert(_oop_array == NULL, "Must be");
_oop_array = NEW_C_HEAP_ARRAY(oop, _objs_num, mtGC);
Node* cur = _head._next;
uint i = 0;
while (cur != NULL) {
assert(cur->_obj != NULL, "Must be");
_oop_array[i++] = cur->_obj;
cur = cur->_next;
}
clear_list();
}

void G1EvacuationFailureObjsInHR::sort() {
QuickSort::sort(_oop_array, _objs_num, order_oop, true);
}

void G1EvacuationFailureObjsInHR::iterate_internal(ObjectClosure* closure) {
oop prev = NULL;
for (uint i = 0; i < _objs_num; i++) {
assert(prev < _oop_array[i], "sanity");
closure->do_object(prev = _oop_array[i]);
}
clear_array();
}

void G1EvacuationFailureObjsInHR::clear_list() {
DEBUG_ONLY(uint i = _objs_num);
Node* cur = _head._next;
_head._next = NULL;

while (cur != NULL) {
Node* next = cur->_next;
cur->_next = NULL;
delete cur;
cur = next;
DEBUG_ONLY(i--);
}
assert(i == 0, "Must be, %u", i);
}

void G1EvacuationFailureObjsInHR::clear_array() {
FREE_C_HEAP_ARRAY(oop, _oop_array);
_oop_array = NULL;
_objs_num = 0;
}

void G1EvacuationFailureObjsInHR::reset() {
Atomic::store(&_tail, &_head);
}

G1EvacuationFailureObjsInHR::G1EvacuationFailureObjsInHR(uint region_idx) :
_region_idx(region_idx),
_objs_num(0),
_oop_array(NULL) {
reset();
}

G1EvacuationFailureObjsInHR::~G1EvacuationFailureObjsInHR() {
clear_list();
clear_array();
}

void G1EvacuationFailureObjsInHR::record(oop obj) {
assert(obj != NULL, "Must be");
Node* new_one = new Node(obj);
while (true) {
Node* t = Atomic::load(&_tail);
Node* next = Atomic::load(&t->_next);
while (next != NULL) {
t = next;
next = Atomic::load(&next->_next);
}
Node* old_one = Atomic::cmpxchg(&t->_next, (Node*)NULL, new_one);
if (old_one == NULL) {
Atomic::store(&_tail, new_one);
Atomic::inc(&_objs_num);
break;
}
}
}

void G1EvacuationFailureObjsInHR::iterate(ObjectClosure* closure) {
compact();
sort();
iterate_internal(closure);
reset();
}
@@ -0,0 +1,69 @@
/*
* 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 "memory/iterator.hpp"
#include "oops/oop.hpp"

// This class
// 1. records the objects per region which have failed to evacuate.
// 2. speeds up removing self forwarded ptrs in post evacuation phase.
//
class G1EvacuationFailureObjsInHR {
class Node : public CHeapObj<mtGC>{
friend G1EvacuationFailureObjsInHR;
private:
Node* volatile _next;
oop _obj;
public:
Node(oop obj = NULL) : _next(NULL), _obj(obj) {}
};

private:
const uint _region_idx;
Node _head;
Node* volatile _tail;
uint _objs_num;
oop* _oop_array;

private:
void compact();
void sort();
void iterate_internal(ObjectClosure* closure);
void clear_list();
void clear_array();
void reset();

public:
G1EvacuationFailureObjsInHR(uint region_idx);
~G1EvacuationFailureObjsInHR();

void record(oop obj);
void iterate(ObjectClosure* closure);
};


#endif //SHARE_GC_G1_G1EVACUATIONFAILUREOBJSINHR_HPP
@@ -602,6 +602,9 @@ oop G1ParScanThreadState::handle_evacuation_failure_par(oop old, markWord m) {
if (forward_ptr == NULL) {
// Forward-to-self succeeded. We are the "owner" of the object.
HeapRegion* r = _g1h->heap_region_containing(old);
// Records evac failure objs, this will help speed up iteration
// of these objs later in *remove self forward* phase of post evacuation.
r->record_evac_failure_obj(old);

if (_g1h->notify_region_failed_evacuation(r->hrm_index())) {
_g1h->hr_printer()->evac_failure(r);
@@ -108,6 +108,14 @@ void HeapRegion::handle_evacuation_failure() {
_next_marked_bytes = 0;
}

void HeapRegion::record_evac_failure_obj(oop obj) {
_evac_failure_objs.record(obj);
}
Copy link
Contributor

@tschatzl tschatzl Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be placed in the .inline.hpp file as it is called in performance sensitive code.


void HeapRegion::iterate_evac_failure_objs(ObjectClosure* closure) {
_evac_failure_objs.iterate(closure);
}
Copy link
Contributor

@tschatzl tschatzl Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This one is fine to be placed in the cpp file - the additional single call per HeapRegion does not matter)


void HeapRegion::unlink_from_list() {
set_next(NULL);
set_prev(NULL);
@@ -248,7 +256,8 @@ HeapRegion::HeapRegion(uint hrm_index,
_prev_marked_bytes(0), _next_marked_bytes(0),
_young_index_in_cset(-1),
_surv_rate_group(NULL), _age_index(G1SurvRateGroup::InvalidAgeIndex), _gc_efficiency(-1.0),
_node_index(G1NUMA::UnknownNodeIndex)
_node_index(G1NUMA::UnknownNodeIndex),
_evac_failure_objs(hrm_index)
{
assert(Universe::on_page_boundary(mr.start()) && Universe::on_page_boundary(mr.end()),
"invalid space boundaries");
@@ -26,6 +26,7 @@
#define SHARE_GC_G1_HEAPREGION_HPP

#include "gc/g1/g1BlockOffsetTable.hpp"
#include "gc/g1/g1EvacuationFailureObjsInHR.hpp"
#include "gc/g1/g1HeapRegionTraceType.hpp"
#include "gc/g1/g1SurvRateGroup.hpp"
#include "gc/g1/heapRegionTracer.hpp"
@@ -258,6 +259,8 @@ class HeapRegion : public CHeapObj<mtGC> {

uint _node_index;

G1EvacuationFailureObjsInHR _evac_failure_objs;

void report_region_type_change(G1HeapRegionTraceType::Type to);

// Returns whether the given object address refers to a dead object, and either the
@@ -554,6 +557,11 @@ class HeapRegion : public CHeapObj<mtGC> {

// Update the region state after a failed evacuation.
void handle_evacuation_failure();
// Records evac failure objs during evaucation, this will help speed up iteration
Copy link
Contributor

@tschatzl tschatzl Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/evucation/evacuation.

Please try to avoid "evac failure objs" in text as it seems strange to me grammatically, and isn't particularly obvious what this is to readers not working on this all the time. Just say "Record an object that failed evacuation within this region.` I do not think the second part of the sentence is necessary, i.e. talking about speeding up something here.

// of these objs later in *remove self forward* phase of post evacuation.
void record_evac_failure_obj(oop obj);
// Iterates evac failure objs which are recorded during evcauation.
Copy link
Contributor

@tschatzl tschatzl Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/evauation/evacuation

Better would probably be "Applies the given closure to all previously recorded objects that failed evacuation in ascending address order"

void iterate_evac_failure_objs(ObjectClosure* closure);

// Iterate over the objects overlapping the given memory region, applying cl
// to all references in the region. This is a helper for