RFR: 8322630: Remove ICStubs and related safepoints [v2]

[Erik Österlund]

On Thu, 25 Jan 2024 10:38:10 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> src/hotspot/share/code/nmethod.cpp line 1470:
>> 
>>> 1468: 
>>> 1469:   purge_ic_callsites();
>>> 1470: 
>> 
>> (Github does not allow me to attach this comment to the correct place):
>> At the start of this method, there is some comment about
>> 
>>     // Already unlinked. It can be invoked twice because concurrent code cache
>>     // unloading might need to restart when inline cache cleaning fails due to
>>     // running out of ICStubs, which can only be refilled at safepoints
>> 
>> This comment and the whole mechanism to prevent this may be outdated since there are no ICStubs and the associated safepoints any more; maybe it is worth keeping the flag to provide an assert though?
>> I did not check the code flow yet, just going from the comment.
>
> I think the flag is still required, just the comment needs to be fixed then.

Good point! I actually think we might not need it any more. I added the _is_unlinked field because of ICStubs causing inline cache cleaning from a concurrent GC to fail because going to the cleaned state requires an ICStub and we are out of memory for it, requiring the GC to request a safepoint and then restart the code cache unloading. If there is still a separate reason why we might call unlink twice on the same nmethod, I don't know why that is. Maybe something new in G1? Having said that, I don't mind keeping the guard to make it more robust. What do you think? Either way, I'll remove the inaccurate comment.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17495#discussion_r1466294600