Skip to content

Commit 15715a8

Browse files
author
Thomas Schatzl
committed
8267924: Misleading G1 eager reclaim detail logging
Reviewed-by: ayang, sjohanss
1 parent e4d0454 commit 15715a8

File tree

3 files changed

+57
-59
lines changed

3 files changed

+57
-59
lines changed

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -3391,6 +3391,16 @@ class G1PrepareEvacuationTask : public AbstractGangTask {
33913391
_g1h->set_humongous_reclaim_candidate(index, false);
33923392
_g1h->register_region_with_region_attr(hr);
33933393
}
3394+
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",
3395+
index,
3396+
(size_t)cast_to_oop(hr->bottom())->size() * HeapWordSize,
3397+
p2i(hr->bottom()),
3398+
hr->rem_set()->occupied(),
3399+
hr->rem_set()->strong_code_roots_list_length(),
3400+
_g1h->concurrent_mark()->next_mark_bitmap()->is_marked(hr->bottom()),
3401+
_g1h->is_humongous_reclaim_candidate(index),
3402+
cast_to_oop(hr->bottom())->is_typeArray()
3403+
);
33943404
_worker_humongous_total++;
33953405

33963406
return false;

src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp

+44-57
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,43 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
9191
uint _humongous_objects_reclaimed;
9292
uint _humongous_regions_reclaimed;
9393
size_t _freed_bytes;
94-
public:
9594

95+
// Returns whether the given humongous object defined by the start region index
96+
// is reclaimable.
97+
//
98+
// At this point in the garbage collection, checking whether the humongous object
99+
// is still a candidate is sufficient because:
100+
//
101+
// - if it has not been a candidate at the start of collection, it will never
102+
// changed to be a candidate during the gc (and live).
103+
// - any found outstanding (i.e. in the DCQ, or in its remembered set)
104+
// references will set the candidate state to false.
105+
// - there can be no references from within humongous starts regions referencing
106+
// the object because we never allocate other objects into them.
107+
// (I.e. there can be no intra-region references)
108+
//
109+
// It is not required to check whether the object has been found dead by marking
110+
// or not, in fact it would prevent reclamation within a concurrent cycle, as
111+
// all objects allocated during that time are considered live.
112+
// SATB marking is even more conservative than the remembered set.
113+
// So if at this point in the collection we did not find a reference during gc
114+
// (or it had enough references to not be a candidate, having many remembered
115+
// set entries), nobody has a reference to it.
116+
// At the start of collection we flush all refinement logs, and remembered sets
117+
// are completely up-to-date wrt to references to the humongous object.
118+
//
119+
// So there is no need to re-check remembered set size of the humongous region.
120+
//
121+
// Other implementation considerations:
122+
// - never consider object arrays at this time because they would pose
123+
// considerable effort for cleaning up the the remembered sets. This is
124+
// required because stale remembered sets might reference locations that
125+
// are currently allocated into.
126+
bool is_reclaimable(uint region_idx) const {
127+
return G1CollectedHeap::heap()->is_humongous_reclaim_candidate(region_idx);
128+
}
129+
130+
public:
96131
G1FreeHumongousRegionClosure() :
97132
_humongous_objects_reclaimed(0),
98133
_humongous_regions_reclaimed(0),
@@ -104,70 +139,17 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
104139
return false;
105140
}
106141

107-
G1CollectedHeap* g1h = G1CollectedHeap::heap();
108-
109-
oop obj = cast_to_oop(r->bottom());
110-
G1CMBitMap* next_bitmap = g1h->concurrent_mark()->next_mark_bitmap();
111-
112-
// The following checks whether the humongous object is live are sufficient.
113-
// The main additional check (in addition to having a reference from the roots
114-
// or the young gen) is whether the humongous object has a remembered set entry.
115-
//
116-
// A humongous object cannot be live if there is no remembered set for it
117-
// because:
118-
// - there can be no references from within humongous starts regions referencing
119-
// the object because we never allocate other objects into them.
120-
// (I.e. there are no intra-region references that may be missed by the
121-
// remembered set)
122-
// - as soon there is a remembered set entry to the humongous starts region
123-
// (i.e. it has "escaped" to an old object) this remembered set entry will stay
124-
// until the end of a concurrent mark.
125-
//
126-
// It is not required to check whether the object has been found dead by marking
127-
// or not, in fact it would prevent reclamation within a concurrent cycle, as
128-
// all objects allocated during that time are considered live.
129-
// SATB marking is even more conservative than the remembered set.
130-
// So if at this point in the collection there is no remembered set entry,
131-
// nobody has a reference to it.
132-
// At the start of collection we flush all refinement logs, and remembered sets
133-
// are completely up-to-date wrt to references to the humongous object.
134-
//
135-
// Other implementation considerations:
136-
// - never consider object arrays at this time because they would pose
137-
// considerable effort for cleaning up the the remembered sets. This is
138-
// required because stale remembered sets might reference locations that
139-
// are currently allocated into.
140142
uint region_idx = r->hrm_index();
141-
if (!g1h->is_humongous_reclaim_candidate(region_idx) ||
142-
!r->rem_set()->is_empty()) {
143-
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",
144-
region_idx,
145-
(size_t)obj->size() * HeapWordSize,
146-
p2i(r->bottom()),
147-
r->rem_set()->occupied(),
148-
r->rem_set()->strong_code_roots_list_length(),
149-
next_bitmap->is_marked(r->bottom()),
150-
g1h->is_humongous_reclaim_candidate(region_idx),
151-
obj->is_typeArray()
152-
);
143+
if (!is_reclaimable(region_idx)) {
153144
return false;
154145
}
155146

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

160-
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",
161-
region_idx,
162-
(size_t)obj->size() * HeapWordSize,
163-
p2i(r->bottom()),
164-
r->rem_set()->occupied(),
165-
r->rem_set()->strong_code_roots_list_length(),
166-
next_bitmap->is_marked(r->bottom()),
167-
g1h->is_humongous_reclaim_candidate(region_idx),
168-
obj->is_typeArray()
169-
);
170-
152+
G1CollectedHeap* g1h = G1CollectedHeap::heap();
171153
G1ConcurrentMark* const cm = g1h->concurrent_mark();
172154
cm->humongous_object_eagerly_reclaimed(r);
173155
assert(!cm->is_marked_in_prev_bitmap(obj) && !cm->is_marked_in_next_bitmap(obj),
@@ -186,6 +168,11 @@ class G1FreeHumongousRegionClosure : public HeapRegionClosure {
186168
r = next;
187169
} while (r != nullptr);
188170

171+
log_debug(gc, humongous)("Reclaimed humongous region %u (object size " SIZE_FORMAT " @ " PTR_FORMAT ")",
172+
region_idx,
173+
(size_t)obj->size() * HeapWordSize,
174+
p2i(r->bottom())
175+
);
189176
return false;
190177
}
191178

test/hotspot/jtreg/gc/g1/TestG1TraceEagerReclaimHumongousObjects.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public static void main(String[] args) throws Exception {
5252

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

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

6162
// As G1TraceReclaimDeadHumongousObjectsAtYoungGC is set and GCWithHumongousObjectTest has humongous objects,
6263
// these logs should be displayed.
63-
output.shouldContain("Live humongous");
64-
output.shouldContain("Dead humongous region");
64+
output.shouldContain("Humongous region");
65+
output.shouldContain("Reclaimed humongous region");
6566
output.shouldHaveExitValue(0);
6667
}
6768

0 commit comments

Comments
 (0)