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

Dean Long dlong at openjdk.org
Thu Feb 8 09:21:00 UTC 2024


On Tue, 30 Jan 2024 09:08:01 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.
>
> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
> 
>   ARM32 fixes

src/hotspot/cpu/aarch64/aarch64.ad line 2224:

> 2222:   // This is the unverified entry point.
> 2223:   C2_MacroAssembler _masm(&cbuf);
> 2224:   __ ic_check(CodeEntryAlignment);

I'm not sure we want to increase the alignement to CodeEntryAlignment here.  I believe C2 already aligns the root block to CodeEntryAlignment.  @theRealAph, what do you think?

src/hotspot/share/opto/output.cpp line 3416:

> 3414:     } else {
> 3415:       if (!target->is_static()) {
> 3416:         _code_offsets.set_value(CodeOffsets::Entry, _first_block_size - MacroAssembler::ic_check_size());

This looks tricky. I think it means CodeOffsets::Entry starts after the alignment padding NOPs. If that's true then the `ic_check` functions could use a comment explaining that alignment needs to come first, not last. A comment here wouldn't hurt either.

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

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


More information about the shenandoah-dev mailing list