RFR: 8336421: ciMethod() constructor should use ConditionalMutexLocker(Compile_lock)

Vladimir Kozlov kvn at openjdk.org
Tue Jul 16 16:13:57 UTC 2024


On Tue, 16 Jul 2024 09:56:32 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> During Leyden testing with `-XX:+LogCompilation` flag I hit assert because `ciMethod()` could be called from `ciEnv::register_method()` which already holds `Compile_lock` lock. See call stack in JBS report.
>> Use `ConditionalMutexLocker` in `ciMethod()`.
>> This is normal code pattern used in other places where we can have nested locking: [compileBroker.cpp#L2847](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/compileBroker.cpp#L2847).
>> 
>> Tested tier1.
>
> src/hotspot/share/ci/ciMethod.cpp line 113:
> 
>> 111:     // 6328518 check hotswap conditions under the right lock.
>> 112:     bool should_take_Compile_lock = !Compile_lock->owned_by_self();
>> 113:     ConditionalMutexLocker locker(Compile_lock, should_take_Compile_lock, Mutex::_safepoint_check_flag);
> 
> I see no point in making an explicit variable. Most of the uses of `CML` just inline it:
> https://github.com/openjdk/jdk/blob/8feabc849ba2f617c8c6dbb2ec5074297beb6437/src/hotspot/share/runtime/deoptimization.cpp#L122

Shorter line length - fit terminal window ;^)
Also it more clear what this parameter for.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20187#discussion_r1679693432


More information about the hotspot-compiler-dev mailing list