From a14d77ce7f0e2047050eb78eb2a12d7ec4eaf46f Mon Sep 17 00:00:00 2001 From: Kim Barrett Date: Wed, 26 Nov 2025 08:11:10 +0000 Subject: [PATCH] stringdedup uses Atomic --- .../gc/shared/stringdedup/stringDedupProcessor.cpp | 13 ++++++------- .../gc/shared/stringdedup/stringDedupProcessor.hpp | 6 ++++-- .../gc/shared/stringdedup/stringDedupStorageUse.cpp | 13 ++++++------- .../gc/shared/stringdedup/stringDedupStorageUse.hpp | 7 ++++--- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp index b8a27f31d0189..f53881522482a 100644 --- a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp +++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp @@ -36,7 +36,6 @@ #include "memory/iterator.hpp" #include "nmt/memTag.hpp" #include "oops/access.inline.hpp" -#include "runtime/atomicAccess.hpp" #include "runtime/cpuTimeCounters.hpp" #include "runtime/interfaceSupport.inline.hpp" #include "runtime/mutexLocker.hpp" @@ -46,17 +45,17 @@ OopStorage* StringDedup::Processor::_storages[2] = {}; -StringDedup::StorageUse* volatile StringDedup::Processor::_storage_for_requests = nullptr; +DeferredStatic> StringDedup::Processor::_storage_for_requests{}; StringDedup::StorageUse* StringDedup::Processor::_storage_for_processing = nullptr; void StringDedup::Processor::initialize_storage() { assert(_storages[0] == nullptr, "storage already created"); assert(_storages[1] == nullptr, "storage already created"); - assert(_storage_for_requests == nullptr, "storage already created"); assert(_storage_for_processing == nullptr, "storage already created"); _storages[0] = OopStorageSet::create_weak("StringDedup Requests0 Weak", mtStringDedup); _storages[1] = OopStorageSet::create_weak("StringDedup Requests1 Weak", mtStringDedup); - _storage_for_requests = new StorageUse(_storages[0]); + auto sfr = new StorageUse(_storages[0]); // Work around lack of std::forward. + _storage_for_requests.initialize(sfr); _storage_for_processing = new StorageUse(_storages[1]); } @@ -75,7 +74,7 @@ void StringDedup::Processor::wait_for_requests() const { { ThreadBlockInVM tbivm(_thread); MonitorLocker ml(StringDedup_lock, Mutex::_no_safepoint_check_flag); - OopStorage* storage = AtomicAccess::load(&_storage_for_requests)->storage(); + OopStorage* storage = _storage_for_requests->load_relaxed()->storage(); while ((storage->allocation_count() == 0) && !Table::is_dead_entry_removal_needed()) { ml.wait(); @@ -83,7 +82,7 @@ void StringDedup::Processor::wait_for_requests() const { } // Swap the request and processing storage objects. log_trace(stringdedup)("swapping request storages"); - _storage_for_processing = AtomicAccess::xchg(&_storage_for_requests, _storage_for_processing); + _storage_for_processing = _storage_for_requests->exchange(_storage_for_processing); GlobalCounter::write_synchronize(); // Wait for the now current processing storage object to no longer be used // by an in-progress GC. Again here, the num-dead notification from the @@ -99,7 +98,7 @@ void StringDedup::Processor::wait_for_requests() const { } StringDedup::StorageUse* StringDedup::Processor::storage_for_requests() { - return StorageUse::obtain(&_storage_for_requests); + return StorageUse::obtain(_storage_for_requests.get()); } void StringDedup::Processor::yield() const { diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp index 7e5e64df9b430..d556763ee6df2 100644 --- a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp +++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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,7 +27,9 @@ #include "gc/shared/stringdedup/stringDedup.hpp" #include "memory/allocation.hpp" +#include "runtime/atomic.hpp" #include "services/cpuTimeUsage.hpp" +#include "utilities/deferredStatic.hpp" #include "utilities/macros.hpp" class JavaThread; @@ -51,7 +53,7 @@ class StringDedup::Processor : public CHeapObj { NONCOPYABLE(Processor); static OopStorage* _storages[2]; - static StorageUse* volatile _storage_for_requests; + static DeferredStatic> _storage_for_requests; static StorageUse* _storage_for_processing; JavaThread* _thread; diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.cpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.cpp index 427058f3e7eb0..f77532161f72c 100644 --- a/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.cpp +++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.cpp @@ -23,7 +23,6 @@ */ #include "gc/shared/stringdedup/stringDedupStorageUse.hpp" -#include "runtime/atomicAccess.hpp" #include "runtime/javaThread.hpp" #include "utilities/debug.hpp" #include "utilities/globalCounter.inline.hpp" @@ -34,18 +33,18 @@ StringDedup::StorageUse::StorageUse(OopStorage* storage) : {} bool StringDedup::StorageUse::is_used_acquire() const { - return AtomicAccess::load_acquire(&_use_count) > 0; + return _use_count.load_acquire() > 0; } StringDedup::StorageUse* -StringDedup::StorageUse::obtain(StorageUse* volatile* ptr) { +StringDedup::StorageUse::obtain(Atomic* ptr) { GlobalCounter::CriticalSection cs(Thread::current()); - StorageUse* storage = AtomicAccess::load(ptr); - AtomicAccess::inc(&storage->_use_count); + StorageUse* storage = ptr->load_relaxed(); + storage->_use_count.add_then_fetch(1u); return storage; } void StringDedup::StorageUse::relinquish() { - size_t result = AtomicAccess::sub(&_use_count, size_t(1)); - assert(result != SIZE_MAX, "use count underflow"); + size_t result = _use_count.fetch_then_sub(1u); + assert(result != 0, "use count underflow"); } diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.hpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.hpp index 8439b9e557e02..ad8cb9eb2c956 100644 --- a/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.hpp +++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupStorageUse.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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,6 +27,7 @@ #include "gc/shared/stringdedup/stringDedup.hpp" #include "memory/allocation.hpp" +#include "runtime/atomic.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" @@ -35,7 +36,7 @@ class OopStorage; // Manage access to one of the OopStorage objects used for requests. class StringDedup::StorageUse : public CHeapObj { OopStorage* const _storage; - volatile size_t _use_count; + Atomic _use_count; NONCOPYABLE(StorageUse); @@ -48,7 +49,7 @@ class StringDedup::StorageUse : public CHeapObj { bool is_used_acquire() const; // Get the current requests object, and increment its in-use count. - static StorageUse* obtain(StorageUse* volatile* ptr); + static StorageUse* obtain(Atomic* ptr); // Discard a prior "obtain" request, decrementing the in-use count, and // permitting the deduplication thread to start processing if needed.