RFR: 8322630: Remove ICStubs and related safepoints
Martin Doerr
mdoerr at openjdk.org
Thu Jan 25 06:46:29 UTC 2024
On Fri, 19 Jan 2024 06:25:20 GMT, Erik Österlund <eosterlund 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.
On linux, the time for "Purge Unlinked NMethods" goes down when I comment out `delete ic->data();` and ignore the memory leak. (MacOS seems to be ok with it.)
Adding trace code to `purge_ic_callsites` shows that we often have 0 or 2 ICData instances, sometimes up to 30 ones.
It would be good to think a bit about the allocation scheme. Some ideas would be
- Allocate ICData in an array per nmethod instead of individually. That should help to some degree and also improve data locality (and hence cache efficiency). Would also save iterating over the relocations. It's not very complex.
- Instead of freeing ICData instances, we could enqueue them and either reuse or free them during a concurrent phase. This may be a bit complicated. Not sure if it's worth it.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17495#issuecomment-1909448311
More information about the shenandoah-dev
mailing list