RFR: 8329488: Move OopStorage code from safepoint cleanup and remove safepoint cleanup code [v3]

[Coleen Phillimore]

On Wed, 10 Apr 2024 12:59:31 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Ah, okay. Then this seems fine. Maybe tweak or remove the comment to reflect this conversation. It seems to be saying the opposite of what it is doing.
>
> The reason we have load_acquire is to match the release_store here. This can be running concurrently with the ServiceThread checking whether cleanup is needed.  This first flag says this OopStorage needs cleanup, the second is the global.  It seems like the order is important.  Therefore, we use all the load_acquire/release_store operations for safety and consistency, and to avoid future head scratching.
> 
> // Record that cleanup is needed, without notifying the Service thread, because
> // we can't lock the Service_lock.  Used by release().
> void OopStorage::record_needs_cleanup() {
>   // Set local flag first, else ServiceThread could wake up and miss
>   // the request.
>   Atomic::release_store(&_needs_cleanup, true);
>   Atomic::release_store_fence(&needs_cleanup_requested, true);
> }

I removed the comment that used to describe the CAS.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18375#discussion_r1559391874