RFR: 8320318: ObjectMonitor Responsible thread
Axel Boldt-Christmas
aboldtch at openjdk.org
Wed Sep 11 07:24:08 UTC 2024
On Tue, 10 Sep 2024 20:02:37 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> Removed the concept of an ObjectMonitor Responsible thread.
>>
>> The reason to have an ObjectMonitor Responsible thread was to avoid threads getting stranded due to a hole in the successor protocol. This hole was there because adding the necessary memory barrier was considered too expensive some 20 years ago.
>>
>> The ObjectMonitor Responsible thread code adds complexity, and doing timed parks just to avoid getting stranded is not the way forward. More info about the problems with the ObjectMonitor responsible thread can be found in [JDK-8320318](https://bugs.openjdk.org/browse/JDK-8320318).
>>
>> After removing the ObjectMonitor Responsible thread we see increased performance on all supported platforms except Windows. [JDK-8339730](https://bugs.openjdk.org/browse/JDK-8339730) has been created to handle this.
>>
>> Passes tier1-tier7 on supported platforms.
>> x64, AArch64, Riscv64, ppc64le and s390x passes ok on the test/micro/org/openjdk/bench/vm/lang/LockUnlock.java test.
>> Arm32 and Zero doesn't need any changes as far as I can tell.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 574:
>
>> 572: for (;;) {
>> 573: if (own == DEFLATER_MARKER) {
>> 574: if (TryLockI(current)) {
>
> I can't tell the difference between TryLockI and enter_for(). Did I previously object to enter_for() here? Maybe I should take that back, and there should be a comment above enter_for() like
> // Enters a lock in behalf of a non-current thread, or a thread that is exiting and has previously given up the lock.
> // and it handles deflation.
>
> You could add a boolean that you expect success for the enter_for() caller from deoptimization (ie. must_succeed).
>
> This code is getting repetitive - it looks the same in all these places only a little bit different and hard to know why.
I'd rather have a `TryLock_with_contention_mark` function which `enter_for` uses.
Have stronger asserts in `enter_for`, essentially that it is used correctly from Deopt and that the result of `TryLock_with_contention_mark` is true.
Suggestion:
// Block out deflation as soon as possible.
ObjectMonitorContentionMark contention_mark(this);
// Check for deflation.
if (enter_is_async_deflating()) {
// Treat deflation as interference
return TryLockResult::Interference;
}
if (TryLock_with_contention_mark(current, contention_mark)) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1753366689
More information about the hotspot-dev
mailing list