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

[Kim Barrett]

On Tue, 9 Apr 2024 16:19:50 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> That comment is a left-over from the old mechanism, and is no longer true.  This is the only place
>> that sets it false (and is holding Service_lock so there can't be concurrent setters to false), and knows
>> it is true (from the earlier test).  The old protocol with the safepoint cleanup trigger was more complex,
>> and this function didn't have the test of the flag.  We probably don't even need a "release" here.
>
> 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.)

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

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