RFR: 8322630: Remove ICStubs and related safepoints

[Thomas Schatzl]

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

>> ICStubs solve an atomicity problem when setting both the destination and data of an inline cache. Unfortunately, it also leads to occasional safepoint carpets when multiple threads need to ICRefill the stubs at the same time, and spurious GuaranteedSafepointInterval "Cleanup" safepoints every second. This patch changes inline caches to not change the data part at all during the nmethod life cycle, hence removing the need for ICStubs.
>> 
>> The new scheme is less stateful. Instead of adding and removing callsite metadata back and forth when transitioning inline cache states, it installs all state any shape of call will ever need at resolution time in a struct that I call CompiledICData. This reduces inline cache state changes to simply changing the destination of the call, and it doesn't really matter what state transitions to what other state.
>> 
>> With this patch, we get rid of ICStub and ICBuffer classes and the related ICRefill and almost all Cleanup safepoints in practice. It also makes the inline cache code much simpler.
>> 
>> I have tested the changes from tier1-7, and run through full aurora performance tests.
>
> 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.

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

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