--- old/src/hotspot/share/gc/z/zBarrier.hpp 2019-11-11 21:16:44.339945436 +0100 +++ new/src/hotspot/share/gc/z/zBarrier.hpp 2019-11-11 21:16:43.834928696 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2019, Oracle 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 @@ -41,6 +41,8 @@ static const bool Publish = true; static const bool Overflow = false; + static void self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_addr); + template static oop barrier(volatile oop* p, oop o); template static oop weak_barrier(volatile oop* p, oop o); template static void root_barrier(oop* p, oop o); --- old/src/hotspot/share/gc/z/zBarrier.inline.hpp 2019-11-11 21:16:44.978966618 +0100 +++ new/src/hotspot/share/gc/z/zBarrier.inline.hpp 2019-11-11 21:16:44.503950873 +0100 @@ -32,66 +32,82 @@ #include "oops/oop.hpp" #include "runtime/atomic.hpp" +inline void ZBarrier::self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_addr) { + if (heal_addr == 0) { + // Never heal with null since it interacts badly with reference processing. + // A mutator clearing an oop would be similar to calling Reference.clear(), + // which would make the reference non-discoverable or silently dropped + // by the reference processor. + return; + } + + for (;;) { + if (addr == heal_addr) { + // Already healed + return; + } + + // Heal + const uintptr_t prev_addr = Atomic::cmpxchg(heal_addr, (volatile uintptr_t*)p, addr); + if (prev_addr == addr) { + // Success + return; + } + + if (ZAddress::is_good_or_null(prev_addr)) { + // No need to heal + return; + } + + // The oop location was healed by another barrier, but it is still not + // good or null. Re-apply healing to make sure the oop is not left with + // weaker (remapped or finalizable) metadata bits than what this barrier + // tried to apply. + assert(ZAddress::offset(prev_addr) == ZAddress::offset(heal_addr), "Invalid offset"); + addr = prev_addr; + } +} + template inline oop ZBarrier::barrier(volatile oop* p, oop o) { uintptr_t addr = ZOop::to_address(o); + // Fast path if (fast_path(addr)) { return ZOop::from_address(addr); } - uintptr_t good_addr = slow_path(addr); - const oop result = ZOop::from_address(good_addr); - - // Self heal, but only if the address was actually updated by the slow path, - // which might not be the case, e.g. when marking through an already good oop. - while (p != NULL && good_addr != addr) { - const uintptr_t prev_addr = Atomic::cmpxchg(good_addr, (volatile uintptr_t*)p, addr); - if (prev_addr != addr) { - // Some other thread overwrote the oop. If this oop was updated by a - // weak barrier the new oop might not be good, in which case we need - // to re-apply this barrier. - addr = prev_addr; - // Fast path - if (fast_path(addr)) { - break; - } + // Slow path + const uintptr_t good_addr = slow_path(addr); - // Slow path - good_addr = slow_path(addr); - } + if (p != NULL) { + self_heal(p, addr, good_addr); } - return result; + return ZOop::from_address(good_addr); } template inline oop ZBarrier::weak_barrier(volatile oop* p, oop o) { const uintptr_t addr = ZOop::to_address(o); + // Fast path if (fast_path(addr)) { // Return the good address instead of the weak good address // to ensure that the currently active heap view is used. return ZOop::from_address(ZAddress::good_or_null(addr)); } + // Slow path const uintptr_t good_addr = slow_path(addr); - const oop result = ZOop::from_address(good_addr); - // Self heal unless the address returned from the slow path is null, - // in which case resurrection was blocked and we must let the reference - // processor clear the oop. Mutators are not allowed to clear oops in - // these cases, since that would be similar to calling Reference.clear(), - // which would make the reference non-discoverable or silently dropped - // by the reference processor. - if (p != NULL && good_addr != 0) { - // The slow path returns a good/marked address, but we never mark oops - // in a weak load barrier so we always self heal with the remapped address. - const uintptr_t weak_good_addr = ZAddress::remapped(good_addr); - Atomic::cmpxchg(weak_good_addr, (volatile uintptr_t*)p, addr); + if (p != NULL) { + // The slow path returns a good/marked address or null, but we never mark + // oops in a weak load barrier so we always heal with the remapped address. + self_heal(p, addr, ZAddress::remapped_or_null(good_addr)); } - return result; + return ZOop::from_address(good_addr); } template @@ -166,7 +182,7 @@ verify_on_weak(p); if (ZResurrection::is_blocked()) { - return weak_barrier(p, o); + return barrier(p, o); } return load_barrier_on_oop_field_preloaded(p, o); @@ -174,7 +190,7 @@ inline oop ZBarrier::load_barrier_on_phantom_oop_field_preloaded(volatile oop* p, oop o) { if (ZResurrection::is_blocked()) { - return weak_barrier(p, o); + return barrier(p, o); } return load_barrier_on_oop_field_preloaded(p, o); @@ -211,7 +227,7 @@ verify_on_weak(p); if (ZResurrection::is_blocked()) { - return weak_barrier(p, o); + return barrier(p, o); } return weak_load_barrier_on_oop_field_preloaded(p, o); @@ -228,7 +244,7 @@ inline oop ZBarrier::weak_load_barrier_on_phantom_oop_field_preloaded(volatile oop* p, oop o) { if (ZResurrection::is_blocked()) { - return weak_barrier(p, o); + return barrier(p, o); } return weak_load_barrier_on_oop_field_preloaded(p, o); --- old/src/hotspot/share/gc/z/zHeap.cpp 2019-11-11 21:16:45.616987767 +0100 +++ new/src/hotspot/share/gc/z/zHeap.cpp 2019-11-11 21:16:45.143972088 +0100 @@ -40,6 +40,7 @@ #include "logging/log.hpp" #include "memory/iterator.hpp" #include "memory/resourceArea.hpp" +#include "runtime/handshake.hpp" #include "runtime/safepoint.hpp" #include "runtime/thread.hpp" #include "utilities/debug.hpp" @@ -315,7 +316,7 @@ // Process weak roots _weak_roots_processor.process_weak_roots(); - // Prepare to unload unused classes and code + // Prepare to unload stale metadata and nmethods _unload.prepare(); return true; @@ -325,6 +326,11 @@ _reference_processor.set_soft_reference_policy(clear); } +class ZRendezvousClosure : public ThreadClosure { +public: + virtual void do_thread(Thread* thread) {} +}; + void ZHeap::process_non_strong_references() { // Process Soft/Weak/Final/PhantomReferences _reference_processor.process_references(); @@ -332,8 +338,22 @@ // Process concurrent weak roots _weak_roots_processor.process_concurrent_weak_roots(); - // Unload unused classes and code - _unload.unload(); + // Unlink stale metadata and nmethods + _unload.unlink(); + + // Perform a handshake. This is needed 1) to make sure that stale + // metadata and nmethods are no longer observable. And 2), to + // prevent the race where a mutator first loads an oop, which is + // logically null but not yet cleared. Then this oop gets cleared + // by the reference processor and resurrection is unblocked. At + // this point the mutator could see the unblocked state and pass + // this invalid oop through the normal barrier path, which would + // incorrectly try to mark the oop. + ZRendezvousClosure cl; + Handshake::execute(&cl); + + // Purge stale metadata and nmethods that were unlinked + _unload.purge(); // Unblock resurrection of weak/phantom references ZResurrection::unblock(); @@ -405,7 +425,7 @@ void ZHeap::relocate_start() { assert(SafepointSynchronize::is_at_safepoint(), "Should be at safepoint"); - // Finish unloading of classes and code + // Finish unloading stale metadata and nmethods _unload.finish(); // Flip address view --- old/src/hotspot/share/gc/z/zUnload.cpp 2019-11-11 21:16:46.324011203 +0100 +++ new/src/hotspot/share/gc/z/zUnload.cpp 2019-11-11 21:16:45.791993568 +0100 @@ -36,7 +36,8 @@ #include "gc/z/zUnload.hpp" #include "oops/access.inline.hpp" -static const ZStatSubPhase ZSubPhaseConcurrentClassesUnload("Concurrent Classes Unload"); +static const ZStatSubPhase ZSubPhaseConcurrentClassesUnlink("Concurrent Classes Unlink"); +static const ZStatSubPhase ZSubPhaseConcurrentClassesPurge("Concurrent Classes Purge"); class ZIsUnloadingOopClosure : public OopClosure { private: @@ -126,6 +127,11 @@ } void ZUnload::unlink() { + if (!ClassUnloading) { + return; + } + + ZStatTimer timer(ZSubPhaseConcurrentClassesUnlink); SuspendibleThreadSetJoiner sts; bool unloading_occurred; @@ -135,13 +141,17 @@ } Klass::clean_weak_klass_links(unloading_occurred); - ZNMethod::unlink(_workers, unloading_occurred); - DependencyContext::cleaning_end(); } void ZUnload::purge() { + if (!ClassUnloading) { + return; + } + + ZStatTimer timer(ZSubPhaseConcurrentClassesPurge); + { SuspendibleThreadSetJoiner sts; ZNMethod::purge(_workers); @@ -151,32 +161,6 @@ CodeCache::purge_exception_caches(); } -class ZUnloadRendezvousClosure : public ThreadClosure { -public: - void do_thread(Thread* thread) {} -}; - -void ZUnload::unload() { - ZUnloadRendezvousClosure cl; - if (!ClassUnloading) { - // Even when we don't use class unloading, we want a handshake to - // close the resurrection block window. - Handshake::execute(&cl); - return; - } - - ZStatTimer timer(ZSubPhaseConcurrentClassesUnload); - - // Unlink stale metadata and nmethods - unlink(); - - // Make sure stale metadata and nmethods are no longer observable - Handshake::execute(&cl); - - // Purge stale metadata and nmethods that were unlinked - purge(); -} - void ZUnload::finish() { // Resize and verify metaspace MetaspaceGC::compute_new_size(); --- old/src/hotspot/share/gc/z/zUnload.hpp 2019-11-11 21:16:47.035034771 +0100 +++ new/src/hotspot/share/gc/z/zUnload.hpp 2019-11-11 21:16:46.499017004 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2019, Oracle 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 @@ -30,14 +30,12 @@ private: ZWorkers* const _workers; - void unlink(); - void purge(); - public: ZUnload(ZWorkers* workers); void prepare(); - void unload(); + void unlink(); + void purge(); void finish(); };