RFR(S) Contended Locking fast notify bucket (8075171)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Jul 21 11:53:24 UTC 2015
On 7/20/15 10:33 PM, David Holmes wrote:
> On 21/07/2015 2:24 PM, Daniel D. Daugherty wrote:
>>
>> On 7/20/15 7:39 PM, David Holmes wrote:
>>> 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. :(
>>
>> Yup. It does feel strange, but I had to add it...
>>
>>
>>>
>>>>
>>>> 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.
>>
>> Since I know nothing about this test other than 8076112 modified
>> it for other intrinsics, I'd rather not muddy the waters of this
>> changeset dealing with whether this test should use
>> -XX:-CheckIntrinsics or not...
>
> Hadn't realized that this test was already using the annotation - in
> that case no problem and no need for follow up.
Thanks!
Dan
>
> Thanks,
> David
>
>> I will file a follow up bug and try to chase down resolution
>> for that test using a different bug.
>>
>> Dan
>>
>>>
>>> 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)
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>
More information about the hotspot-runtime-dev
mailing list