From 7f8ec3942373d156f6c59608e7c76f43cc3475e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20=C3=96sterlund?= Date: Thu, 9 Jan 2025 10:10:53 +0000 Subject: [PATCH 1/4] 8347335: ZGC: Use limitless mark stack memory --- src/hotspot/share/gc/z/zAddressSpaceLimit.cpp | 6 - src/hotspot/share/gc/z/zAddressSpaceLimit.hpp | 1 - src/hotspot/share/gc/z/zArguments.cpp | 9 - src/hotspot/share/gc/z/zBarrierSet.cpp | 2 +- src/hotspot/share/gc/z/zGeneration.cpp | 8 +- src/hotspot/share/gc/z/zGeneration.hpp | 4 +- src/hotspot/share/gc/z/zGlobals.hpp | 11 - src/hotspot/share/gc/z/zHeap.cpp | 8 +- src/hotspot/share/gc/z/zHeap.hpp | 2 +- src/hotspot/share/gc/z/zInitialize.cpp | 1 - src/hotspot/share/gc/z/zMark.cpp | 68 ++-- src/hotspot/share/gc/z/zMark.hpp | 31 +- src/hotspot/share/gc/z/zMark.inline.hpp | 2 +- src/hotspot/share/gc/z/zMarkStack.cpp | 331 +++++++++++------- src/hotspot/share/gc/z/zMarkStack.hpp | 107 +++--- src/hotspot/share/gc/z/zMarkStack.inline.hpp | 208 ++++------- .../share/gc/z/zMarkStackAllocator.cpp | 236 ------------- .../share/gc/z/zMarkTerminate.inline.hpp | 3 +- src/hotspot/share/gc/z/zMarkingSMR.cpp | 112 ++++++ ...MarkStackAllocator.hpp => zMarkingSMR.hpp} | 65 +--- src/hotspot/share/gc/z/zRemembered.cpp | 2 +- src/hotspot/share/gc/z/zStat.cpp | 9 +- src/hotspot/share/gc/z/zStat.hpp | 1 - src/hotspot/share/gc/z/z_globals.hpp | 4 - src/hotspot/share/runtime/arguments.cpp | 1 + 25 files changed, 497 insertions(+), 735 deletions(-) delete mode 100644 src/hotspot/share/gc/z/zMarkStackAllocator.cpp create mode 100644 src/hotspot/share/gc/z/zMarkingSMR.cpp rename src/hotspot/share/gc/z/{zMarkStackAllocator.hpp => zMarkingSMR.hpp} (50%) diff --git a/src/hotspot/share/gc/z/zAddressSpaceLimit.cpp b/src/hotspot/share/gc/z/zAddressSpaceLimit.cpp index c737644472c6e..fc42d9f3db1fa 100644 --- a/src/hotspot/share/gc/z/zAddressSpaceLimit.cpp +++ b/src/hotspot/share/gc/z/zAddressSpaceLimit.cpp @@ -39,12 +39,6 @@ static size_t address_space_limit() { return SIZE_MAX; } -size_t ZAddressSpaceLimit::mark_stack() { - // Allow mark stacks to occupy 10% of the address space - const size_t limit = address_space_limit() / 10; - return align_up(limit, ZMarkStackSpaceExpandSize); -} - size_t ZAddressSpaceLimit::heap() { // Allow the heap to occupy 50% of the address space const size_t limit = address_space_limit() / MaxVirtMemFraction; diff --git a/src/hotspot/share/gc/z/zAddressSpaceLimit.hpp b/src/hotspot/share/gc/z/zAddressSpaceLimit.hpp index d8e7e7cfd3617..66e01f0ebb08c 100644 --- a/src/hotspot/share/gc/z/zAddressSpaceLimit.hpp +++ b/src/hotspot/share/gc/z/zAddressSpaceLimit.hpp @@ -29,7 +29,6 @@ class ZAddressSpaceLimit : public AllStatic { public: - static size_t mark_stack(); static size_t heap(); }; diff --git a/src/hotspot/share/gc/z/zArguments.cpp b/src/hotspot/share/gc/z/zArguments.cpp index 7c082252133cd..ee761d03e1848 100644 --- a/src/hotspot/share/gc/z/zArguments.cpp +++ b/src/hotspot/share/gc/z/zArguments.cpp @@ -120,15 +120,6 @@ void ZArguments::select_max_gc_threads() { void ZArguments::initialize() { GCArguments::initialize(); - // Check mark stack size - const size_t mark_stack_space_limit = ZAddressSpaceLimit::mark_stack(); - if (ZMarkStackSpaceLimit > mark_stack_space_limit) { - if (!FLAG_IS_DEFAULT(ZMarkStackSpaceLimit)) { - vm_exit_during_initialization("ZMarkStackSpaceLimit too large for limited address space"); - } - FLAG_SET_DEFAULT(ZMarkStackSpaceLimit, mark_stack_space_limit); - } - // Enable NUMA by default if (FLAG_IS_DEFAULT(UseNUMA)) { FLAG_SET_DEFAULT(UseNUMA, true); diff --git a/src/hotspot/share/gc/z/zBarrierSet.cpp b/src/hotspot/share/gc/z/zBarrierSet.cpp index f88a593b1c307..a4893fc351aa8 100644 --- a/src/hotspot/share/gc/z/zBarrierSet.cpp +++ b/src/hotspot/share/gc/z/zBarrierSet.cpp @@ -101,7 +101,7 @@ void ZBarrierSet::on_thread_attach(Thread* thread) { void ZBarrierSet::on_thread_detach(Thread* thread) { // Flush and free any remaining mark stacks - ZHeap::heap()->mark_flush_and_free(thread); + ZHeap::heap()->mark_flush(thread); } static void deoptimize_allocation(JavaThread* thread) { diff --git a/src/hotspot/share/gc/z/zGeneration.cpp b/src/hotspot/share/gc/z/zGeneration.cpp index 9a2535d527f6f..0dc8e93b8e716 100644 --- a/src/hotspot/share/gc/z/zGeneration.cpp +++ b/src/hotspot/share/gc/z/zGeneration.cpp @@ -131,10 +131,6 @@ ZGeneration::ZGeneration(ZGenerationId id, ZPageTable* page_table, ZPageAllocato _stat_relocation(), _gc_timer(nullptr) {} -bool ZGeneration::is_initialized() const { - return _mark.is_initialized(); -} - ZWorkers* ZGeneration::workers() { return &_workers; } @@ -151,8 +147,8 @@ void ZGeneration::threads_do(ThreadClosure* tc) const { _workers.threads_do(tc); } -void ZGeneration::mark_flush_and_free(Thread* thread) { - _mark.flush_and_free(thread); +void ZGeneration::mark_flush(Thread* thread) { + _mark.flush(thread); } void ZGeneration::mark_free() { diff --git a/src/hotspot/share/gc/z/zGeneration.hpp b/src/hotspot/share/gc/z/zGeneration.hpp index 3e18919eee43a..2670ab5169909 100644 --- a/src/hotspot/share/gc/z/zGeneration.hpp +++ b/src/hotspot/share/gc/z/zGeneration.hpp @@ -99,8 +99,6 @@ class ZGeneration { void log_phase_switch(Phase from, Phase to); public: - bool is_initialized() const; - // GC phases void set_phase(Phase new_phase); bool is_phase_relocate() const; @@ -161,7 +159,7 @@ class ZGeneration { void mark_object(zaddress addr); template void mark_object_if_active(zaddress addr); - void mark_flush_and_free(Thread* thread); + void mark_flush(Thread* thread); // Relocation void synchronize_relocation(); diff --git a/src/hotspot/share/gc/z/zGlobals.hpp b/src/hotspot/share/gc/z/zGlobals.hpp index f7c0a3eaa2698..918301b18532c 100644 --- a/src/hotspot/share/gc/z/zGlobals.hpp +++ b/src/hotspot/share/gc/z/zGlobals.hpp @@ -67,17 +67,6 @@ const int ZObjectAlignmentLarge = 1 << ZObjectAlignmentLargeShif const size_t ZCacheLineSize = ZPlatformCacheLineSize; #define ZCACHE_ALIGNED ATTRIBUTE_ALIGNED(ZCacheLineSize) -// Mark stack space -const size_t ZMarkStackSpaceExpandSize = (size_t)1 << 25; // 32M - -// Mark stack and magazine sizes -const size_t ZMarkStackSizeShift = 11; // 2K -const size_t ZMarkStackSize = (size_t)1 << ZMarkStackSizeShift; -const size_t ZMarkStackHeaderSize = (size_t)1 << 4; // 16B -const size_t ZMarkStackSlots = (ZMarkStackSize - ZMarkStackHeaderSize) / sizeof(uintptr_t); -const size_t ZMarkStackMagazineSize = (size_t)1 << 15; // 32K -const size_t ZMarkStackMagazineSlots = (ZMarkStackMagazineSize / ZMarkStackSize) - 1; - // Mark stripe size const size_t ZMarkStripeShift = ZGranuleSizeShift; diff --git a/src/hotspot/share/gc/z/zHeap.cpp b/src/hotspot/share/gc/z/zHeap.cpp index 7169b915cb5ee..bca9e2f8c4156 100644 --- a/src/hotspot/share/gc/z/zHeap.cpp +++ b/src/hotspot/share/gc/z/zHeap.cpp @@ -68,7 +68,7 @@ ZHeap::ZHeap() assert(_heap == nullptr, "Already initialized"); _heap = this; - if (!_page_allocator.is_initialized() || !_young.is_initialized() || !_old.is_initialized()) { + if (!_page_allocator.is_initialized()) { return; } @@ -271,9 +271,9 @@ void ZHeap::keep_alive(oop obj) { ZBarrier::mark(addr); } -void ZHeap::mark_flush_and_free(Thread* thread) { - _young.mark_flush_and_free(thread); - _old.mark_flush_and_free(thread); +void ZHeap::mark_flush(Thread* thread) { + _young.mark_flush(thread); + _old.mark_flush(thread); } bool ZHeap::is_allocating(zaddress addr) const { diff --git a/src/hotspot/share/gc/z/zHeap.hpp b/src/hotspot/share/gc/z/zHeap.hpp index 955cdf6fd79e4..4bf4727c3e35c 100644 --- a/src/hotspot/share/gc/z/zHeap.hpp +++ b/src/hotspot/share/gc/z/zHeap.hpp @@ -99,7 +99,7 @@ class ZHeap { bool is_object_live(zaddress addr) const; bool is_object_strongly_live(zaddress addr) const; void keep_alive(oop obj); - void mark_flush_and_free(Thread* thread); + void mark_flush(Thread* thread); // Page allocation ZPage* alloc_page(ZPageType type, size_t size, ZAllocationFlags flags, ZPageAge age); diff --git a/src/hotspot/share/gc/z/zInitialize.cpp b/src/hotspot/share/gc/z/zInitialize.cpp index a8d6a32a7d5ee..125231355ac2e 100644 --- a/src/hotspot/share/gc/z/zInitialize.cpp +++ b/src/hotspot/share/gc/z/zInitialize.cpp @@ -31,7 +31,6 @@ #include "gc/z/zInitialize.hpp" #include "gc/z/zJNICritical.hpp" #include "gc/z/zLargePages.hpp" -#include "gc/z/zMarkStackAllocator.hpp" #include "gc/z/zNMT.hpp" #include "gc/z/zNUMA.hpp" #include "gc/z/zStat.hpp" diff --git a/src/hotspot/share/gc/z/zMark.cpp b/src/hotspot/share/gc/z/zMark.cpp index 554cef8cec5fe..89ed585b5c2ab 100644 --- a/src/hotspot/share/gc/z/zMark.cpp +++ b/src/hotspot/share/gc/z/zMark.cpp @@ -81,8 +81,8 @@ static const ZStatSubPhase ZSubPhaseConcurrentMarkRootColoredOld("Concurrent Mar ZMark::ZMark(ZGeneration* generation, ZPageTable* page_table) : _generation(generation), _page_table(page_table), - _allocator(), - _stripes(_allocator.start()), + _marking_smr(), + _stripes(), _terminate(), _work_nproactiveflush(0), _work_nterminateflush(0), @@ -92,10 +92,6 @@ ZMark::ZMark(ZGeneration* generation, ZPageTable* page_table) _ncontinue(0), _nworkers(0) {} -bool ZMark::is_initialized() const { - return _allocator.is_initialized(); -} - size_t ZMark::calculate_nstripes(uint nworkers) const { // Calculate the number of stripes from the number of workers we use, // where the number of stripes must be a power of two and we want to @@ -196,7 +192,7 @@ void ZMark::push_partial_array(zpointer* addr, size_t length, bool finalizable) log_develop_trace(gc, marking)("Array push partial: " PTR_FORMAT " (%zu), stripe: %zu", p2i(addr), length, _stripes.stripe_id(stripe)); - stacks->push(&_allocator, &_stripes, stripe, &_terminate, entry, false /* publish */); + stacks->push(&_stripes, stripe, &_terminate, entry, false /* publish */); } static void mark_barrier_on_oop_array(volatile zpointer* p, size_t length, bool finalizable, bool young) { @@ -471,21 +467,26 @@ bool ZMark::rebalance_work(ZMarkContext* context) { const size_t nstripes = _stripes.nstripes(); if (assumed_nstripes != nstripes) { + // The number of stripes has changed; reflect that change locally context->set_nstripes(nstripes); - } else if (nstripes < calculate_nstripes(_nworkers) && _allocator.clear_and_get_expanded_recently()) { - const size_t new_nstripes = nstripes << 1; - _stripes.set_nstripes(new_nstripes); - context->set_nstripes(new_nstripes); + } else if (nstripes < calculate_nstripes(_nworkers) && _stripes.is_crowded()) { + // We are running on a reduced number of threads to minimize the amount of work + // hidden in local stacks when the stripes are less well balanced. When this situation + // starts getting crowded, we bump the number of stripes again. + size_t new_nstripes = nstripes << 1; + if (_stripes.try_set_nstripes(nstripes, new_nstripes)) { + context->set_nstripes(new_nstripes); + } } ZMarkStripe* stripe = _stripes.stripe_for_worker(_nworkers, WorkerThread::worker_id()); if (context->stripe() != stripe) { // Need to switch stripe context->set_stripe(stripe); - flush_and_free(); + flush(Thread::current()); } else if (!_terminate.saturated()) { // Work imbalance detected; striped marking is likely going to be in the way - flush_and_free(); + flush(Thread::current()); } SuspendibleThreadSet::yield(); @@ -502,7 +503,7 @@ bool ZMark::drain(ZMarkContext* context) { context->set_nstripes(_stripes.nstripes()); // Drain stripe stacks - while (stacks->pop(&_allocator, &_stripes, context->stripe(), entry)) { + while (stacks->pop(&_marking_smr, &_stripes, context->stripe(), &entry)) { mark_and_follow(context, entry); if ((processed++ & 31) == 0 && rebalance_work(context)) { @@ -541,7 +542,7 @@ bool ZMark::try_steal_global(ZMarkContext* context) { for (ZMarkStripe* victim_stripe = _stripes.stripe_next(stripe); victim_stripe != stripe; victim_stripe = _stripes.stripe_next(victim_stripe)) { - ZMarkStack* const stack = victim_stripe->steal_stack(); + ZMarkStack* const stack = victim_stripe->steal_stack(&_marking_smr); if (stack != nullptr) { // Success, install the stolen stack stacks->install(&_stripes, stripe, stack); @@ -557,19 +558,19 @@ bool ZMark::try_steal(ZMarkContext* context) { return try_steal_local(context) || try_steal_global(context); } -class ZMarkFlushAndFreeStacksClosure : public HandshakeClosure { +class ZMarkFlushStacksClosure : public HandshakeClosure { private: ZMark* const _mark; bool _flushed; public: - ZMarkFlushAndFreeStacksClosure(ZMark* mark) - : HandshakeClosure("ZMarkFlushAndFreeStacks"), + ZMarkFlushStacksClosure(ZMark* mark) + : HandshakeClosure("ZMarkFlushStacks"), _mark(mark), _flushed(false) {} void do_thread(Thread* thread) { - if (_mark->flush_and_free(thread)) { + if (_mark->flush(thread)) { _flushed = true; if (SafepointSynchronize::is_at_safepoint()) { log_debug(gc, marking)("Thread broke mark termination %s", thread->name()); @@ -606,7 +607,7 @@ class VM_ZMarkFlushOperation : public VM_Operation { }; bool ZMark::flush() { - ZMarkFlushAndFreeStacksClosure cl(this); + ZMarkFlushStacksClosure cl(this); VM_ZMarkFlushOperation vm_cl(&cl); Handshake::execute(&cl); VMThread::execute(&vm_cl); @@ -623,8 +624,7 @@ bool ZMark::try_terminate_flush() { verify_worker_stacks_empty(); } - return flush() || - _terminate.resurrected(); + return flush() || _terminate.resurrected(); } bool ZMark::try_proactive_flush() { @@ -850,7 +850,7 @@ class ZMarkOldRootsTask : public ZTask { // the set of workers executing during root scanning // can be different from the set of workers executing // during mark. - ZHeap::heap()->mark_flush_and_free(Thread::current()); + ZHeap::heap()->mark_flush(Thread::current()); } }; @@ -908,7 +908,7 @@ class ZMarkYoungRootsTask : public ZTask { // the set of workers executing during root scanning // can be different from the set of workers executing // during mark. - ZHeap::heap()->mark_flush_and_free(Thread::current()); + ZHeap::heap()->mark_flush(Thread::current()); } }; @@ -934,7 +934,7 @@ class ZMarkTask : public ZRestartableTask { // publish such marking stacks to prevent that generation from getting a mark continue. // We also flush in case of a resize where a new worker thread continues the marking // work, causing a mark continue for the collected generation. - ZHeap::heap()->mark_flush_and_free(Thread::current()); + ZHeap::heap()->mark_flush(Thread::current()); } virtual void resize_workers(uint nworkers) { @@ -978,7 +978,7 @@ bool ZMark::try_end() { } // Try end marking - ZMarkFlushAndFreeStacksClosure cl(this); + ZMarkFlushStacksClosure cl(this); Threads::non_java_threads_do(&cl); // Check if non-java threads have any pending marking @@ -1012,25 +1012,15 @@ bool ZMark::end() { void ZMark::free() { // Free any unused mark stack space - _allocator.free(); - - // Update statistics - _generation->stat_mark()->at_mark_free(_allocator.size()); -} - -void ZMark::flush_and_free() { - Thread* const thread = Thread::current(); - flush_and_free(thread); + _marking_smr.free(); } -bool ZMark::flush_and_free(Thread* thread) { +bool ZMark::flush(Thread* thread) { if (thread->is_Java_thread()) { ZThreadLocalData::store_barrier_buffer(thread)->flush(); } ZMarkThreadLocalStacks* const stacks = ZThreadLocalData::mark_stacks(thread, _generation->id()); - const bool flushed = stacks->flush(&_allocator, &_stripes, &_terminate); - stacks->free(&_allocator); - return flushed; + return stacks->flush(&_stripes, &_terminate); } class ZVerifyMarkStacksEmptyClosure : public ThreadClosure { diff --git a/src/hotspot/share/gc/z/zMark.hpp b/src/hotspot/share/gc/z/zMark.hpp index 552bf3b959ddd..07e73849af6cf 100644 --- a/src/hotspot/share/gc/z/zMark.hpp +++ b/src/hotspot/share/gc/z/zMark.hpp @@ -25,8 +25,8 @@ #define SHARE_GC_Z_ZMARK_HPP #include "gc/z/zAddress.hpp" +#include "gc/z/zMarkingSMR.hpp" #include "gc/z/zMarkStack.hpp" -#include "gc/z/zMarkStackAllocator.hpp" #include "gc/z/zMarkStackEntry.hpp" #include "gc/z/zMarkTerminate.hpp" #include "oops/oopsHierarchy.hpp" @@ -55,18 +55,18 @@ class ZMark { static const bool Finalizable = true; private: - ZGeneration* const _generation; - ZPageTable* const _page_table; - ZMarkStackAllocator _allocator; - ZMarkStripeSet _stripes; - ZMarkTerminate _terminate; - volatile size_t _work_nproactiveflush; - volatile size_t _work_nterminateflush; - size_t _nproactiveflush; - size_t _nterminateflush; - size_t _ntrycomplete; - size_t _ncontinue; - uint _nworkers; + ZGeneration* const _generation; + ZPageTable* const _page_table; + ZMarkingSMR _marking_smr; + ZMarkStripeSet _stripes; + ZMarkTerminate _terminate; + volatile size_t _work_nproactiveflush; + volatile size_t _work_nterminateflush; + size_t _nproactiveflush; + size_t _nterminateflush; + size_t _ntrycomplete; + size_t _ncontinue; + uint _nworkers; size_t calculate_nstripes(uint nworkers) const; @@ -101,8 +101,6 @@ class ZMark { public: ZMark(ZGeneration* generation, ZPageTable* page_table); - bool is_initialized() const; - template void mark_object(zaddress addr); @@ -113,8 +111,7 @@ class ZMark { bool end(); void free(); - void flush_and_free(); - bool flush_and_free(Thread* thread); + bool flush(Thread* thread); // Following work void prepare_work(); diff --git a/src/hotspot/share/gc/z/zMark.inline.hpp b/src/hotspot/share/gc/z/zMark.inline.hpp index 9edc57a60002e..26c84679cc074 100644 --- a/src/hotspot/share/gc/z/zMark.inline.hpp +++ b/src/hotspot/share/gc/z/zMark.inline.hpp @@ -84,7 +84,7 @@ inline void ZMark::mark_object(zaddress addr) { assert(ZHeap::heap()->is_young(addr) == _generation->is_young(), "Phase/object mismatch"); const bool publish = !gc_thread; - stacks->push(&_allocator, &_stripes, stripe, &_terminate, entry, publish); + stacks->push(&_stripes, stripe, &_terminate, entry, publish); } #endif // SHARE_GC_Z_ZMARK_INLINE_HPP diff --git a/src/hotspot/share/gc/z/zMarkStack.cpp b/src/hotspot/share/gc/z/zMarkStack.cpp index 00a534d236ff2..bc5b6f44db958 100644 --- a/src/hotspot/share/gc/z/zMarkStack.cpp +++ b/src/hotspot/share/gc/z/zMarkStack.cpp @@ -21,39 +21,213 @@ * questions. */ +#include "gc/z/zMarkingSMR.hpp" #include "gc/z/zMarkStack.inline.hpp" -#include "gc/z/zMarkStackAllocator.hpp" #include "gc/z/zMarkTerminate.inline.hpp" #include "logging/log.hpp" #include "runtime/atomic.hpp" +#include "runtime/orderAccess.hpp" #include "utilities/debug.hpp" #include "utilities/powerOfTwo.hpp" -ZMarkStripe::ZMarkStripe(uintptr_t base) - : _published(base), - _overflowed(base) {} +ZMarkStack* ZMarkStack::create(bool first_stack) { + // When allocating the first stack on a stripe, we try to use a + // smaller mark stack to promote sharing of stacks with other + // threads instead. Once more than one stack is needed, we revert + // to a larger stack size instead, which reduces synchronization + // overhead of churning around stacks on a stripe. + size_t capacity = first_stack ? 128 : 512; + + size_t size = sizeof(ZMarkStack) + capacity * sizeof(ZMarkStackEntry); + char* memory = NEW_C_HEAP_ARRAY(char, size, mtGC); + return new (memory) ZMarkStack(capacity); +} -ZMarkStripeSet::ZMarkStripeSet(uintptr_t base) - : _nstripes_mask(0), - _stripes() { +void ZMarkStack::destroy(ZMarkStack* stack) { + char* memory = (char*)stack; + FREE_C_HEAP_ARRAY(char, memory); +} + +ZMarkStack::ZMarkStack(size_t capacity) + : _top(0), + _capacity(capacity) {} + +ZMarkStackListNode::ZMarkStackListNode(ZMarkStack* stack) + : _stack(stack), + _next() {} + +ZMarkStack* ZMarkStackListNode::stack() const { + return _stack; +} + +ZMarkStackListNode* ZMarkStackListNode::next() const { + return _next; +} + +void ZMarkStackListNode::set_next(ZMarkStackListNode* next) { + _next = next; +} + +ZMarkStackList::ZMarkStackList() + : _head(), + _length() {} + +bool ZMarkStackList::is_empty() const { + return Atomic::load(&_head) == nullptr; +} + +void ZMarkStackList::push(ZMarkStack* stack) { + ZMarkStackListNode* node = new ZMarkStackListNode(stack); + ZMarkStackListNode* head = Atomic::load(&_head); + for (;;) { + node->set_next(head); + // Between reading the head and the linearizing CAS that pushes + // the node onto the list, there could be an ABA problem. Except, + // on the pushing sidee, that is benign. The node is never + // dereferenced while pushing and if we were to detect the ABA + // situation and run this loop one more time, we would end up + // having the same side effects: set the next pointer to the same + // head again, and CAS the head link. + ZMarkStackListNode* prev = Atomic::cmpxchg(&_head, head, node, memory_order_release); + + if (prev == head) { + // Success + + // Bookkeep the population count + Atomic::inc(&_length, memory_order_relaxed); + return; + } + + // Retry + head = prev; + } +} + +ZMarkStack* ZMarkStackList::pop(ZMarkingSMR* marking_smr) { + ZMarkStackListNode* volatile* hazard_ptr = marking_smr->hazard_ptr(); + + ZMarkStackListNode* head = Atomic::load(&_head); + for (;;) { + if (head == nullptr) { + // Stack is empty + return nullptr; + } + + // Establish what the head is and publish a hazard pointer denoting + // that the head is not safe to concurrently free while we are in the + // middle of popping it and finding out that we lost the race. + Atomic::store(hazard_ptr, head); + + // A full fence is needed to ensure the store and subsequent load do + // not reorder. If they did reorder, the second head load could happen + // before other threads scanning hazard poitners can observe it, meaning + // it could get concurrently freed. + OrderAccess::fence(); + + // The acquire fence when loading the head is necessary to make sure + // the next pointer load below observes the next pointer published + // with the releasing CAS for the push operation that published the + // marking stack. + ZMarkStackListNode* const head_after_publish = Atomic::load_acquire(&_head); + if (head_after_publish != head) { + // Race during hazard pointer publishing + head = head_after_publish; + continue; + } + + // With the hazard pointer published, we can read the next pointer, + // knowing that it is indeed the next pointer of the intended logical + // head node that we established above. + ZMarkStackListNode* const next = head->next(); + + // Popping entries from the list does not require any particular memory + // ordering. + ZMarkStackListNode* const prev = Atomic::cmpxchg(&_head, head, next, memory_order_relaxed); + + if (prev == head) { + // Success + + // The ABA hazard is gone after the CAS. We use release_store to ensure + // that the relinquishing of the hazard pointer becomes observable after + // the unlinking CAS. + Atomic::release_store(hazard_ptr, (ZMarkStackListNode*)nullptr); + + // Perform bookkeeping of the population count. + Atomic::dec(&_length, memory_order_relaxed); + + ZMarkStack* result = head->stack(); + + marking_smr->free_node(head); + + return result; + } + + // Retry + head = prev; + } +} + +size_t ZMarkStackList::length() const { + ssize_t result = Atomic::load(&_length); - // Re-construct array elements with the correct base - for (size_t i = 0; i < ARRAY_SIZE(_stripes); i++) { - _stripes[i] = ZMarkStripe(base); + if (result < 0) { + return 0; } + + return (size_t)result; +} + +ZMarkStripe::ZMarkStripe() + : _published(), + _overflowed() {} + +ZMarkStack* ZMarkStripe::steal_stack(ZMarkingSMR* marking_smr) { + // Steal overflowed stacks first, then published stacks + ZMarkStack* const stack = _overflowed.pop(marking_smr); + if (stack != nullptr) { + return stack; + } + + return _published.pop(marking_smr); +} + +size_t ZMarkStripe::population() const { + return _overflowed.length() + _published.length(); } +ZMarkStripeSet::ZMarkStripeSet() + : _nstripes_mask(0), + _stripes() {} + void ZMarkStripeSet::set_nstripes(size_t nstripes) { assert(is_power_of_2(nstripes), "Must be a power of two"); assert(is_power_of_2(ZMarkStripesMax), "Must be a power of two"); assert(nstripes >= 1, "Invalid number of stripes"); assert(nstripes <= ZMarkStripesMax, "Invalid number of stripes"); + size_t new_nstripes_mask = nstripes - 1; + _nstripes_mask = new_nstripes_mask; + + log_debug(gc, marking)("Using %zu mark stripes", nstripes); +} + +bool ZMarkStripeSet::try_set_nstripes(size_t old_nstripes, size_t new_nstripes) { + assert(is_power_of_2(new_nstripes), "Must be a power of two"); + assert(is_power_of_2(ZMarkStripesMax), "Must be a power of two"); + assert(new_nstripes >= 1, "Invalid number of stripes"); + assert(new_nstripes <= ZMarkStripesMax, "Invalid number of stripes"); + + size_t old_nstripes_mask = old_nstripes - 1; + size_t new_nstripes_mask = new_nstripes - 1; + // Mutators may read these values concurrently. It doesn't matter // if they see the old or new values. - Atomic::store(&_nstripes_mask, nstripes - 1); + if (Atomic::cmpxchg(&_nstripes_mask, old_nstripes_mask, new_nstripes_mask) == old_nstripes_mask) { + log_debug(gc, marking)("Using %zu mark stripes", new_nstripes); + return true; + } - log_debug(gc, marking)("Using %zu mark stripes", nstripes); + return false; } size_t ZMarkStripeSet::nstripes() const { @@ -70,6 +244,20 @@ bool ZMarkStripeSet::is_empty() const { return true; } +bool ZMarkStripeSet::is_crowded() const { + size_t population = 0; + const size_t crowded_threshold = nstripes() << 4; + + for (size_t i = 0; i < ZMarkStripesMax; i++) { + population += _stripes[i].population(); + if (population > crowded_threshold) { + return true; + } + } + + return false; +} + ZMarkStripe* ZMarkStripeSet::stripe_for_worker(uint nworkers, uint worker_id) { const size_t mask = Atomic::load(&_nstripes_mask); const size_t nstripes = mask + 1; @@ -92,8 +280,7 @@ ZMarkStripe* ZMarkStripeSet::stripe_for_worker(uint nworkers, uint worker_id) { return &_stripes[index]; } -ZMarkThreadLocalStacks::ZMarkThreadLocalStacks() - : _magazine(nullptr) { +ZMarkThreadLocalStacks::ZMarkThreadLocalStacks() { for (size_t i = 0; i < ZMarkStripesMax; i++) { _stacks[i] = nullptr; } @@ -110,104 +297,8 @@ bool ZMarkThreadLocalStacks::is_empty(const ZMarkStripeSet* stripes) const { return true; } -ZMarkStack* ZMarkThreadLocalStacks::allocate_stack(ZMarkStackAllocator* allocator) { - if (_magazine == nullptr) { - // Allocate new magazine - _magazine = allocator->alloc_magazine(); - if (_magazine == nullptr) { - return nullptr; - } - } - - ZMarkStack* stack = nullptr; - - if (!_magazine->pop(stack)) { - // Magazine is empty, convert magazine into a new stack - _magazine->~ZMarkStackMagazine(); - stack = new ((void*)_magazine) ZMarkStack(); - _magazine = nullptr; - } - - return stack; -} - -void ZMarkThreadLocalStacks::free_stack(ZMarkStackAllocator* allocator, ZMarkStack* stack) { - for (;;) { - if (_magazine == nullptr) { - // Convert stack into a new magazine - stack->~ZMarkStack(); - _magazine = new ((void*)stack) ZMarkStackMagazine(); - return; - } - - if (_magazine->push(stack)) { - // Success - return; - } - - // Free and uninstall full magazine - allocator->free_magazine(_magazine); - _magazine = nullptr; - } -} - -bool ZMarkThreadLocalStacks::push_slow(ZMarkStackAllocator* allocator, - ZMarkStripe* stripe, - ZMarkStack** stackp, - ZMarkTerminate* terminate, - ZMarkStackEntry entry, - bool publish) { - ZMarkStack* stack = *stackp; - - for (;;) { - if (stack == nullptr) { - // Allocate and install new stack - *stackp = stack = allocate_stack(allocator); - if (stack == nullptr) { - // Out of mark stack memory - return false; - } - } - - if (stack->push(entry)) { - // Success - return true; - } - - // Publish/Overflow and uninstall stack - stripe->publish_stack(stack, terminate, publish); - *stackp = stack = nullptr; - } -} - -bool ZMarkThreadLocalStacks::pop_slow(ZMarkStackAllocator* allocator, - ZMarkStripe* stripe, - ZMarkStack** stackp, - ZMarkStackEntry& entry) { - ZMarkStack* stack = *stackp; - - for (;;) { - if (stack == nullptr) { - // Try steal and install stack - *stackp = stack = stripe->steal_stack(); - if (stack == nullptr) { - // Nothing to steal - return false; - } - } - - if (stack->pop(entry)) { - // Success - return true; - } - - // Free and uninstall stack - free_stack(allocator, stack); - *stackp = stack = nullptr; - } -} - -bool ZMarkThreadLocalStacks::flush(ZMarkStackAllocator* allocator, ZMarkStripeSet* stripes, ZMarkTerminate* terminate) { +bool ZMarkThreadLocalStacks::flush(ZMarkStripeSet* stripes, + ZMarkTerminate* terminate) { bool flushed = false; // Flush all stacks @@ -220,22 +311,10 @@ bool ZMarkThreadLocalStacks::flush(ZMarkStackAllocator* allocator, ZMarkStripeSe } // Free/Publish and uninstall stack - if (stack->is_empty()) { - free_stack(allocator, stack); - } else { - stripe->publish_stack(stack, terminate, true /* publish */); - flushed = true; - } + stripe->publish_stack(stack, terminate, true /* publish */); + flushed = true; *stackp = nullptr; } return flushed; } - -void ZMarkThreadLocalStacks::free(ZMarkStackAllocator* allocator) { - // Free and uninstall magazine - if (_magazine != nullptr) { - allocator->free_magazine(_magazine); - _magazine = nullptr; - } -} diff --git a/src/hotspot/share/gc/z/zMarkStack.hpp b/src/hotspot/share/gc/z/zMarkStack.hpp index 8a24c84c8d9c8..7b32956c30097 100644 --- a/src/hotspot/share/gc/z/zMarkStack.hpp +++ b/src/hotspot/share/gc/z/zMarkStack.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, 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 @@ -26,71 +26,76 @@ #include "gc/z/zGlobals.hpp" #include "gc/z/zMarkStackEntry.hpp" +#include "memory/allocation.hpp" #include "utilities/globalDefinitions.hpp" +class ZMarkingSMR; +class ZMarkStripe; class ZMarkTerminate; -template -class ZStack { +class ZMarkStack { private: - size_t _top; - ZStack* _next; - T _slots[S]; + size_t _top; + size_t _capacity; - bool is_full() const; + ZMarkStackEntry* slots(); + + ZMarkStack(size_t capacity); public: - ZStack(); + static ZMarkStack* create(bool first_stack); + static void destroy(ZMarkStack* stack); bool is_empty() const; + bool is_full() const; - bool push(T value); - bool pop(T& value); - - ZStack* next() const; - ZStack** next_addr(); + void push(ZMarkStackEntry value); + ZMarkStackEntry pop(); }; -template -class ZCACHE_ALIGNED ZStackList { +class ZMarkStackListNode : public CHeapObj { private: - uintptr_t _base; - T* volatile _head; + ZMarkStack* const _stack; + ZMarkStackListNode* _next; - T* encode_versioned_pointer(const T* stack, uint32_t version) const; - void decode_versioned_pointer(const T* vstack, T** stack, uint32_t* version) const; +public: + ZMarkStackListNode(ZMarkStack* stack); + + ZMarkStack* stack() const; + + ZMarkStackListNode* next() const; + void set_next(ZMarkStackListNode* next); +}; + +class ZCACHE_ALIGNED ZMarkStackList { +private: + ZMarkStackListNode* volatile _head; + ssize_t volatile _length; public: - explicit ZStackList(uintptr_t base); + ZMarkStackList(); bool is_empty() const; - void push(T* stack); - T* pop(); + size_t length() const; - void clear(); + void push(ZMarkStack* stack); + ZMarkStack* pop(ZMarkingSMR* marking_smr); }; -using ZMarkStack = ZStack; -using ZMarkStackList = ZStackList; -using ZMarkStackMagazine = ZStack; -using ZMarkStackMagazineList = ZStackList; - -static_assert(sizeof(ZMarkStack) == ZMarkStackSize, "ZMarkStack size mismatch"); -static_assert(sizeof(ZMarkStackMagazine) <= ZMarkStackSize, "ZMarkStackMagazine size too large"); - class ZMarkStripe { private: ZCACHE_ALIGNED ZMarkStackList _published; ZCACHE_ALIGNED ZMarkStackList _overflowed; public: - explicit ZMarkStripe(uintptr_t base = 0); + explicit ZMarkStripe(); bool is_empty() const; + size_t population() const; void publish_stack(ZMarkStack* stack, ZMarkTerminate* terminate, bool publish); - ZMarkStack* steal_stack(); + ZMarkStack* steal_stack(ZMarkingSMR* marking_smr); }; class ZMarkStripeSet { @@ -99,12 +104,14 @@ class ZMarkStripeSet { ZMarkStripe _stripes[ZMarkStripesMax]; public: - explicit ZMarkStripeSet(uintptr_t base); + explicit ZMarkStripeSet(); void set_nstripes(size_t nstripes); + bool try_set_nstripes(size_t old_nstripes, size_t new_nstripes); size_t nstripes() const; bool is_empty() const; + bool is_crowded() const; size_t stripe_id(const ZMarkStripe* stripe) const; ZMarkStripe* stripe_at(size_t index); @@ -113,27 +120,9 @@ class ZMarkStripeSet { ZMarkStripe* stripe_for_addr(uintptr_t addr); }; -class ZMarkStackAllocator; - class ZMarkThreadLocalStacks { private: - ZMarkStackMagazine* _magazine; - ZMarkStack* _stacks[ZMarkStripesMax]; - - ZMarkStack* allocate_stack(ZMarkStackAllocator* allocator); - void free_stack(ZMarkStackAllocator* allocator, ZMarkStack* stack); - - bool push_slow(ZMarkStackAllocator* allocator, - ZMarkStripe* stripe, - ZMarkStack** stackp, - ZMarkTerminate* terminate, - ZMarkStackEntry entry, - bool publish); - - bool pop_slow(ZMarkStackAllocator* allocator, - ZMarkStripe* stripe, - ZMarkStack** stackp, - ZMarkStackEntry& entry); + ZMarkStack* _stacks[ZMarkStripesMax]; public: ZMarkThreadLocalStacks(); @@ -147,23 +136,19 @@ class ZMarkThreadLocalStacks { ZMarkStack* steal(ZMarkStripeSet* stripes, ZMarkStripe* stripe); - bool push(ZMarkStackAllocator* allocator, - ZMarkStripeSet* stripes, + void push(ZMarkStripeSet* stripes, ZMarkStripe* stripe, ZMarkTerminate* terminate, ZMarkStackEntry entry, bool publish); - bool pop(ZMarkStackAllocator* allocator, + bool pop(ZMarkingSMR* marking_smr, ZMarkStripeSet* stripes, ZMarkStripe* stripe, - ZMarkStackEntry& entry); + ZMarkStackEntry* entry); - bool flush(ZMarkStackAllocator* allocator, - ZMarkStripeSet* stripes, + bool flush(ZMarkStripeSet* stripes, ZMarkTerminate* terminate); - - void free(ZMarkStackAllocator* allocator); }; #endif // SHARE_GC_Z_ZMARKSTACK_HPP diff --git a/src/hotspot/share/gc/z/zMarkStack.inline.hpp b/src/hotspot/share/gc/z/zMarkStack.inline.hpp index cd8667dd73ff9..9c4a8efa3c5ec 100644 --- a/src/hotspot/share/gc/z/zMarkStack.inline.hpp +++ b/src/hotspot/share/gc/z/zMarkStack.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, 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 @@ -27,141 +27,30 @@ #include "gc/z/zMarkStack.hpp" #include "gc/z/zMarkTerminate.inline.hpp" -#include "runtime/atomic.hpp" #include "utilities/debug.hpp" -template -inline ZStack::ZStack() - : _top(0), - _next(nullptr) {} - -template -inline bool ZStack::is_empty() const { +inline bool ZMarkStack::is_empty() const { return _top == 0; } -template -inline bool ZStack::is_full() const { - return _top == S; +inline bool ZMarkStack::is_full() const { + return _top == _capacity; } -template -inline bool ZStack::push(T value) { - if (is_full()) { - return false; - } - - _slots[_top++] = value; - return true; -} - -template -inline bool ZStack::pop(T& value) { - if (is_empty()) { - return false; - } - - value = _slots[--_top]; - return true; -} - -template -inline ZStack* ZStack::next() const { - return _next; -} - -template -inline ZStack** ZStack::next_addr() { - return &_next; -} - -template -inline ZStackList::ZStackList(uintptr_t base) - : _base(base), - _head(encode_versioned_pointer(nullptr, 0)) {} - -template -inline T* ZStackList::encode_versioned_pointer(const T* stack, uint32_t version) const { - uint64_t addr; - - if (stack == nullptr) { - addr = (uint32_t)-1; - } else { - addr = ((uint64_t)stack - _base) >> ZMarkStackSizeShift; - } - - return (T*)((addr << 32) | (uint64_t)version); -} - -template -inline void ZStackList::decode_versioned_pointer(const T* vstack, T** stack, uint32_t* version) const { - const uint64_t addr = (uint64_t)vstack >> 32; - - if (addr == (uint32_t)-1) { - *stack = nullptr; - } else { - *stack = (T*)((addr << ZMarkStackSizeShift) + _base); - } - - *version = (uint32_t)(uint64_t)vstack; -} - -template -inline bool ZStackList::is_empty() const { - const T* vstack = _head; - T* stack = nullptr; - uint32_t version = 0; - - decode_versioned_pointer(vstack, &stack, &version); - return stack == nullptr; -} - -template -inline void ZStackList::push(T* stack) { - T* vstack = _head; - uint32_t version = 0; - - for (;;) { - decode_versioned_pointer(vstack, stack->next_addr(), &version); - T* const new_vstack = encode_versioned_pointer(stack, version + 1); - T* const prev_vstack = Atomic::cmpxchg(&_head, vstack, new_vstack); - if (prev_vstack == vstack) { - // Success - break; - } - - // Retry - vstack = prev_vstack; - } +inline ZMarkStackEntry* ZMarkStack::slots() { + uintptr_t start = (uintptr_t)this; + uintptr_t result = start + sizeof(ZMarkStack); + return (ZMarkStackEntry*)result; } -template -inline T* ZStackList::pop() { - T* vstack = _head; - T* stack = nullptr; - uint32_t version = 0; - - for (;;) { - decode_versioned_pointer(vstack, &stack, &version); - if (stack == nullptr) { - return nullptr; - } - - T* const new_vstack = encode_versioned_pointer(stack->next(), version + 1); - T* const prev_vstack = Atomic::cmpxchg(&_head, vstack, new_vstack); - if (prev_vstack == vstack) { - // Success - return stack; - } - - // Retry - vstack = prev_vstack; - } +inline void ZMarkStack::push(ZMarkStackEntry value) { + assert(!is_full(), "can't push to full stack"); + slots()[_top++] = value; } -template -inline void ZStackList::clear() { - _head = encode_versioned_pointer(nullptr, 0); +inline ZMarkStackEntry ZMarkStack::pop() { + assert(!is_empty(), "can't pop from empty stack"); + return slots()[--_top]; } inline bool ZMarkStripe::is_empty() const { @@ -175,6 +64,8 @@ inline void ZMarkStripe::publish_stack(ZMarkStack* stack, ZMarkTerminate* termin // to publish stacks that overflowed. The intention here is to avoid // contention between mutators and GC workers as much as possible, while // still allowing GC workers to help out and steal work from each other. + assert(!stack->is_empty(), "we never publish empty stacks"); + if (publish) { _published.push(stack); } else { @@ -184,16 +75,6 @@ inline void ZMarkStripe::publish_stack(ZMarkStack* stack, ZMarkTerminate* termin terminate->wake_up(); } -inline ZMarkStack* ZMarkStripe::steal_stack() { - // Steal overflowed stacks first, then published stacks - ZMarkStack* const stack = _overflowed.pop(); - if (stack != nullptr) { - return stack; - } - - return _published.pop(); -} - inline size_t ZMarkStripeSet::stripe_id(const ZMarkStripe* stripe) const { const size_t index = ((uintptr_t)stripe - (uintptr_t)_stripes) / sizeof(ZMarkStripe); assert(index < ZMarkStripesMax, "Invalid index"); @@ -236,32 +117,63 @@ inline ZMarkStack* ZMarkThreadLocalStacks::steal(ZMarkStripeSet* stripes, return stack; } -inline bool ZMarkThreadLocalStacks::push(ZMarkStackAllocator* allocator, - ZMarkStripeSet* stripes, +inline void ZMarkThreadLocalStacks::push(ZMarkStripeSet* stripes, ZMarkStripe* stripe, ZMarkTerminate* terminate, ZMarkStackEntry entry, bool publish) { - ZMarkStack** const stackp = &_stacks[stripes->stripe_id(stripe)]; - ZMarkStack* const stack = *stackp; - if (stack != nullptr && stack->push(entry)) { - return true; + const size_t stripe_id = stripes->stripe_id(stripe); + ZMarkStack** const stackp = &_stacks[stripe_id]; + ZMarkStack* const prev_stack = *stackp; + + if (prev_stack != nullptr) { + if (!prev_stack->is_full()) { + // There's a stack and it isn't full: just push + prev_stack->push(entry); + return; + } + + // Publish full stacks + stripe->publish_stack(prev_stack, terminate, publish); + *stackp = nullptr; } - return push_slow(allocator, stripe, stackp, terminate, entry, publish); + // If no stack was available, allocate one and push to it + const bool first_stack = prev_stack == nullptr; + ZMarkStack* const new_stack = ZMarkStack::create(first_stack); + *stackp = new_stack; + + new_stack->push(entry); } -inline bool ZMarkThreadLocalStacks::pop(ZMarkStackAllocator* allocator, +inline bool ZMarkThreadLocalStacks::pop(ZMarkingSMR* marking_smr, ZMarkStripeSet* stripes, ZMarkStripe* stripe, - ZMarkStackEntry& entry) { + ZMarkStackEntry* entry) { ZMarkStack** const stackp = &_stacks[stripes->stripe_id(stripe)]; - ZMarkStack* const stack = *stackp; - if (stack != nullptr && stack->pop(entry)) { - return true; + ZMarkStack* stack = *stackp; + + // First make sure there is a stack to pop from + if (stack == nullptr) { + // If we have no stack, try to steal one + stack = stripe->steal_stack(marking_smr); + *stackp = stack; + + if (stack == nullptr) { + // Out of stacks to pop from + return false; + } + } + + *entry = stack->pop(); + + if (stack->is_empty()) { + // Eagerly free empty stacks while on a worker thread + ZMarkStack::destroy(stack); + *stackp = nullptr; } - return pop_slow(allocator, stripe, stackp, entry); + return true; } #endif // SHARE_GC_Z_ZMARKSTACK_INLINE_HPP diff --git a/src/hotspot/share/gc/z/zMarkStackAllocator.cpp b/src/hotspot/share/gc/z/zMarkStackAllocator.cpp deleted file mode 100644 index 02a903985e71c..0000000000000 --- a/src/hotspot/share/gc/z/zMarkStackAllocator.cpp +++ /dev/null @@ -1,236 +0,0 @@ -/* - * Copyright (c) 2016, 2025, 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -#include "gc/shared/gc_globals.hpp" -#include "gc/z/zInitialize.hpp" -#include "gc/z/zLock.inline.hpp" -#include "gc/z/zMarkStack.inline.hpp" -#include "gc/z/zMarkStackAllocator.hpp" -#include "logging/log.hpp" -#include "runtime/atomic.hpp" -#include "runtime/os.hpp" -#include "utilities/debug.hpp" - -ZMarkStackSpace::ZMarkStackSpace() - : _expand_lock(), - _start(0), - _top(0), - _end(0) { - assert(ZMarkStackSpaceLimit >= ZMarkStackSpaceExpandSize, "ZMarkStackSpaceLimit too small"); - - // Reserve address space - const size_t size = ZMarkStackSpaceLimit; - const uintptr_t addr = (uintptr_t)os::reserve_memory(size, !ExecMem, mtGC); - if (addr == 0) { - ZInitialize::error_d("Failed to reserve address space for mark stacks"); - return; - } - - // Successfully initialized - _start = _top = _end = addr; - - // Prime space - _end += expand_space(); -} - -bool ZMarkStackSpace::is_initialized() const { - return _start != 0; -} - -uintptr_t ZMarkStackSpace::start() const { - return _start; -} - -size_t ZMarkStackSpace::size() const { - return _end - _start; -} - -size_t ZMarkStackSpace::used() const { - return _top - _start; -} - -size_t ZMarkStackSpace::expand_space() { - const size_t expand_size = ZMarkStackSpaceExpandSize; - const size_t old_size = size(); - const size_t new_size = old_size + expand_size; - - if (new_size > ZMarkStackSpaceLimit) { - // Expansion limit reached. This is a fatal error since we - // currently can't recover from running out of mark stack space. - fatal("Mark stack space exhausted. Use -XX:ZMarkStackSpaceLimit= to increase the " - "maximum number of bytes allocated for mark stacks. Current limit is %zuM.", - ZMarkStackSpaceLimit / M); - } - - log_debug(gc, marking)("Expanding mark stack space: %zuM->%zuM", - old_size / M, new_size / M); - - // Expand - os::commit_memory_or_exit((char*)_end, expand_size, false /* executable */, "Mark stack space"); - - return expand_size; -} - -size_t ZMarkStackSpace::shrink_space() { - // Shrink to what is currently used - const size_t old_size = size(); - const size_t new_size = align_up(used(), ZMarkStackSpaceExpandSize); - const size_t shrink_size = old_size - new_size; - - if (shrink_size > 0) { - // Shrink - log_debug(gc, marking)("Shrinking mark stack space: %zuM->%zuM", - old_size / M, new_size / M); - - const uintptr_t shrink_start = _end - shrink_size; - os::uncommit_memory((char*)shrink_start, shrink_size, false /* executable */); - } - - return shrink_size; -} - -uintptr_t ZMarkStackSpace::alloc_space(size_t size) { - uintptr_t top = Atomic::load(&_top); - - for (;;) { - const uintptr_t end = Atomic::load(&_end); - const uintptr_t new_top = top + size; - if (new_top > end) { - // Not enough space left - return 0; - } - - const uintptr_t prev_top = Atomic::cmpxchg(&_top, top, new_top); - if (prev_top == top) { - // Success - return top; - } - - // Retry - top = prev_top; - } -} - -uintptr_t ZMarkStackSpace::expand_and_alloc_space(size_t size) { - ZLocker locker(&_expand_lock); - - // Retry allocation before expanding - uintptr_t addr = alloc_space(size); - if (addr != 0) { - return addr; - } - - // Expand - const size_t expand_size = expand_space(); - - // Increment top before end to make sure another - // thread can't steal out newly expanded space. - addr = Atomic::fetch_then_add(&_top, size); - Atomic::add(&_end, expand_size); - - return addr; -} - -uintptr_t ZMarkStackSpace::alloc(size_t size) { - assert(size <= ZMarkStackSpaceExpandSize, "Invalid size"); - - const uintptr_t addr = alloc_space(size); - if (addr != 0) { - return addr; - } - - return expand_and_alloc_space(size); -} - -void ZMarkStackSpace::free() { - _end -= shrink_space(); - _top = _start; -} - -ZMarkStackAllocator::ZMarkStackAllocator() - : _space(), - _freelist(_space.start()), - _expanded_recently(false) {} - -bool ZMarkStackAllocator::is_initialized() const { - return _space.is_initialized(); -} - -uintptr_t ZMarkStackAllocator::start() const { - return _space.start(); -} - -size_t ZMarkStackAllocator::size() const { - return _space.size(); -} - -ZMarkStackMagazine* ZMarkStackAllocator::create_magazine_from_space(uintptr_t addr, size_t size) { - assert(is_aligned(size, ZMarkStackSize), "Invalid size"); - - // Use first stack as magazine - ZMarkStackMagazine* const magazine = new ((void*)addr) ZMarkStackMagazine(); - for (size_t i = ZMarkStackSize; i < size; i += ZMarkStackSize) { - ZMarkStack* const stack = new ((void*)(addr + i)) ZMarkStack(); - const bool success = magazine->push(stack); - assert(success, "Magazine should never get full"); - } - - return magazine; -} - -ZMarkStackMagazine* ZMarkStackAllocator::alloc_magazine() { - // Try allocating from the free list first - ZMarkStackMagazine* const magazine = _freelist.pop(); - if (magazine != nullptr) { - return magazine; - } - - if (!Atomic::load(&_expanded_recently)) { - Atomic::cmpxchg(&_expanded_recently, false, true); - } - - // Allocate new magazine - const uintptr_t addr = _space.alloc(ZMarkStackMagazineSize); - if (addr == 0) { - return nullptr; - } - - return create_magazine_from_space(addr, ZMarkStackMagazineSize); -} - -bool ZMarkStackAllocator::clear_and_get_expanded_recently() { - if (!Atomic::load(&_expanded_recently)) { - return false; - } - - return Atomic::cmpxchg(&_expanded_recently, true, false); -} - -void ZMarkStackAllocator::free_magazine(ZMarkStackMagazine* magazine) { - _freelist.push(magazine); -} - -void ZMarkStackAllocator::free() { - _freelist.clear(); - _space.free(); -} diff --git a/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp b/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp index 79d3f8ef2a018..cebe08e6e5354 100644 --- a/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp +++ b/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp @@ -60,8 +60,7 @@ inline void ZMarkTerminate::leave() { inline void ZMarkTerminate::maybe_reduce_stripes(ZMarkStripeSet* stripes, size_t used_nstripes) { size_t nstripes = stripes->nstripes(); if (used_nstripes == nstripes && nstripes > 1u) { - nstripes >>= 1; - stripes->set_nstripes(nstripes); + stripes->try_set_nstripes(nstripes, nstripes >> 1); } } diff --git a/src/hotspot/share/gc/z/zMarkingSMR.cpp b/src/hotspot/share/gc/z/zMarkingSMR.cpp new file mode 100644 index 0000000000000..82281e6f7c830 --- /dev/null +++ b/src/hotspot/share/gc/z/zMarkingSMR.cpp @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2016, 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#include "gc/z/zMarkingSMR.hpp" +#include "gc/z/zMarkStack.inline.hpp" +#include "gc/z/zValue.inline.hpp" +#include "runtime/atomic.hpp" + +ZMarkingSMR::ZMarkingSMR() + : _worker_states(), + _expanded_recently() { +} + +void ZMarkingSMR::free_node(ZMarkStackListNode* node) { + // We use hazard pointers as an safe memory reclamation (SMR) technique, + // for marking stacks. Each stripe has a lock-free stack of mark stacks. + // When a GC thread (1) pops a mark stack from this lock-free stack, + // there is a small window of time when the head has been read and we + // are about to read its next pointer. It is then of great importance + // that the node is not concurrently freed by another concurrent GC + // thread (2), popping the same entry. In such an event, the memory + // of the freed node could, for example become part of a separate + // node, and potentially pushed onto a separate stripe, with a + // different next pointer referring to a node of the other stripe. + // When GC thread (1) then reads the next pointer of what it believed + // to be the current head node of the first stripe, it actually read + // a next pointer of a logically different node, pointing into the + // other stripe. GC thread (2) could then pop the node from the second + // mark stripe and re-insert it as the head of the first stripe. + // Disaster eventually hits when GC thread (1) succeeds with its + // CAS (ABA problem), switching the loaded head to the loaded next + // pointer of the head. Due to the next pointer belonging to a logically + // different node than the logical head, we can accidentally corrupt the + // stack integrity. Using hazarad pointers involves publishing what head + // was observed by GC thread (1), so that GC thread (2) knows not to + // free the node when popping it in this race. This prevents the racy + // interactions from causing any such use-after-free problems. + + assert(Thread::current()->is_Worker_thread(), "must be a worker"); + + ZWorkerState* local_state = _worker_states.addr(); + ZArray* freeing = &local_state->_freeing; + freeing->append(node); + + if (freeing->length() < (int)ZPerWorkerStorage::count() * 8) { + return; + } + + ZPerWorkerIterator iter(&_worker_states); + ZArray* scanned_hazards = &local_state->_scanned_hazards; + + for (ZWorkerState* remote_state; iter.next(&remote_state);) { + ZMarkStackListNode* hazard = Atomic::load(&remote_state->_hazard_ptr); + + if (hazard != nullptr) { + scanned_hazards->append(hazard); + } + } + + int kept = 0; + for (int i = 0; i < freeing->length(); ++i) { + ZMarkStackListNode* node = freeing->at(i); + freeing->at_put(i, nullptr); + + if (scanned_hazards->contains(node)) { + // Keep + freeing->at_put(kept++, node); + } else { + // Delete + delete node; + } + } + + scanned_hazards->clear(); + freeing->trunc_to(kept); +} + +void ZMarkingSMR::free() { + // Here it is free by definition to free mark stacks. + ZPerWorkerIterator iter(&_worker_states); + for (ZWorkerState* worker_state; iter.next(&worker_state);) { + ZArray* freeing = &worker_state->_freeing; + for (ZMarkStackListNode* node: *freeing) { + delete node; + } + freeing->clear(); + } +} + +ZMarkStackListNode* volatile* ZMarkingSMR::hazard_ptr() { + return &_worker_states.addr()->_hazard_ptr; +} diff --git a/src/hotspot/share/gc/z/zMarkStackAllocator.hpp b/src/hotspot/share/gc/z/zMarkingSMR.hpp similarity index 50% rename from src/hotspot/share/gc/z/zMarkStackAllocator.hpp rename to src/hotspot/share/gc/z/zMarkingSMR.hpp index dc18a23e101e6..a6c5afe76c941 100644 --- a/src/hotspot/share/gc/z/zMarkStackAllocator.hpp +++ b/src/hotspot/share/gc/z/zMarkingSMR.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, 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 @@ -24,61 +24,30 @@ #ifndef SHARE_GC_Z_ZMARKSTACKALLOCATOR_HPP #define SHARE_GC_Z_ZMARKSTACKALLOCATOR_HPP -#include "gc/z/zGlobals.hpp" -#include "gc/z/zLock.hpp" -#include "gc/z/zMarkStack.hpp" +#include "gc/z/zArray.hpp" +#include "gc/z/zValue.hpp" #include "utilities/globalDefinitions.hpp" +#include "memory/allocation.hpp" -class ZMarkStackSpace { -private: - ZLock _expand_lock; - uintptr_t _start; - volatile uintptr_t _top; - volatile uintptr_t _end; - volatile bool _recently_expanded; - - size_t used() const; - - size_t expand_space(); - size_t shrink_space(); - - uintptr_t alloc_space(size_t size); - uintptr_t expand_and_alloc_space(size_t size); - -public: - ZMarkStackSpace(); - - bool is_initialized() const; - - uintptr_t start() const; - size_t size() const; +class ZMarkStackListNode; - uintptr_t alloc(size_t size); - void free(); -}; - -class ZMarkStackAllocator : public CHeapObj { +class ZMarkingSMR: public CHeapObj { private: - ZCACHE_ALIGNED ZMarkStackSpace _space; - ZCACHE_ALIGNED ZMarkStackMagazineList _freelist; - ZCACHE_ALIGNED volatile bool _expanded_recently; + struct ZWorkerState { + ZMarkStackListNode* volatile _hazard_ptr; + ZArray _scanned_hazards; + ZArray _freeing; + }; - ZMarkStackMagazine* create_magazine_from_space(uintptr_t addr, size_t size); + ZPerWorker _worker_states; + volatile bool _expanded_recently; public: - ZMarkStackAllocator(); - - bool is_initialized() const; - - uintptr_t start() const; - size_t size() const; - - bool clear_and_get_expanded_recently(); - - ZMarkStackMagazine* alloc_magazine(); - void free_magazine(ZMarkStackMagazine* magazine); - + ZMarkingSMR(); void free(); + ZMarkStackListNode* allocate_stack(); + void free_node(ZMarkStackListNode* stack); + ZMarkStackListNode* volatile* hazard_ptr(); }; #endif // SHARE_GC_Z_ZMARKSTACKALLOCATOR_HPP diff --git a/src/hotspot/share/gc/z/zRemembered.cpp b/src/hotspot/share/gc/z/zRemembered.cpp index 4cfc349acc47e..3bb17152d4192 100644 --- a/src/hotspot/share/gc/z/zRemembered.cpp +++ b/src/hotspot/share/gc/z/zRemembered.cpp @@ -550,7 +550,7 @@ class ZRememberedScanMarkFollowTask : public ZRestartableTask { // publish such marking stacks to prevent that generation from getting a mark continue. // We also flush in case of a resize where a new worker thread continues the marking // work, causing a mark continue for the collected generation. - ZHeap::heap()->mark_flush_and_free(Thread::current()); + ZHeap::heap()->mark_flush(Thread::current()); } virtual void resize_workers(uint nworkers) { diff --git a/src/hotspot/share/gc/z/zStat.cpp b/src/hotspot/share/gc/z/zStat.cpp index 3fe60f48e8950..558ccdf105acd 100644 --- a/src/hotspot/share/gc/z/zStat.cpp +++ b/src/hotspot/share/gc/z/zStat.cpp @@ -1424,8 +1424,7 @@ ZStatMark::ZStatMark() _nproactiveflush(), _nterminateflush(), _ntrycomplete(), - _ncontinue(), - _mark_stack_usage() {} + _ncontinue() {} void ZStatMark::at_mark_start(size_t nstripes) { _nstripes = nstripes; @@ -1441,10 +1440,6 @@ void ZStatMark::at_mark_end(size_t nproactiveflush, _ncontinue = ncontinue; } -void ZStatMark::at_mark_free(size_t mark_stack_usage) { - _mark_stack_usage = mark_stack_usage; -} - void ZStatMark::print() { log_info(gc, marking)("Mark: " "%zu stripe(s), " @@ -1457,8 +1452,6 @@ void ZStatMark::print() { _nterminateflush, _ntrycomplete, _ncontinue); - - log_info(gc, marking)("Mark Stack Usage: %zuM", _mark_stack_usage / M); } // diff --git a/src/hotspot/share/gc/z/zStat.hpp b/src/hotspot/share/gc/z/zStat.hpp index d169385791890..0078c9636264a 100644 --- a/src/hotspot/share/gc/z/zStat.hpp +++ b/src/hotspot/share/gc/z/zStat.hpp @@ -506,7 +506,6 @@ class ZStatMark { size_t nterminateflush, size_t ntrycomplete, size_t ncontinue); - void at_mark_free(size_t mark_stack_usage); void print(); }; diff --git a/src/hotspot/share/gc/z/z_globals.hpp b/src/hotspot/share/gc/z/z_globals.hpp index 4e3076329691b..4555b470cac92 100644 --- a/src/hotspot/share/gc/z/z_globals.hpp +++ b/src/hotspot/share/gc/z/z_globals.hpp @@ -41,10 +41,6 @@ "Maximum allowed heap fragmentation") \ range(0, 100) \ \ - product(size_t, ZMarkStackSpaceLimit, 8*G, \ - "Maximum number of bytes allocated for mark stacks") \ - range(32*M, 1024*G) \ - \ product(double, ZCollectionInterval, 0, \ "Force GC at a fixed time interval (in seconds). " \ "Backwards compatible alias for ZCollectionIntervalMajor") \ diff --git a/src/hotspot/share/runtime/arguments.cpp b/src/hotspot/share/runtime/arguments.cpp index baeb591ef1a77..b3f70592f6f12 100644 --- a/src/hotspot/share/runtime/arguments.cpp +++ b/src/hotspot/share/runtime/arguments.cpp @@ -534,6 +534,7 @@ static SpecialFlag const special_jvm_flags[] = { { "MetaspaceReclaimPolicy", JDK_Version::undefined(), JDK_Version::jdk(21), JDK_Version::undefined() }, { "ZGenerational", JDK_Version::jdk(23), JDK_Version::jdk(24), JDK_Version::undefined() }, + { "ZMarkStackSpaceLimit", JDK_Version::undefined(), JDK_Version::jdk(25), JDK_Version::undefined() }, #ifdef ASSERT { "DummyObsoleteTestFlag", JDK_Version::undefined(), JDK_Version::jdk(18), JDK_Version::undefined() }, From 7009d6dca4c54beef100537970ccf5b49d98d98e Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Thu, 20 Feb 2025 06:02:31 +0000 Subject: [PATCH 2/4] Use ZAttachedArray --- src/hotspot/share/gc/z/zMarkStack.cpp | 11 +++++------ src/hotspot/share/gc/z/zMarkStack.hpp | 7 +++++-- src/hotspot/share/gc/z/zMarkStack.inline.hpp | 7 +++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/hotspot/share/gc/z/zMarkStack.cpp b/src/hotspot/share/gc/z/zMarkStack.cpp index bc5b6f44db958..a47a9774bbf46 100644 --- a/src/hotspot/share/gc/z/zMarkStack.cpp +++ b/src/hotspot/share/gc/z/zMarkStack.cpp @@ -38,19 +38,18 @@ ZMarkStack* ZMarkStack::create(bool first_stack) { // overhead of churning around stacks on a stripe. size_t capacity = first_stack ? 128 : 512; - size_t size = sizeof(ZMarkStack) + capacity * sizeof(ZMarkStackEntry); - char* memory = NEW_C_HEAP_ARRAY(char, size, mtGC); - return new (memory) ZMarkStack(capacity); + void* const memory = AttachedArray::alloc(capacity); + return ::new (memory) ZMarkStack(capacity); } void ZMarkStack::destroy(ZMarkStack* stack) { - char* memory = (char*)stack; - FREE_C_HEAP_ARRAY(char, memory); + stack->~ZMarkStack(); + AttachedArray::free(stack); } ZMarkStack::ZMarkStack(size_t capacity) : _top(0), - _capacity(capacity) {} + _entries(capacity) {} ZMarkStackListNode::ZMarkStackListNode(ZMarkStack* stack) : _stack(stack), diff --git a/src/hotspot/share/gc/z/zMarkStack.hpp b/src/hotspot/share/gc/z/zMarkStack.hpp index 7b32956c30097..7c0e6d4c71209 100644 --- a/src/hotspot/share/gc/z/zMarkStack.hpp +++ b/src/hotspot/share/gc/z/zMarkStack.hpp @@ -24,6 +24,7 @@ #ifndef SHARE_GC_Z_ZMARKSTACK_HPP #define SHARE_GC_Z_ZMARKSTACK_HPP +#include "gc/z/zAttachedArray.hpp" #include "gc/z/zGlobals.hpp" #include "gc/z/zMarkStackEntry.hpp" #include "memory/allocation.hpp" @@ -35,8 +36,10 @@ class ZMarkTerminate; class ZMarkStack { private: - size_t _top; - size_t _capacity; + using AttachedArray = ZAttachedArray; + + size_t _top; + const AttachedArray _entries; ZMarkStackEntry* slots(); diff --git a/src/hotspot/share/gc/z/zMarkStack.inline.hpp b/src/hotspot/share/gc/z/zMarkStack.inline.hpp index 9c4a8efa3c5ec..ed3b167762cde 100644 --- a/src/hotspot/share/gc/z/zMarkStack.inline.hpp +++ b/src/hotspot/share/gc/z/zMarkStack.inline.hpp @@ -26,6 +26,7 @@ #include "gc/z/zMarkStack.hpp" +#include "gc/z/zAttachedArray.inline.hpp" #include "gc/z/zMarkTerminate.inline.hpp" #include "utilities/debug.hpp" @@ -34,13 +35,11 @@ inline bool ZMarkStack::is_empty() const { } inline bool ZMarkStack::is_full() const { - return _top == _capacity; + return _top == _entries.length(); } inline ZMarkStackEntry* ZMarkStack::slots() { - uintptr_t start = (uintptr_t)this; - uintptr_t result = start + sizeof(ZMarkStack); - return (ZMarkStackEntry*)result; + return _entries(this); } inline void ZMarkStack::push(ZMarkStackEntry value) { From 89491f1ad71867f41c336bdb710200f80799baf3 Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Thu, 20 Feb 2025 06:49:46 +0000 Subject: [PATCH 3/4] Spelling and const --- src/hotspot/share/gc/z/zMark.cpp | 2 +- src/hotspot/share/gc/z/zMarkStack.cpp | 18 +++++++++--------- src/hotspot/share/gc/z/zMarkingSMR.cpp | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/hotspot/share/gc/z/zMark.cpp b/src/hotspot/share/gc/z/zMark.cpp index 89ed585b5c2ab..e0c68213ca3e8 100644 --- a/src/hotspot/share/gc/z/zMark.cpp +++ b/src/hotspot/share/gc/z/zMark.cpp @@ -473,7 +473,7 @@ bool ZMark::rebalance_work(ZMarkContext* context) { // We are running on a reduced number of threads to minimize the amount of work // hidden in local stacks when the stripes are less well balanced. When this situation // starts getting crowded, we bump the number of stripes again. - size_t new_nstripes = nstripes << 1; + const size_t new_nstripes = nstripes << 1; if (_stripes.try_set_nstripes(nstripes, new_nstripes)) { context->set_nstripes(new_nstripes); } diff --git a/src/hotspot/share/gc/z/zMarkStack.cpp b/src/hotspot/share/gc/z/zMarkStack.cpp index a47a9774bbf46..692defc50f184 100644 --- a/src/hotspot/share/gc/z/zMarkStack.cpp +++ b/src/hotspot/share/gc/z/zMarkStack.cpp @@ -36,7 +36,7 @@ ZMarkStack* ZMarkStack::create(bool first_stack) { // threads instead. Once more than one stack is needed, we revert // to a larger stack size instead, which reduces synchronization // overhead of churning around stacks on a stripe. - size_t capacity = first_stack ? 128 : 512; + const size_t capacity = first_stack ? 128 : 512; void* const memory = AttachedArray::alloc(capacity); return ::new (memory) ZMarkStack(capacity); @@ -76,13 +76,13 @@ bool ZMarkStackList::is_empty() const { } void ZMarkStackList::push(ZMarkStack* stack) { - ZMarkStackListNode* node = new ZMarkStackListNode(stack); + ZMarkStackListNode* const node = new ZMarkStackListNode(stack); ZMarkStackListNode* head = Atomic::load(&_head); for (;;) { node->set_next(head); // Between reading the head and the linearizing CAS that pushes // the node onto the list, there could be an ABA problem. Except, - // on the pushing sidee, that is benign. The node is never + // on the pushing side, that is benign. The node is never // dereferenced while pushing and if we were to detect the ABA // situation and run this loop one more time, we would end up // having the same side effects: set the next pointer to the same @@ -103,7 +103,7 @@ void ZMarkStackList::push(ZMarkStack* stack) { } ZMarkStack* ZMarkStackList::pop(ZMarkingSMR* marking_smr) { - ZMarkStackListNode* volatile* hazard_ptr = marking_smr->hazard_ptr(); + ZMarkStackListNode* volatile* const hazard_ptr = marking_smr->hazard_ptr(); ZMarkStackListNode* head = Atomic::load(&_head); for (;;) { @@ -119,7 +119,7 @@ ZMarkStack* ZMarkStackList::pop(ZMarkingSMR* marking_smr) { // A full fence is needed to ensure the store and subsequent load do // not reorder. If they did reorder, the second head load could happen - // before other threads scanning hazard poitners can observe it, meaning + // before other threads scanning hazard pointers can observe it, meaning // it could get concurrently freed. OrderAccess::fence(); @@ -167,7 +167,7 @@ ZMarkStack* ZMarkStackList::pop(ZMarkingSMR* marking_smr) { } size_t ZMarkStackList::length() const { - ssize_t result = Atomic::load(&_length); + const ssize_t result = Atomic::load(&_length); if (result < 0) { return 0; @@ -204,7 +204,7 @@ void ZMarkStripeSet::set_nstripes(size_t nstripes) { assert(nstripes >= 1, "Invalid number of stripes"); assert(nstripes <= ZMarkStripesMax, "Invalid number of stripes"); - size_t new_nstripes_mask = nstripes - 1; + const size_t new_nstripes_mask = nstripes - 1; _nstripes_mask = new_nstripes_mask; log_debug(gc, marking)("Using %zu mark stripes", nstripes); @@ -216,8 +216,8 @@ bool ZMarkStripeSet::try_set_nstripes(size_t old_nstripes, size_t new_nstripes) assert(new_nstripes >= 1, "Invalid number of stripes"); assert(new_nstripes <= ZMarkStripesMax, "Invalid number of stripes"); - size_t old_nstripes_mask = old_nstripes - 1; - size_t new_nstripes_mask = new_nstripes - 1; + const size_t old_nstripes_mask = old_nstripes - 1; + const size_t new_nstripes_mask = new_nstripes - 1; // Mutators may read these values concurrently. It doesn't matter // if they see the old or new values. diff --git a/src/hotspot/share/gc/z/zMarkingSMR.cpp b/src/hotspot/share/gc/z/zMarkingSMR.cpp index 82281e6f7c830..2335d009c74ff 100644 --- a/src/hotspot/share/gc/z/zMarkingSMR.cpp +++ b/src/hotspot/share/gc/z/zMarkingSMR.cpp @@ -51,15 +51,15 @@ void ZMarkingSMR::free_node(ZMarkStackListNode* node) { // CAS (ABA problem), switching the loaded head to the loaded next // pointer of the head. Due to the next pointer belonging to a logically // different node than the logical head, we can accidentally corrupt the - // stack integrity. Using hazarad pointers involves publishing what head + // stack integrity. Using hazard pointers involves publishing what head // was observed by GC thread (1), so that GC thread (2) knows not to // free the node when popping it in this race. This prevents the racy // interactions from causing any such use-after-free problems. assert(Thread::current()->is_Worker_thread(), "must be a worker"); - ZWorkerState* local_state = _worker_states.addr(); - ZArray* freeing = &local_state->_freeing; + ZWorkerState* const local_state = _worker_states.addr(); + ZArray* const freeing = &local_state->_freeing; freeing->append(node); if (freeing->length() < (int)ZPerWorkerStorage::count() * 8) { @@ -67,10 +67,10 @@ void ZMarkingSMR::free_node(ZMarkStackListNode* node) { } ZPerWorkerIterator iter(&_worker_states); - ZArray* scanned_hazards = &local_state->_scanned_hazards; + ZArray* const scanned_hazards = &local_state->_scanned_hazards; for (ZWorkerState* remote_state; iter.next(&remote_state);) { - ZMarkStackListNode* hazard = Atomic::load(&remote_state->_hazard_ptr); + ZMarkStackListNode* const hazard = Atomic::load(&remote_state->_hazard_ptr); if (hazard != nullptr) { scanned_hazards->append(hazard); @@ -99,7 +99,7 @@ void ZMarkingSMR::free() { // Here it is free by definition to free mark stacks. ZPerWorkerIterator iter(&_worker_states); for (ZWorkerState* worker_state; iter.next(&worker_state);) { - ZArray* freeing = &worker_state->_freeing; + ZArray* const freeing = &worker_state->_freeing; for (ZMarkStackListNode* node: *freeing) { delete node; } From 7962b9a92d5ec962e838b665f1dddef787b24d6a Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Thu, 20 Feb 2025 06:50:02 +0000 Subject: [PATCH 4/4] Preexisting: Missing Include --- src/hotspot/share/gc/z/zMarkTerminate.inline.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp b/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp index cebe08e6e5354..cdd9e7dce2525 100644 --- a/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp +++ b/src/hotspot/share/gc/z/zMarkTerminate.inline.hpp @@ -28,6 +28,7 @@ #include "gc/shared/suspendibleThreadSet.hpp" #include "gc/z/zLock.inline.hpp" +#include "gc/z/zMarkStack.hpp" #include "logging/log.hpp" #include "runtime/atomic.hpp" #include "runtime/osThread.hpp"