RFR(S) Contended Locking fast enter bucket (8061553)

[David Holmes]

Hi Dan,

Looks good. Couple of nits and one semantic query below ...

src/cpu/sparc/vm/macroAssembler_sparc.cpp

Formatting changes were a bit of a distraction.

---

src/cpu/x86/vm/macroAssembler_x86.cpp

Formatting changes were a bit of a distraction.

1929     // unconditionally set stackBox->_displaced_header = 3
1930     movptr(Address(boxReg, 0), 
(int32_t)intptr_t(markOopDesc::unused_mark()));

At 1870 we refer to box rather than stackBox. Also it takes some 
sleuthing to realize that "3" here is somehow a pseudonym for 
unused_mark(). Back up at 1808 we have a to-do:

1808     //   use markOop::unused_mark() instead of "3".

so the current change seems to be implementing that, even though other 
uses of "3" are left untouched.

---

src/share/vm/runtime/sharedRuntime.cpp

1794 JRT_BLOCK_ENTRY(void, 
SharedRuntime::complete_monitor_locking_C(oopDesc* _obj, BasicLock* 
lock, JavaThread* thread))
1795   if (!SafepointSynchronize::is_synchronizing()) {
1796     if (ObjectSynchronizer::quick_enter(_obj, thread, lock)) return;

Is it necessary to check is_synchronizing? If we are executing this code 
we are not at a safepoint and the quick_enter wont change that, so I'm 
not sure what we are guarding against.

---

src/share/vm/runtime/synchronizer.cpp

Minor nit: line 153 the usual acronym is NPE (for NullPointerException) 
not NPX

Nit:  159     Thread * const ox

Please change ox to owner.

---

Thanks,
David



On 31/10/2014 8:50 AM, Daniel D. Daugherty wrote:
> Greetings,
>
> I have the Contended Locking fast enter bucket ready for review.
>
> The code changes in this bucket are primarily a quick_enter()
> function that works on inflated but uncontended Java monitors.
> This quick_enter() function is used on the "slow path" for Java
> Monitor enter operations when the built-in "fast path" (read
> assembly code) doesn't work.
>
> This work is being tracked by the following bug ID:
>
>      JDK-8061553 Contended Locking fast enter bucket
>      https://bugs.openjdk.java.net/browse/JDK-8061553
>
> Here is the webrev URL:
>
> http://cr.openjdk.java.net/~dcubed/8061553-webrev/0-jdk9-hs-rt/
>
> Here is the JEP link:
>
>      https://bugs.openjdk.java.net/browse/JDK-8046133
>
> 8061553 summary of changes:
>
> macroAssembler_sparc.cpp: MacroAssembler::compiler_lock_object()
>
> - clean up spacing around some
>    'ObjectMonitor::owner_offset_in_bytes() - 2' uses
> - remove optional (EmitSync & 64) code
> - change from cmp() to andcc() so icc.zf flag is set
>
> macroAssembler_x86.cpp: MacroAssembler::fast_lock()
>
> - remove optional (EmitSync & 2) code
> - rewrite LP64 inflated lock code that tries to CAS in
>    the new owner value to be more efficient
>
> interfaceSupport.hpp:
>
> - add JRT_BLOCK_NO_ASYNC to permit splitting a
>    JRT_BLOCK_ENTRY into two pieces.
>
> sharedRuntime.cpp: SharedRuntime::complete_monitor_locking_C()
>
> - change entry type from JRT_ENTRY_NO_ASYNC to JRT_BLOCK_ENTRY
>    to permit ObjectSynchronizer::quick_enter() call
> - add JRT_BLOCK_NO_ASYNC use if the quick_enter() doesn't work
>    to revert to JRT_ENTRY_NO_ASYNC-like semantics
>
> synchronizer.[ch]pp:
>
> - add ObjectSynchronizer::quick_enter() for entering an
>    inflated but unowned Java monitor without thread state
>    changes
>
> Testing:
>
> - Aurora Adhoc RT/SVC baseline batch
> - JPRT test jobs
> - MonitorEnterStresser micro-benchmark (in process)
> - CallTimerGrid stress testing (in process)
> - Aurora performance testing:
>    - out of the box for the "promotion" and 32-bit server configs
>    - heavy weight monitors for the "promotion" and 32-bit server configs
>      (-XX:-UseBiasedLocking -XX:+UseHeavyMonitors)
>      (in process)
>
>
> Thanks, in advance, for any comments, questions or suggestions.
>
> Dan