On Fri, 21 Nov 2025 18:53:17 GMT, Mat Carter <[email protected]> wrote:
>> src/hotspot/share/cds/aotMetaspace.cpp line 1062:
>>
>>> 1060: bool AOTMetaspace::preimage_static_archive_dumped() {
>>> 1061: assert(CDSConfig::is_dumping_preimage_static_archive(), "Required");
>>> 1062: return _preimage_static_archive_dumped == 1;
>>
>> Should it be AtomicAccess::load(&_preimage_static_archive_dumped) here?
>
> Andrew brought this up also; I'll add but should it be AtomicAccess::load or
> AtomicAccess::load_acquire?
Just in case this might be called from a different thread to the one which is
executing the `cmpxchg` you should really be using
`AtomicAccess::load_acquire(&_preimage_static_archive_dumped)` here
You also need acquire-release semantics for the comparison below which you get
as the default from your current use of `cmpxchg` below.
if (AtomicAccess::cmpxchg(&_preimage_static_archive_dumped, 0, 1) != 0) {
Depending on the implementation that might be more heavyweight than you really
need. e.g. given that the change is idempotent you could rely on a bare `xchg`
with acq_rel semantics
if (AtomicAccess::xchg(&_preimage_static_archive_dumped, 1,
atomic_memory_order::memory_order_acq_rel) != 0) {
However, it's not going to matter very much since these are low-frequency
operations.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2555662830
