RFR(S) Contended Locking fast notify bucket (8075171)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu Jul 23 21:46:59 UTC 2015
Thanks!
Dan
On 7/23/15 3:45 PM, Coleen Phillimore wrote:
>
> 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)
>>>>
>>>>
>>>
>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list