RFR(S) Contended Locking fast notify bucket (8075171)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Jul 21 04:24:35 UTC 2015
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...
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