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