From 0a70da46f6fe98711f6f4b8b4e49e8ee3e4cb59e Mon Sep 17 00:00:00 2001 From: Kim Barrett Date: Fri, 19 Dec 2025 22:37:09 +0000 Subject: [PATCH] 8372564: Convert StringDedup to use Atomic Reviewed-by: tschatzl, aboldtch, iwalulya --- .../gc/shared/stringdedup/stringDedupProcessor.cpp | 11 +++++------ .../gc/shared/stringdedup/stringDedupProcessor.hpp | 5 +++-- .../gc/shared/stringdedup/stringDedupStorageUse.cpp | 13 ++++++------- .../gc/shared/stringdedup/stringDedupStorageUse.hpp | 7 ++++--- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.cpp index b8a27f31d01..765d247cb91 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; +Atomic 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_requests.load_relaxed() == 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]); + _storage_for_requests.store_relaxed(new StorageUse(_storages[0])); _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 diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupProcessor.hpp index 7e5e64df9b4..d111e750c2d 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,6 +27,7 @@ #include "gc/shared/stringdedup/stringDedup.hpp" #include "memory/allocation.hpp" +#include "runtime/atomic.hpp" #include "services/cpuTimeUsage.hpp" #include "utilities/macros.hpp" @@ -51,7 +52,7 @@ class StringDedup::Processor : public CHeapObj { NONCOPYABLE(Processor); static OopStorage* _storages[2]; - static StorageUse* volatile _storage_for_requests; + static Atomic _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 427058f3e7e..f77532161f7 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 8439b9e557e..ad8cb9eb2c9 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 @@ public: 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.