RFR: 8300926: Several startup regressions ~6-70% in 21-b6 all platforms [v2]

[Daniel D. Daugherty]

On Mon, 20 Feb 2023 09:23:19 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> src/hotspot/share/code/codeCache.cpp line 1392:
>> 
>>> 1390:     if (nm->is_marked_for_deoptimization() && !nm->has_been_deoptimized() && nm->can_be_deoptimized()) {
>>> 1391:       nm->make_not_entrant();
>>> 1392:       nm->make_deoptimized();
>> 
>> `CodeCache::make_marked_nmethods_deoptimized()` used to call
>> `make_nmethod_deoptimized(nm)` which made a couple of checks
>> before it called `nm->make_deoptimized()`. `make_deoptimized()`
>> does not make these checks so why are they no longer needed?
>> 
>> Here's the removed code:
>> 
>> void CodeCache::make_nmethod_deoptimized(CompiledMethod* nm) {
>>   if (nm->is_marked_for_deoptimization() && nm->can_be_deoptimized()) {
>>     nm->make_deoptimized();
>>   }
>> }
>
> If the method can't be deoptimized we should not make "not entrant".
> If it is already deoptimize no need to try to make it "not entrant" once more.
> If it's not marked we shouldn't make it "not entrant".
> So only if we make it "not entrant" we should also make_deopt().
> 
> This is checked at the line just before:
> 
> if (nm->is_marked_for_deoptimization() && !nm->has_been_deoptimized() && nm->can_be_deoptimized()) {
>       nm->make_not_entrant();
>       nm->make_deoptimized();
> }
> 
> 
> So we do not need to recheck it.
> 
> But I'll add a check that we do not re-patch the nops.

Thanks!

>> src/hotspot/share/code/nmethod.cpp line 1166:
>> 
>>> 1164:     set_deoptimized_done();
>>> 1165:     return;
>>> 1166:   }
>> 
>> I don't understand this check-and-bailout logic at all. If continuations are
>> not enabled, then why does `nmethod::make_deoptimized()` do nothing?
>> This isn't an issue introduced by your patch, but I just happened to notice
>> when I was reading thru...
>
> We two deopts mechanisms:
> * Patching return PC in frames for nmethod that are no longer valid.
>    Done by handshaking all threads and walking their stack.
>    
> * Virtual threads stacks are only patched this way when mounted.
>    If we have a million VT we can't walk the entire heap and patch PC is all these stacks.
>    Instead there is nop-sledge after each calls from nmethods, so patch these to jump to the deopt
>    when returning into a deopted nmethod.
>    Calls into VM (causing the VT to pin) do not have these nop-sledges, so when using Loom, both mechanism is needed.

Thanks for explanation.

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

PR: https://git.openjdk.org/jdk/pull/12585