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

[Lev Priima]

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