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

[Coleen Phillimore]

On Tue, 9 Apr 2024 16:33:48 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> I thought the comment about "avoid dropping a concurrent request" was rather talking about the flag being concurrently set from false to true (in record_needs_cleanup), but then immediately getting cleared to false here due to the lack of atomics, leading to the cleanup request being essentially ignored and handled as done, even though nothing was done.
>
> If the flag was false at the preceding load-acquire then there wasn't a notification (yet) so we don't
> attempt to do any work this time around.  But there's always next (now at least periodic) time.  It was
> more complicated before the ServiceThread became periodic.  (Possibly overly complicated.  While
> discussing these changes with Coleen offline I had trouble understanding some of the old interaction
> and thought there was a much simpler way to accomplish what it was trying to do.  But making the
> ServiceThread periodic allows even more simplification.)

Here, if needs_cleanup_requested was true and then we set it to false, we still return true so a concurrent thread setting it to true is ok because we still return 'true'.  That is, we need to do some work.   If we return false here, while another thread is setting it to true, we'll get it on the next periodic timeout.
The only reason for the store-release is because we use load-acquire and it's consistent. The comment refers to the old behaviour which was more complicated.  I removed the comment to remove the confusion.

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

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