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

[Daniel D. Daugherty]

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)
>>
>>
>
>
>