RFR: 8310513: [s390x] Intrinsify recursive ObjectMonitor locking
Lutz Schmidt
lucy at openjdk.org
Mon Apr 8 14:35:11 UTC 2024
On Fri, 23 Feb 2024 05:23:29 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:
> s390 implementation of [JDK-8277180](https://bugs.openjdk.org/browse/JDK-8277180). PPC implementation for the same: https://github.com/openjdk/jdk/pull/7305
>
> I had tested `tier1` on `fastdebug`, `release` vm.
>
> BenchMarking:
>
>
> ./build/linux-s390x-server-release/jdk/bin/java -Xms4g -Xmx4g -jar dacapo-9.12-MR1-bach.jar h2 -s huge -t 1 -n 1
>
> without patch:
> ===== DaCapo 9.12-MR1 h2 PASSED in 223023 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 225686 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 219824 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 226719 msec =====
>
>
>
> with patch:
> ===== DaCapo 9.12-MR1 h2 PASSED in 167816 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 174368 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 170517 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 169349 msec =====
Looks good overall.
My change requests have the intention to make identical actions look identical. That helps with understanding the code.
src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3199:
> 3197: NearLabel done, object_has_monitor;
> 3198:
> 3199: assert_different_registers(temp1, temp2);
If you want to make it fool-proof, assert that all four registers are distinct.
src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3302:
> 3300: Register temp = temp1;
> 3301:
> 3302: const int hdr_offset = oopDesc::mark_offset_in_bytes();
Either use this aux. variable in both, lock and unlock, methods or in none. I prefer using the aux. variable.
src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3304:
> 3302: const int hdr_offset = oopDesc::mark_offset_in_bytes();
> 3303:
> 3304: Label done, object_has_monitor, not_recursive;
Please insert the same register assert as in the lock method.
-------------
Changes requested by lucy (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17975#pullrequestreview-1986548916
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1555901183
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1555938529
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1555939449
More information about the hotspot-dev
mailing list