Skip to content

Commit

Permalink
8267924: Misleading G1 eager reclaim detail logging
Browse files Browse the repository at this point in the history 
Reviewed-by: ayang, sjohanss
  • Loading branch information
Thomas Schatzl committed Jun 7, 2021
1 parent e4d0454 commit 15715a8
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 59 deletions.
10 changes: 10 additions & 0 deletions src/hotspot/share/gc/g1/g1CollectedHeap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3391,6 +3391,16 @@ class G1PrepareEvacuationTask : public AbstractGangTask {
_g1h->set_humongous_reclaim_candidate(index, false);
_g1h->register_region_with_region_attr(hr);
}
log_debug(gc, humongous)("Humongous region %u (object size " SIZE_FORMAT " @ " PTR_FORMAT ") remset " SIZE_FORMAT " code roots " SIZE_FORMAT " marked %d reclaim candidate %d type array %d",
index,
(size_t)cast_to_oop(hr->bottom())->size() * HeapWordSize,
p2i(hr->bottom()),
hr->rem_set()->occupied(),
hr->rem_set()->strong_code_roots_list_length(),
_g1h->concurrent_mark()->next_mark_bitmap()->is_marked(hr->bottom()),
_g1h->is_humongous_reclaim_candidate(index),
cast_to_oop(hr->bottom())->is_typeArray()
);
_worker_humongous_total++;

return false;
Expand Down
101 changes: 44 additions & 57 deletions src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,43 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
uint _humongous_objects_reclaimed;
uint _humongous_regions_reclaimed;
size_t _freed_bytes;
public:

// Returns whether the given humongous object defined by the start region index
// is reclaimable.
//
// At this point in the garbage collection, checking whether the humongous object
// is still a candidate is sufficient because:
//
// - if it has not been a candidate at the start of collection, it will never
// changed to be a candidate during the gc (and live).
// - any found outstanding (i.e. in the DCQ, or in its remembered set)
// references will set the candidate state to false.
// - there can be no references from within humongous starts regions referencing
// the object because we never allocate other objects into them.
// (I.e. there can be no intra-region references)
//
// It is not required to check whether the object has been found dead by marking
// or not, in fact it would prevent reclamation within a concurrent cycle, as
// all objects allocated during that time are considered live.
// SATB marking is even more conservative than the remembered set.
// So if at this point in the collection we did not find a reference during gc
// (or it had enough references to not be a candidate, having many remembered
// set entries), nobody has a reference to it.
// At the start of collection we flush all refinement logs, and remembered sets
// are completely up-to-date wrt to references to the humongous object.
//
// So there is no need to re-check remembered set size of the humongous region.
//
// Other implementation considerations:
// - never consider object arrays at this time because they would pose
// considerable effort for cleaning up the the remembered sets. This is
// required because stale remembered sets might reference locations that
// are currently allocated into.
bool is_reclaimable(uint region_idx) const {
return G1CollectedHeap::heap()->is_humongous_reclaim_candidate(region_idx);
}

public:
G1FreeHumongousRegionClosure() :
_humongous_objects_reclaimed(0),
_humongous_regions_reclaimed(0),
Expand All @@ -104,70 +139,17 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
return false;
}

G1CollectedHeap* g1h = G1CollectedHeap::heap();

oop obj = cast_to_oop(r->bottom());
G1CMBitMap* next_bitmap = g1h->concurrent_mark()->next_mark_bitmap();

// The following checks whether the humongous object is live are sufficient.
// The main additional check (in addition to having a reference from the roots
// or the young gen) is whether the humongous object has a remembered set entry.
//
// A humongous object cannot be live if there is no remembered set for it
// because:
// - there can be no references from within humongous starts regions referencing
// the object because we never allocate other objects into them.
// (I.e. there are no intra-region references that may be missed by the
// remembered set)
// - as soon there is a remembered set entry to the humongous starts region
// (i.e. it has "escaped" to an old object) this remembered set entry will stay
// until the end of a concurrent mark.
//
// It is not required to check whether the object has been found dead by marking
// or not, in fact it would prevent reclamation within a concurrent cycle, as
// all objects allocated during that time are considered live.
// SATB marking is even more conservative than the remembered set.
// So if at this point in the collection there is no remembered set entry,
// nobody has a reference to it.
// At the start of collection we flush all refinement logs, and remembered sets
// are completely up-to-date wrt to references to the humongous object.
//
// Other implementation considerations:
// - never consider object arrays at this time because they would pose
// considerable effort for cleaning up the the remembered sets. This is
// required because stale remembered sets might reference locations that
// are currently allocated into.
uint region_idx = r->hrm_index();
if (!g1h->is_humongous_reclaim_candidate(region_idx) ||
!r->rem_set()->is_empty()) {
log_debug(gc, humongous)("Live humongous region %u object size " SIZE_FORMAT " start " PTR_FORMAT " with remset " SIZE_FORMAT " code roots " SIZE_FORMAT " is marked %d reclaim candidate %d type array %d",
region_idx,
(size_t)obj->size() * HeapWordSize,
p2i(r->bottom()),
r->rem_set()->occupied(),
r->rem_set()->strong_code_roots_list_length(),
next_bitmap->is_marked(r->bottom()),
g1h->is_humongous_reclaim_candidate(region_idx),
obj->is_typeArray()
);
if (!is_reclaimable(region_idx)) {
return false;
}

oop obj = cast_to_oop(r->bottom());
guarantee(obj->is_typeArray(),
"Only eagerly reclaiming type arrays is supported, but the object "
PTR_FORMAT " is not.", p2i(r->bottom()));

log_debug(gc, humongous)("Dead humongous region %u object size " SIZE_FORMAT " start " PTR_FORMAT " with remset " SIZE_FORMAT " code roots " SIZE_FORMAT " is marked %d reclaim candidate %d type array %d",
region_idx,
(size_t)obj->size() * HeapWordSize,
p2i(r->bottom()),
r->rem_set()->occupied(),
r->rem_set()->strong_code_roots_list_length(),
next_bitmap->is_marked(r->bottom()),
g1h->is_humongous_reclaim_candidate(region_idx),
obj->is_typeArray()
);

G1CollectedHeap* g1h = G1CollectedHeap::heap();
G1ConcurrentMark* const cm = g1h->concurrent_mark();
cm->humongous_object_eagerly_reclaimed(r);
assert(!cm->is_marked_in_prev_bitmap(obj) && !cm->is_marked_in_next_bitmap(obj),
Expand All @@ -186,6 +168,11 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
r = next;
} while (r != nullptr);

log_debug(gc, humongous)("Reclaimed humongous region %u (object size " SIZE_FORMAT " @ " PTR_FORMAT ")",
region_idx,
(size_t)obj->size() * HeapWordSize,
p2i(r->bottom())
);
return false;
}

Expand Down
5 changes: 3 additions & 2 deletions test/hotspot/jtreg/gc/g1/TestG1TraceEagerReclaimHumongousObjects.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public static void main(String[] args) throws Exception {

OutputAnalyzer output = new OutputAnalyzer(pb.start());

System.out.println(output.getStdout());
// As G1ReclaimDeadHumongousObjectsAtYoungGC is set(default), below logs should be displayed.
output.shouldContain("Humongous Reclaim");
output.shouldContain("Humongous Total");
Expand All @@ -60,8 +61,8 @@ public static void main(String[] args) throws Exception {

// As G1TraceReclaimDeadHumongousObjectsAtYoungGC is set and GCWithHumongousObjectTest has humongous objects,
// these logs should be displayed.
output.shouldContain("Live humongous");
output.shouldContain("Dead humongous region");
output.shouldContain("Humongous region");
output.shouldContain("Reclaimed humongous region");
output.shouldHaveExitValue(0);
}

Expand Down

0 comments on commit 15715a8

Please sign in to comment.