8205022: ZGC: SoftReferences not always cleared before throwing OOME
Reviewed-by: stefank, eosterlund
--- a/src/hotspot/share/gc/z/zDriver.cpp Fri Jun 15 13:31:20 2018 +0200
+++ b/src/hotspot/share/gc/z/zDriver.cpp Fri Jun 15 13:31:27 2018 +0200
@@ -126,6 +126,26 @@
}
};
+static bool should_clear_soft_references() {
+ // Clear if one or more allocations have stalled
+ const bool stalled = ZHeap::heap()->is_alloc_stalled();
+ if (stalled) {
+ // Clear
+ return true;
+ }
+
+ // Clear if implied by the GC cause
+ const GCCause::Cause cause = ZCollectedHeap::heap()->gc_cause();
+ if (cause == GCCause::_wb_full_gc ||
+ cause == GCCause::_metadata_GC_clear_soft_refs) {
+ // Clear
+ return true;
+ }
+
+ // Don't clear
+ return false;
+}
+
class ZMarkStartClosure : public ZOperationClosure {
public:
virtual const char* name() const {
@@ -140,6 +160,10 @@
ZStatTimer timer(ZPhasePauseMarkStart);
ZServiceabilityMarkStartTracer tracer;
+ // Setup soft reference policy
+ const bool clear = should_clear_soft_references();
+ ZHeap::heap()->set_soft_reference_policy(clear);
+
ZCollectedHeap::heap()->increment_total_collections(true /* full */);
ZHeap::heap()->mark_start();
@@ -247,40 +271,11 @@
return _gc_cycle_port.receive();
}
-class ZSoftReferencePolicyScope : public StackObj {
-private:
- bool should_clear_soft_reference(GCCause::Cause cause) const {
- const bool clear = ZCollectedHeap::heap()->soft_ref_policy()->should_clear_all_soft_refs();
-
- // Clear all soft reference if the policy says so, or if
- // the GC cause indicates that we're running low on memory.
- return clear ||
- cause == GCCause::_z_allocation_stall ||
- cause == GCCause::_metadata_GC_clear_soft_refs;
- }
-
- void clear_should_clear_soft_reference() const {
- ZCollectedHeap::heap()->soft_ref_policy()->set_should_clear_all_soft_refs(false);
- }
-
-public:
- ZSoftReferencePolicyScope(GCCause::Cause cause) {
- const bool clear = should_clear_soft_reference(cause);
- ZHeap::heap()->set_soft_reference_policy(clear);
- clear_should_clear_soft_reference();
- }
-
- ~ZSoftReferencePolicyScope() {
- Universe::update_heap_info_at_gc();
- }
-};
-
class ZDriverCycleScope : public StackObj {
private:
- GCIdMark _gc_id;
- GCCauseSetter _gc_cause_setter;
- ZSoftReferencePolicyScope _soft_ref_policy;
- ZStatTimer _timer;
+ GCIdMark _gc_id;
+ GCCauseSetter _gc_cause_setter;
+ ZStatTimer _timer;
bool should_boost_worker_threads(GCCause::Cause cause) const {
return cause == GCCause::_java_lang_system_gc ||
@@ -291,7 +286,6 @@
ZDriverCycleScope(GCCause::Cause cause) :
_gc_id(),
_gc_cause_setter(ZCollectedHeap::heap(), cause),
- _soft_ref_policy(cause),
_timer(ZPhaseCycle) {
// Update statistics
ZStatCycle::at_start();
@@ -308,6 +302,9 @@
// Update statistics
ZStatCycle::at_end(boost_factor);
+
+ // Update data used by soft reference policy
+ Universe::update_heap_info_at_gc();
}
};
--- a/src/hotspot/share/gc/z/zHeap.hpp Fri Jun 15 13:31:20 2018 +0200
+++ b/src/hotspot/share/gc/z/zHeap.hpp Fri Jun 15 13:31:27 2018 +0200
@@ -124,6 +124,7 @@
uintptr_t alloc_object(size_t size);
uintptr_t alloc_object_for_relocation(size_t size);
void undo_alloc_object_for_relocation(uintptr_t addr, size_t size);
+ bool is_alloc_stalled() const;
void check_out_of_memory();
// Marking
--- a/src/hotspot/share/gc/z/zHeap.inline.hpp Fri Jun 15 13:31:20 2018 +0200
+++ b/src/hotspot/share/gc/z/zHeap.inline.hpp Fri Jun 15 13:31:27 2018 +0200
@@ -89,6 +89,10 @@
_object_allocator.undo_alloc_object_for_relocation(page, addr, size);
}
+inline bool ZHeap::is_alloc_stalled() const {
+ return _page_allocator.is_alloc_stalled();
+}
+
inline void ZHeap::check_out_of_memory() {
_page_allocator.check_out_of_memory();
}
--- a/src/hotspot/share/gc/z/zPageAllocator.cpp Fri Jun 15 13:31:20 2018 +0200
+++ b/src/hotspot/share/gc/z/zPageAllocator.cpp Fri Jun 15 13:31:27 2018 +0200
@@ -459,24 +459,25 @@
satisfy_alloc_queue();
}
+bool ZPageAllocator::is_alloc_stalled() const {
+ assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint");
+ return !_queue.is_empty();
+}
+
void ZPageAllocator::check_out_of_memory() {
ZLocker locker(&_lock);
- ZPageAllocRequest* const first = _queue.first();
- if (first == NULL) {
- // Allocation queue is empty
- return;
- }
-
- // Fail the allocation request if it was enqueued before the
+ // Fail allocation requests that were enqueued before the
// last GC cycle started, otherwise start a new GC cycle.
- if (first->total_collections() < ZCollectedHeap::heap()->total_collections()) {
- // Out of memory, fail all enqueued requests
- for (ZPageAllocRequest* request = _queue.remove_first(); request != NULL; request = _queue.remove_first()) {
- request->satisfy(NULL);
+ for (ZPageAllocRequest* request = _queue.first(); request != NULL; request = _queue.first()) {
+ if (request->total_collections() == ZCollectedHeap::heap()->total_collections()) {
+ // Start a new GC cycle, keep allocation requests enqueued
+ request->satisfy(gc_marker);
+ return;
}
- } else {
- // Start another GC cycle, keep all enqueued requests
- first->satisfy(gc_marker);
+
+ // Out of memory, fail allocation request
+ _queue.remove_first();
+ request->satisfy(NULL);
}
}
--- a/src/hotspot/share/gc/z/zPageAllocator.hpp Fri Jun 15 13:31:20 2018 +0200
+++ b/src/hotspot/share/gc/z/zPageAllocator.hpp Fri Jun 15 13:31:27 2018 +0200
@@ -102,6 +102,7 @@
void flip_pre_mapped();
+ bool is_alloc_stalled() const;
void check_out_of_memory();
};