# HG changeset patch # User pliden # Date 1529062287 -7200 # Node ID 6464882498b5e85b5b4df0bf1ddb12267792d15d # Parent 0cc4711c2112f57f5de325ab8b76d3d265b417b1 8205022: ZGC: SoftReferences not always cleared before throwing OOME Reviewed-by: stefank, eosterlund diff -r 0cc4711c2112 -r 6464882498b5 src/hotspot/share/gc/z/zDriver.cpp --- 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(); } }; diff -r 0cc4711c2112 -r 6464882498b5 src/hotspot/share/gc/z/zHeap.hpp --- 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 diff -r 0cc4711c2112 -r 6464882498b5 src/hotspot/share/gc/z/zHeap.inline.hpp --- 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(); } diff -r 0cc4711c2112 -r 6464882498b5 src/hotspot/share/gc/z/zPageAllocator.cpp --- 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); } } diff -r 0cc4711c2112 -r 6464882498b5 src/hotspot/share/gc/z/zPageAllocator.hpp --- 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(); };