[9] RFR (XS) 8044339: wrong comment for FilterSpuriousWakeups option

[David Holmes]

Looks good to me!

You'll need a second reviewer and a sponsor.

Thanks,
David

On 2/06/2014 7:22 PM, Lev Priima wrote:
> Thanks, please review update:
> http://cr.openjdk.java.net/~iignatyev/lpriima/8044339/webrev.02/
>
> Lev
>
> On 06/02/2014 07:43 AM, David Holmes wrote:
>> Hi Lev,
>>
>> On 31/05/2014 1:57 AM, Lev Priima wrote:
>>> Thanks for comments, please look at update:
>>> http://cr.openjdk.java.net/~iignatyev/lpriima/8044339/webrev.01/
>>
>> Please generalize the CR summary and description if you are going to
>> pursue all these changes. But include additional information to
>> clarify why you made the changes in the os_XXX.cpp files please. On
>> that note can you please change:
>>
>> !   // We must filter out spurious wakeups.
>>
>> to
>>
>> !   // We ignore spurious OS wakeups unless FilterSpuriousWakeups is
>> false
>>
>> In globals.hpp: Note that CompilerThreadHintNoPreempt and
>> VMThreadHintNoPreempt are actually only meaningful on Solaris. They
>> guard the call to os::hint_no_preempt which is a no-op except on
>> Solaris. So those changes should be reverted.
>>
>> Thanks,
>> David
>>
>>> Lev
>>>
>>> On 05/30/2014 10:03 AM, David Holmes wrote:
>>>> Hi Lev,
>>>>
>>>> On 30/05/2014 4:33 AM, Lev Priima wrote:
>>>>> Please review and help me with integration:
>>>>>
>>>>> diff -r 102506d9d873 src/share/vm/runtime/globals.hpp
>>>>> --- a/src/share/vm/runtime/globals.hpp    Wed May 28 14:42:00 2014
>>>>> +0400
>>>>> +++ b/src/share/vm/runtime/globals.hpp    Thu May 29 22:29:37 2014
>>>>> +0400
>>>>> @@ -1144,7 +1144,7 @@
>>>>> \
>>>>>     product(bool, FilterSpuriousWakeups,
>>>>> true,                                \
>>>>>             "Prevent spurious or premature wakeups from object.wait
>>>>> "         \
>>>>> -          "(Solaris
>>>>> only)")                                                 \
>>>>> +          "(Ignored for
>>>>> Windows)")                                          \
>>>>> \
>>>>
>>>> Sorry to make life complicated but as I put in the bug report:
>>>>
>>>> The "Solaris" part dates from the days when Solaris meant "not
>>>> Windows". :)
>>>>
>>>> A bigger problem here is that the main part of the description:
>>>>
>>>> "Prevent spurious or premature wakeups from object.wait "
>>>>
>>>> may not match what people expect - it depends on whether "filter"
>>>> means to remove the item being filtered, or to keep only the item
>>>> being filtered. It would clearer if it read "When true prevents ..."
>>>> but even then it is not accurate as this only excludes spurious
>>>> wakeups from the OS pthread_cond[timed]wait implementation - the JVM
>>>> can still induce its own "spurious wakeups". So the better description
>>>> would be:
>>>>
>>>> "When true prevents OS-level spurious, or premature, wakeups from
>>>> Object.wait"
>>>>
>>>> But it should also be noted that where this flag is checked there is
>>>> an even more inaccurate comment:
>>>>
>>>>   // The underlying Solaris implementation, cond_timedwait, admits
>>>>   // spurious/premature wakeups, but the JLS/JVM spec prevents the
>>>>   // JVM from making those visible to Java code.
>>>>
>>>> Ironically this comment appears in the linux and bsd versions but not
>>>> the Solaris one! Further it is incorrect - the JLS/JVMS does not
>>>> prevent spurious wakeups from being made visible to Java code. That
>>>> has always been the intent but it took until Java 5 before we got the
>>>> specification for Object.wait updated to make it clear.
>>>>
>>>> So really this is all legacy crud that should be removed along with
>>>> the other planned cleanups (ie removing the safe_cond_* functions and
>>>> WorkAroundNPTLTimedWaitHang) and the os_posix refactoring.
>>>>
>>>> ---
>>>>
>>>> Sorry to mess up your "starter" bug.
>>>>
>>>> David
>>>>
>>>>
>>>>>     product(intx, NativeMonitorTimeout, -1,
>>>>> "(Unstable)")                     \
>>>>> \
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8044339
>>>>>
>>>
>