RFR(S) Contended Locking fast notify bucket (8075171)

[Daniel D. Daugherty]

On 7/8/15 11:53 AM, Karen Kinnear wrote:
> Dan,
>
> Looks good.
>
> Only comment is -
>
> In ObjectMonitor::INotify - lines 1668-1671
> the default value of policy is "2"
> -- which basically is "b" not "a" unless the EntryList is empty.

Nice catch! For folks on the alias that don't have this code
tattooed on the inside of their eyelids. This is the relevant code:

src/share/vm/runtime/objectMonitor.cpp:

  136 static int Knob_MoveNotifyee        = 2;       // notify() - 
disposition of notifyee

<snip>

1659 void ObjectMonitor::INotify(Thread * Self) {
1660   const int policy = Knob_MoveNotifyee;

<snip>

1668     // Disposition - what might we do with iterator ?
1669     // a.  add it directly to the EntryList - either tail or head.
1670     // b.  push it onto the front of the _cxq.
1671     // For now we use (a).

<snip>

1710     } else if (policy == 2) {      // prepend to cxq

I will update the comment.

Dan


>
> thanks,
> Karen
>
> On Jul 3, 2015, at 8:11 PM, Daniel D. Daugherty wrote:
>
>> Greetings,
>>
>> I've updated the patch for Contended Locking fast notify bucket
>> based on David H's and Karen's comments.
>>
>> This work is being tracked by the following bug ID:
>>
>>     JDK-8075171 Contended Locking fast notify bucket
>>     https://bugs.openjdk.java.net/browse/JDK-8075171
>>
>> Here is the webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8075171-webrev/1-jdk9-hs-rt/
>>
>> Here is the JEP link:
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8046133
>>
>> Testing:
>>
>> - RBT vm.quick batches (in process)
>> - JPRT test jobs
>> - MonitorWaitNotifyStresser micro-benchmark (in process)
>> - CallTimerGrid stress testing (in process)
>>
>>
>> The changes in this round are in only four of the files:
>>
>> src/share/vm/opto/runtime.cpp
>>
>>   - deleted comment about hashCode() that belongs in a different bucket
>>
>> src/share/vm/runtime/objectMonitor.cpp
>>
>>   - add MAX_RECHECK_INTERVAL define for use in ObjectMonitor::EnterI()
>>   - cleanup recheck interval code
>>   - ObjectMonitor::INotify() return type is now "void"
>>   - delete duplicate comments, clarify/rewrite some comments
>>
>> src/share/vm/runtime/objectMonitor.hpp
>>
>>   - ObjectMonitor::INotify() return type is now "void"
>>
>> src/share/vm/runtime/synchronizer.cpp
>>
>>   - clarify/rewrite comments, add additional comments
>>   - cleanup variables in ObjectSynchronizer::quick_notify
>>   - refactor loop to be more clear
>>   - delete unused enum MaximumRecheckInterval
>>
>> The easiest way to review the delta is to download the two patch
>> files and compare them in something like jfilemerge:
>>
>> http://cr.openjdk.java.net/~dcubed/8075171-webrev/0-jdk9-hs-rt/hotspot.patch
>> http://cr.openjdk.java.net/~dcubed/8075171-webrev/1-jdk9-hs-rt/hotspot.patch
>>
>> Dan
>>
>>
>> On 5/18/15 2:07 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have the Contended Locking fast notify bucket ready for review.
>>>
>>> The code changes in this bucket are the final piece in the triad
>>> of {fast-enter, fast-exit, fast-notify} changes. There are no
>>> macro assembler changes in this bucket (yeah!), but there are
>>> code review cleanups motivated by comments on other buckets, e.g.,
>>> changing variables like 'Tally' -> 'tally'.
>>>
>>> This work is being tracked by the following bug ID:
>>>
>>>     JDK-8075171 Contended Locking fast notify bucket
>>>     https://bugs.openjdk.java.net/browse/JDK-8075171
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8075171-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
>>> - MonitorWaitNotifyStresser 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)
>>>
>>> The hang somehow introduced by the "fast enter" bucket is still
>>> unresolved, but progress is being made. That work is being tracked
>>> by the following bug:
>>>
>>>     JDK-8077392 Stream fork/join tasks occasionally fail to complete
>>>     https://bugs.openjdk.java.net/browse/JDK-8077392
>>>
>>> You'll see a change that re-enables the "fast enter" bucket in this
>>> webrev, but that change will not be pushed when we're done with the
>>> review of this bucket unless JDK-8077392 is resolved.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Gory details about the changes are below...
>>>
>>> Dan
>>>
>>>
>>> 8075171 summary of changes:
>>>
>>> src/share/vm/classfile/vmSymbols.hpp
>>>   - Add do_intrinsic() entries for _notify and _notifyAll
>>>
>>> src/share/vm/opto/library_call.cpp
>>>   - Add optional inline_notify() that is controlled by new
>>>     '-XX:[+-]InlineNotify' option
>>>
>>> src/share/vm/opto/runtime.cpp
>>> src/share/vm/opto/runtime.hpp
>>>   - Add OptoRuntime::monitor_notify_C() and
>>>     OptoRuntime::monitor_notifyAll_C() functions to support
>>>     the new "fast notify" code paths
>>>   - Add new OptoRuntime::monitor_notify_Type() to support
>>>     the above two new functions
>>>
>>> src/share/vm/runtime/globals.hpp
>>>   - Add '-XX:[+-]InlineNotify' option; default option value is
>>>     enabled (true)
>>>
>>> src/share/vm/runtime/objectMonitor.cpp
>>> src/share/vm/runtime/objectMonitor.hpp
>>>   - Refactor ObjectMonitor::notify() and ObjectMonitor::notifyAll()
>>>     into wrappers that call the new ObjectMonitor::INotify()
>>>   - Add new ObjectMonitor::INotify() to reduce code duplication
>>>     between "notify" and "notifyAll" code paths
>>>
>>> src/share/vm/runtime/sharedRuntime.cpp
>>>   - Temporarily re-enable the "fast enter" optimization in order
>>>     to do performance testing. JDK-8077392 is still unresolved,
>>>     but progress is being made.
>>>
>>> src/share/vm/runtime/synchronizer.cpp
>>> src/share/vm/runtime/synchronizer.hpp
>>>   - Add ObjectSynchronizer::quick_notify() that implements the
>>>     new "fast notify" code paths
>>>   - The new code handles a couple of special cases where the
>>>     "notify" can be done without the heavy weight slow-path
>>>     (thread state transitions and other baggage)
>>>
>>>