[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8300926: Several startup regressions ~6-70% in 21-b6 all platforms
From:       Erik =?UTF-8?B?w5ZzdGVybHVuZA==?= <eosterlund () openjdk ! org>
Date:       2023-02-16 16:07:30
Message-ID: XdD303NdaxoMXJJZ68-Zi4_YsZ32cnfxUrS2HcjrdW4=.7612be31-44da-46c0-8ff8-4b6869a5c3bb () github ! com
[Download RAW message or body]

On Thu, 16 Feb 2023 08:38:42 GMT, Robbin Ehn <rehn@openjdk.org> wrote:

> Hi all, please consider.
> 
> The original issue was when thread 1 asked to deopt nmethod set X and thread 2 \
> asked for the same or a subset of X. All method will already be marked, but the \
> actual deoptimizing, not entrant, patching PC on stacks and patching post call \
> nops, was not done yet. Which meant thread 2 could 'pass' thread 1. Most places did \
> deopt under Compile_lock, thus this is not an issue, but WB and \
> clearCallSiteContext do not. 
> Since a handshakes may take long before completion and Compile_lock is used for so \
> much more than deopts. The fix in https://bugs.openjdk.org/browse/JDK-8299074 \
> instead always emitted a handshake even when everything was already marked. \
> (instead of adding Compile_lock to all places) 
> This turnout to be problematic in the startup, for example the number of deopt \
> handshakes in jetty dry run startup went from 5 to 39 handshakes. 
> This fix first adds a barrier for which you do not pass until the requested deopts \
> have happened and it coalesces the handshakes. Secondly it moves handshakes part \
> out of the Compile_lock where it is possible. 
> Which means we fix the performance bug and we reduce the contention on \
> Compile_lock, meaning higher throughput in compiler and things such as \
> class-loading. 
> It passes t1-t7 with flying colours! t8 still not completed and I'm redoing some \
> testing due to last minute simplifications. 
> Thanks, Robbin

Changes requested by eosterlund (Reviewer).

src/hotspot/share/runtime/deoptimization.cpp line 110:

> 108:   MutexLocker ml(CompiledMethod_lock, Mutex::_no_safepoint_check_flag);
> 109:   // If there is nothing to deopt _gen is the same as comitted.
> 110:   _gen = DeoptimizationScope::_committed_deopt_gen;

Could we find a better name for the "_gen" member? If I'm reading this right, it's \
some kind of _required_gen, right? As in we need to have committed up until this \
number, or it isn't safe to continue.

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

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


[prev in list] [next in list] [prev in thread] [next in thread]