RFR: 8320318: ObjectMonitor Responsible thread

[Fredrik Bredberg]

On Thu, 12 Sep 2024 11:54:09 GMT, Martin Doerr <mdoerr 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.
>
> I've run it through our nightly testing (x86_64, aarch64, PPC64 with several OSes) and the good news is that I haven't seen any functional problems. Performance looks also good for the SPEC benchmarks. I don't think they stress Java monitors very strongly.
> 
> I've rerun the `LockUnlock` micro benchmark with this patch applied, but `LockUnlock.java` reverted to the original version. This makes `LockUnlock.testContendedLock` faster, but not as fast as without this patch (on the 96 Thread Xeon linux server, similar on Power10). Would be great if anybody could confirm.
> I think this should at least be documented and the description of the JBS issue improved.

@TheRealMDoerr 
> I've run it through our nightly testing (x86_64, aarch64, PPC64 with several OSes) and the good news is that I haven't seen any functional problems. Performance looks also good for the SPEC benchmarks. I don't think they stress Java monitors very strongly.

That really is good news! Thanks for testing!

> I've rerun the `LockUnlock` micro benchmark with this patch applied, but `LockUnlock.java` reverted to the original version. This makes `LockUnlock.testContendedLock` faster, but not as fast as without this patch (on the 96 Thread Xeon linux server, similar on Power10). Would be great if anybody could confirm. I think this should at least be documented and the description of the JBS issue improved.

Tanks for confirming that my suspension was right. As I stated earlier, due to the added StoreLoad barrier a slight decrease in performance is probably to be expected if you just run `LockUnlock.testContendedLock`, but it shouldn't really matter when running real life applications. Anyhow I'll update the description of the JBS issue.

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

PR Comment: https://git.openjdk.org/jdk/pull/19454#issuecomment-2346149800