# HG changeset patch # User zgu # Date 1579275882 18000 # Fri Jan 17 10:44:42 2020 -0500 # Node ID 5f4ddf4773093b639c20db3970642490c0441dfd # Parent d8341e9ad86d60a0f462d44182a5efb0d62c3e0f 8236880: Shenandoah: Move string dedup cleanup into concurrent phase diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp @@ -25,6 +25,7 @@ #define SHARE_GC_SHENANDOAH_SHENANDOAHCLOSURES_HPP #include "memory/iterator.hpp" +#include "oops/accessDecorators.hpp" class ShenandoahHeap; class ShenandoahMarkingContext; @@ -81,6 +82,7 @@ inline void do_oop_work(T* p); }; +template class ShenandoahEvacuateUpdateRootsClosure: public BasicOopIterateClosure { private: ShenandoahHeap* _heap; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp @@ -106,12 +106,14 @@ void ShenandoahTraversalUpdateRefsClosure::do_oop(oop* p) { do_oop_work(p); } void ShenandoahTraversalUpdateRefsClosure::do_oop(narrowOop* p) { do_oop_work(p); } -ShenandoahEvacuateUpdateRootsClosure::ShenandoahEvacuateUpdateRootsClosure() : +template +ShenandoahEvacuateUpdateRootsClosure::ShenandoahEvacuateUpdateRootsClosure() : _heap(ShenandoahHeap::heap()), _thread(Thread::current()) { } +template template -void ShenandoahEvacuateUpdateRootsClosure::do_oop_work(T* p) { +void ShenandoahEvacuateUpdateRootsClosure::do_oop_work(T* p) { assert(_heap->is_concurrent_root_in_progress(), "Only do this when evacuation is in progress"); T o = RawAccess<>::oop_load(p); @@ -124,15 +126,17 @@ if (resolved == obj) { resolved = _heap->evacuate_object(obj, _thread); } - RawAccess::oop_store(p, resolved); + RawAccess::oop_store(p, resolved); } } } -void ShenandoahEvacuateUpdateRootsClosure::do_oop(oop* p) { +template +void ShenandoahEvacuateUpdateRootsClosure::do_oop(oop* p) { do_oop_work(p); } -void ShenandoahEvacuateUpdateRootsClosure::do_oop(narrowOop* p) { +template +void ShenandoahEvacuateUpdateRootsClosure::do_oop(narrowOop* p) { do_oop_work(p); } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -1064,7 +1064,7 @@ void work(uint worker_id) { ShenandoahParallelWorkerSession worker_session(worker_id); ShenandoahEvacOOMScope oom_evac_scope; - ShenandoahEvacuateUpdateRootsClosure cl; + ShenandoahEvacuateUpdateRootsClosure<> cl; MarkingCodeBlobClosure blobsCl(&cl, CodeBlobToOopClosure::FixRelocations); _rp->roots_do(worker_id, &cl); } @@ -1306,10 +1306,9 @@ ShenandoahHeapIterationRootScanner rp; ObjectIterateScanRootClosure oops(&_aux_bit_map, &oop_stack); - // If we are unloading classes right now, we should not touch weak roots, - // on the off-chance we would evacuate them and make them live accidentally. - // In other cases, we have to scan all roots. - if (is_evacuation_in_progress() && unload_classes()) { + // When concurrent root is in progress, weak roots may contain dead oops, they should not be used + // for root scanning. + if (is_concurrent_root_in_progress()) { rp.strong_roots_do(&oops); } else { rp.roots_do(&oops); @@ -1577,6 +1576,7 @@ if (ShenandoahConcurrentRoots::should_do_concurrent_roots()) { types = ShenandoahRootVerifier::combine(ShenandoahRootVerifier::JNIHandleRoots, ShenandoahRootVerifier::WeakRoots); types = ShenandoahRootVerifier::combine(types, ShenandoahRootVerifier::CLDGRoots); + types = ShenandoahRootVerifier::combine(types, ShenandoahRootVerifier::StringDedupRoots); } if (ShenandoahConcurrentRoots::should_do_concurrent_class_unloading()) { @@ -1653,6 +1653,7 @@ ShenandoahVMRoots _vm_roots; ShenandoahWeakRoots _weak_roots; ShenandoahClassLoaderDataRoots _cld_roots; + ShenandoahConcurrentStringDedupRoots _dedup_roots; bool _include_weak_roots; public: @@ -1675,10 +1676,16 @@ } { - ShenandoahEvacuateUpdateRootsClosure cl; + ShenandoahEvacuateUpdateRootsClosure<> cl; CLDToOopClosure clds(&cl, ClassLoaderData::_claim_strong); _cld_roots.cld_do(&clds); } + + { + ShenandoahForwardedIsAliveClosure is_alive; + ShenandoahEvacuateUpdateRootsClosure keep_alive; + _dedup_roots.oops_do(&is_alive, &keep_alive, worker_id); + } } }; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahNMethod.cpp b/src/hotspot/share/gc/shenandoah/shenandoahNMethod.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahNMethod.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahNMethod.cpp @@ -177,7 +177,7 @@ assert(data->lock()->owned_by_self(), "Must hold the lock"); ShenandoahEvacOOMScope evac_scope; - ShenandoahEvacuateUpdateRootsClosure cl; + ShenandoahEvacuateUpdateRootsClosure<> cl; data->oops_do(&cl, true /*fix relocation*/); } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp @@ -150,6 +150,33 @@ } } +ShenandoahConcurrentStringDedupRoots::ShenandoahConcurrentStringDedupRoots() { + if (ShenandoahStringDedup::is_enabled()) { + StringDedup::gc_prologue(true); + StringDedupTable_lock->lock_without_safepoint_check(); + StringDedupQueue_lock->lock_without_safepoint_check(); + } +} + +ShenandoahConcurrentStringDedupRoots::~ShenandoahConcurrentStringDedupRoots() { + if (ShenandoahStringDedup::is_enabled()) { + StringDedup::gc_epilogue(); + StringDedupQueue_lock->unlock(); + StringDedupTable_lock->unlock(); + } +} + +void ShenandoahConcurrentStringDedupRoots::oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, uint worker_id) { + if (ShenandoahStringDedup::is_enabled()) { + assert_locked_or_safepoint_weak(StringDedupQueue_lock); + assert_locked_or_safepoint_weak(StringDedupTable_lock); + + StringDedupUnlinkOrOopsDoClosure sd_cl(is_alive, keep_alive); + StringDedupQueue::unlink_or_oops_do(&sd_cl); + StringDedupTable::unlink_or_oops_do(&sd_cl, worker_id); + } +} + ShenandoahRootProcessor::ShenandoahRootProcessor(ShenandoahPhaseTimings::Phase phase) : _heap(ShenandoahHeap::heap()), _phase(phase) { @@ -187,6 +214,7 @@ _vm_roots.oops_do(oops, worker_id); _cld_roots.cld_do(&clds, worker_id); _weak_roots.oops_do(oops, worker_id); + _dedup_roots.oops_do(&always_true, oops, worker_id); } if (_include_concurrent_code_roots) { @@ -195,8 +223,6 @@ } else { _thread_roots.oops_do(oops, codes_cl, worker_id); } - - _dedup_roots.oops_do(&always_true, oops, worker_id); } ShenandoahRootUpdater::ShenandoahRootUpdater(uint n_workers, ShenandoahPhaseTimings::Phase phase) : diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp @@ -188,6 +188,14 @@ void oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, uint worker_id); }; +class ShenandoahConcurrentStringDedupRoots { +public: + ShenandoahConcurrentStringDedupRoots(); + ~ShenandoahConcurrentStringDedupRoots(); + + void oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, uint worker_id); +}; + template class ShenandoahCodeCacheRoots { private: diff --git a/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.hpp b/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.hpp @@ -32,8 +32,8 @@ template class ShenandoahOopBuffer : public CHeapObj { private: - oop _buf[buffer_size]; - uint _index; + oop _buf[buffer_size]; + volatile uint _index; ShenandoahOopBuffer* _next; public: @@ -53,6 +53,10 @@ void unlink_or_oops_do(StringDedupUnlinkOrOopsDoClosure* cl); void oops_do(OopClosure* cl); + +private: + uint index_acquire() const; + void set_index_release(uint index); }; typedef ShenandoahOopBuffer<64> ShenandoahQueueBuffer; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.inline.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahStrDedupQueue.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2019, Red Hat, Inc. All rights reserved. + * Copyright (c) 2017, 2020, Red Hat, Inc. 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 @@ -26,7 +26,17 @@ #define SHARE_GC_SHENANDOAH_SHENANDOAHSTRDEDUPQUEUE_INLINE_HPP #include "gc/shenandoah/shenandoahStrDedupQueue.hpp" +#include "oops/access.hpp" +#include "runtime/atomic.hpp" +// With concurrent string dedup cleaning up, GC worker threads +// may see oops just enqueued, so release_store and load_acquire +// relationship needs to be established between enqueuing threads +// and GC workers. +// For example, when GC sees a slot (index), there must be a valid +// (dead or live) oop. +// Note: There is no concern if GC misses newly enqueued oops, +// since LRB ensures they are in to-space. template ShenandoahOopBuffer::ShenandoahOopBuffer() : _index(0), _next(NULL) { @@ -34,29 +44,34 @@ template bool ShenandoahOopBuffer::is_full() const { - return _index >= buffer_size; + return index_acquire() >= buffer_size; } template bool ShenandoahOopBuffer::is_empty() const { - return _index == 0; + return index_acquire() == 0; } template uint ShenandoahOopBuffer::size() const { - return _index; + return index_acquire(); } template void ShenandoahOopBuffer::push(oop obj) { assert(!is_full(), "Buffer is full"); - _buf[_index ++] = obj; + uint idx = index_acquire(); + RawAccess::oop_store(&_buf[idx], obj); + set_index_release(idx + 1); } template oop ShenandoahOopBuffer::pop() { assert(!is_empty(), "Buffer is empty"); - return _buf[--_index]; + uint idx = index_acquire() - 1; + oop value = NativeAccess::oop_load(&_buf[idx]); + set_index_release(idx); + return value; } template @@ -76,14 +91,25 @@ } template +uint ShenandoahOopBuffer::index_acquire() const { + return Atomic::load_acquire(&_index); +} + +template +void ShenandoahOopBuffer::set_index_release(uint index) { + return Atomic::release_store(&_index, index); +} + +template void ShenandoahOopBuffer::unlink_or_oops_do(StringDedupUnlinkOrOopsDoClosure* cl) { - for (uint index = 0; index < size(); index ++) { + uint len = size(); + for (uint index = 0; index < len; index ++) { oop* obj_addr = &_buf[index]; if (*obj_addr != NULL) { if (cl->is_alive(*obj_addr)) { cl->keep_alive(obj_addr); } else { - *obj_addr = NULL; + RawAccess::oop_store(&_buf[index], oop()); } } } @@ -91,7 +117,8 @@ template void ShenandoahOopBuffer::oops_do(OopClosure* cl) { - for (uint index = 0; index < size(); index ++) { + uint len = size(); + for (uint index = 0; index < len; index ++) { cl->do_oop(&_buf[index]); } } diff --git a/src/hotspot/share/runtime/mutexLocker.cpp b/src/hotspot/share/runtime/mutexLocker.cpp --- a/src/hotspot/share/runtime/mutexLocker.cpp +++ b/src/hotspot/share/runtime/mutexLocker.cpp @@ -228,7 +228,7 @@ } if (UseShenandoahGC) { def(StringDedupQueue_lock , PaddedMonitor, leaf, true, _safepoint_check_never); - def(StringDedupTable_lock , PaddedMutex , leaf, true, _safepoint_check_never); + def(StringDedupTable_lock , PaddedMutex , leaf + 1, true, _safepoint_check_never); } def(ParGCRareEvent_lock , PaddedMutex , leaf , true, _safepoint_check_always); def(CGCPhaseManager_lock , PaddedMonitor, leaf, false, _safepoint_check_always);