On Mon, 21 Aug 2023 21:47:14 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

> During [JDK-8151815](https://bugs.openjdk.org/browse/JDK-8151815) it was 
> noted that the PerfMemory _initialized and _destroyed fields should be 
> volatile, but VMStructs didn't have the needed support for doing that, so it 
> was left as a future task. @YaSuenag provided a patch at the time to take 
> care of the VMStructs support. I've integrated it, although it was far from 
> clean due to some changes in VMStructs, and also moving 
> OrderAccess::release_store to Atomic::release_store.
> 
> One other change I made to the patch had to do with consistency with using 
> "volatile static" vs "static volatile". We already have 
> volatile_nonstatic_field. The patch renamed static_ptr_volatile_field to 
> static_volatile_field to make it more general purpose, but this was 
> inconsistent with the name of volatile_nonstatic_field, so I chose the name 
> volatile_static_field instead. This carried over into some other areas like 
> the names of the GENERATE_VOLATILE_STATIC_VM_STRUCT_ENTRY and 
> CHECK_VOLATILE_STATIC_VM_STRUCT_ENTRY macros.

src/hotspot/share/runtime/perfMemory.cpp line 200:

> 198:   }
> 199: 
> 200:   Atomic::release_store(&_destroyed, 1);

As discussed in the final review thread for 
[JDK-8151815](https://bugs.openjdk.org/browse/JDK-8151815) a release_store is 
not actually needed here as there is no data that we access based on 
`_destroyed` being set to 1. Hence also no need for `load_acquire` in 
`is_destroyed()`.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15373#discussion_r1301124254

Reply via email to