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

Tue Jul 21 01:39:15 UTC 2015

Hi Dan,

On 21/07/2015 7:06 AM, Daniel D. Daugherty wrote:
> Greetings,
>
> In rebasing this bucket, I've run into the need for
> @HotSpotIntrinsicCandidate (8076112). That results
> in the following trivial change in the JDK repo:
>
> $ hg diff
> diff -r c2e706c73dd5 src/java.base/share/classes/java/lang/Object.java
> --- a/src/java.base/share/classes/java/lang/Object.java Fri Jul 17
> 08:46:54 2015 -0700
> +++ b/src/java.base/share/classes/java/lang/Object.java Mon Jul 20
> 13:59:10 2015 -0700
> @@ -278,6 +278,7 @@ public class Object {
>        * @see        java.lang.Object#notifyAll()
>        * @see        java.lang.Object#wait()
>        */
> +    @HotSpotIntrinsicCandidate
>       public final native void notify();
>
>       /**
> @@ -302,6 +303,7 @@ public class Object {
>        * @see        java.lang.Object#notify()
>        * @see        java.lang.Object#wait()
>        */
> +    @HotSpotIntrinsicCandidate
>       public final native void notifyAll();
>
>       /**

I'm regretting not having paid more attention to this annotation. I'm 
really not seeing it's value. And the word "Hotspot" should NOT be 
appearing as part of the Java platform API docs. :(

>
> and the following trivial test change in the HotSpot repo:
>
> diff -r 67c4a62090e5
> test/compiler/dependencies/MonomorphicObjectCall/java/lang/Object.java
> ---
> a/test/compiler/dependencies/MonomorphicObjectCall/java/lang/Object.java
> +++
> b/test/compiler/dependencies/MonomorphicObjectCall/java/lang/Object.java
> @@ -58,8 +58,10 @@ public class Object {
>           return getClass().getName() + "@" +
> Integer.toHexString(hashCode());
>       }
>
> +    @HotSpotIntrinsicCandidate
>       public final native void notify();
>
> +    @HotSpotIntrinsicCandidate
>       public final native void notifyAll();
>
>       public final native void wait(long timeout) throws
> InterruptedException;

Sorry to do this to you but as we already hit this with some other tests 
it may be better to disable CheckIntrinsics for the test. Otherwise the 
test can now only be compiled by a sufficiently recent JDK 9.

Thanks,
David

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