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

[Karen Kinnear]

Dan,

The code looks really good. Thank you for the detailed description and the
extensive testing.

Two minor comments:
1. objectMonitor.cpp ~ 1706 - after the else if (policy == 2)
- could you possibly move the iterator->TState = ObjectWaiter::TS_CXQ to before the if (List == NULL)/else?
   - otherwise I think it is only set on the else

2. opto.runtime.cpp
lines 433-435 -- did you want the comment about intrinsifying hashCode()? My memory is we aren't picking up
those changes

thanks,
Karen


On May 18, 2015, at 4: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)