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

Thu Jul 23 21:45:59 UTC 2015

I approve.  This is better.
Coleen

On 7/23/15 5:44 PM, Daniel D. Daugherty wrote:
> David H and I were IM'ing about adding options and it occurred to us
> that the new "InlineNotify" option is added as a "product" option and
> it should have been added as a "diagnostic" option. Diagnostic options
> are easier to remove in the future.
>
> JDK-8132286 change 'InlineNotify' flag option from "product" to 
> "diagnostic"
> https://bugs.openjdk.java.net/browse/JDK-8132286
>
> And here's the diff:
>
> $ hg diff
> diff -r 5ec83d7b8a2d src/share/vm/runtime/globals.hpp
> --- a/src/share/vm/runtime/globals.hpp  Thu Jul 23 16:36:36 2015 +0000
> +++ b/src/share/vm/runtime/globals.hpp  Thu Jul 23 14:36:23 2015 -0700
> @@ -1286,7 +1286,7 @@ public:
> \
>    experimental(intx, SyncVerbose, 0, 
> "(Unstable)")                          \
> \
> -  product(bool, InlineNotify, true, "intrinsify subset of notify" 
> )         \
> +  diagnostic(bool, InlineNotify, true, "intrinsify subset of 
> notify")       \
> \
>    experimental(intx, ClearFPUAtPark, 0, "(Unsafe, 
> Unstable)")               \
> \
>
> Under the new "trivial" change rules, may I have a single (R)eviewer
> for this change?
>
> Thanks, in advance, for any comments, questions or suggestions.
>
> Dan
>
> P.S.
> For completeness, I also filed:
>
> JDK-8132287 deprecate the "InlineNotify" flag option in JDK10
> https://bugs.openjdk.java.net/browse/JDK-8132287
>
> and set the "Fix Version/s" to '10'
>
>
> On 7/3/15 6: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)
>>>
>>>
>>
>>
>>
>