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

[David Holmes]

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