RFR(S) Contended Locking fast exit bucket (8073165)

[Daniel D. Daugherty]

Greetings,

I have the revised Contended Locking fast exit bucket ready for review.

This work is being tracked by the following bug ID:

     JDK-8073165 Contended Locking fast exit bucket
     https://bugs.openjdk.java.net/browse/JDK-8073165

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8073165-webrev/1-jdk9-hs-rt/

Here is the JEP link:

     https://bugs.openjdk.java.net/browse/JDK-8046133

Thanks to David H, Aleksey S and Coleen for their comments!

The best way to review this round is to download the two patch files:

Summary of changes:    284 lines changed: 163 ins; 51 del; 70 mod; 34386 
unchg
http://cr.openjdk.java.net/~dcubed/8073165-webrev/0-jdk9-hs-rt/hotspot.patch

Summary of changes:    356 lines changed: 166 ins; 55 del; 135 mod; 
32524 unchg
http://cr.openjdk.java.net/~dcubed/8073165-webrev/1-jdk9-hs-rt/hotspot.patch

and view them in your favorite file merge tool.

Here are the changes from Code Review Round 0:

- The changes to src/share/vm/runtime/thread.hpp have been
   backed out (no longer adding a _TSelf field); removed the
   (EmitSync & 32) optimization that used the _TSelf field
- merge the SPARC and X86 comments for the (EmitSync & 1024)
   optimization and apply to both
- remove the FenceInstruction option; still need to file a
   fast-track CCC request
- cleaned up various comments
- cleaned up variable naming in synchronizer.[ch]pp
   - global variables now consistently begin with a 'g'
     like gCamelCaseGlobalVariable
   - locals like 'inusetally' are renamed to 'in_use_tally'
   - made functions with a mix of lowercase and camelCase
     parameters consistently camelCase
- added and/or made comments consistent in synchronizer.[ch]pp
- made phrase "in-use" consistent in comments and asserts

Dan


On 3/9/15 11:11 AM, Daniel D. Daugherty wrote:
> Greetings,
>
> I have the Contended Locking fast exit bucket ready for review.
>
> The code changes in this bucket are primarily assembly language
> tweaks that come into play when an inflated ObjectMonitor is in
> use. There are tweaks that apply to the default code path and
> there are tweaks that only apply when an experimental flag is
> set. This code code review is focused on the default code path
> or the "out of the box" configuration.
>
> This work is being tracked by the following bug ID:
>
>     JDK-8073165 Contended Locking fast exit bucket
>     https://bugs.openjdk.java.net/browse/JDK-8073165
>
> Here is the webrev URL:
>
> http://cr.openjdk.java.net/~dcubed/8073165-webrev/0-jdk9-hs-rt/
>
> Here is the JEP link:
>
>     https://bugs.openjdk.java.net/browse/JDK-8046133
>
> Testing:
>
> - Aurora Adhoc RT/SVC baseline batch
> - JPRT test jobs
> - MonitorExitStresser micro-benchmark (in process)
> - MonitorEnterExitStresser 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)
>
>
> 8073165 summary of changes:
>
> macroAssembler_sparc.cpp: MacroAssembler::compiler_unlock_object()
>
> - default (OOTB) EmitSync value == 0
> - an _owner field optimization is in the non-default path under
>   a (EmitSync & 1024) check
>
> - classic lock release without 1-0 support is in the non-default
>   path under a (EmitSync & 512) check
>
> - an EntryList, cxq and _succ field optimization is in the
>   non-default path under a (EmitSync & 16384) check
>
> - the default path is now the 1-0 form that avoids CAS and membar
>   in the common case; see the "1-0 exit" section in objectMonitor.cpp
>
> sharedRuntime_sparc.cpp:
>
> - add JavaThread* param for
>   SharedRuntime::complete_monitor_unlocking_C call
>
> macroAssembler_x86.cpp: MacroAssembler::fast_unlock()
>
> - with all the new/modified comments and all the optional
>   code path updates, it is hard to see that the only real
>   changes to the default code path are 1) the _owner==Self
>   check has been made optional and 2) the optional succ
>   field check for the slow path is now on the default path
>
> - default (OOTB) EmitSync value == 0
> - remove optional (EmitSync & 8) code
> - refactor code in MacroAssembler::fast_unlock
> - remove old mfence() support
> - an _owner field optimization is in the non-default path under
>   a (EmitSync & 1024) check
> - an _owner field optimization is in the non-default path under
>   a (EmitSync & 16) check
> - a barrier optimization is in the non-default path under a
>   (EmitSync & 32) check
> - remove old AMD optional work around
>
> sharedRuntime_x86_32.cpp:
> sharedRuntime_x86_64.cpp:
>
> - add thread param for SharedRuntime::complete_monitor_unlocking_C
>   call
>
> macro.[ch]pp: PhaseMacroExpand::make_slow_call()
>
> - add param2 and update callers
>
> runtime.[ch]pp: OptoRuntime::complete_monitor_exit_Type()
>
> - add JavaThread* parameter
>
> sharedRuntime.[ch]pp: SharedRuntime::complete_monitor_unlocking_C()
>
> - add JavaThread* parameter
>
> synchronizer.cpp: ObjectSynchronizer::omRelease()
>
> - cleanup in use list traversal
> - add assert that ObjectMonitor being released was
>   actually found and extracted
>
> synchronizer.[ch]pp:
>
> - add some comments in various places
> - rename walk_monitor_list() -> deflate_monitor_list()
>   and update callers
> - minor logic refactoring
>
> thread.hpp:
>
> - add _TSelf field for mfence() replacement
>
>
> Thanks, in advance, for any comments, questions or suggestions.
>
> Dan
>
>