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

[Kim Barrett]

On Tue, 9 Apr 2024 17:35:33 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Never mind. I misremembered how the work limiting operated. It's not a fixed
>> limit on how much work to do. Rather, it's (roughly) process at most the
>> number of blocks as there were in the list on entry. The point is that if
>> other threads are allocating and then emptying blocks while we're working,
>> that can't cause us to keep working for some potentially arbitrary amount of
>> time.
>> 
>> Also, the result indicating more work to do is unused by the ServiceThread.
>> It is used by the gtest for delete_empty_blocks, but I think will never be
>> true as used there.
>> 
>> We could change ServiceThread to pay attention to the result, but that would
>> make that code a bit more complicated, and because of how the "work limit"
>> works I don't think there's much benefit to that.  There would be if the "work
>> limit" were some arbitrary fixed count, like 10 blocks or something, but since
>> it's not...
>> 
>> So I think just further updating the comment is sufficient.  I think just keep
>> the first sentence ("Exceeded ... last block.") and delete the rest, about
>> making the ServiceThread loop.
>> 
>> I might file a new RFE to do something useful with that bool result or
>> eliminate it.
>
> But we still want to have record_needs_cleanup() right?  So that the ServiceThread will find work to do on the next iteration, but it's true that it wont *cause* the service thread to loop.

Yes, or at least not immediately loop, because of the deferral period.

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

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