RFR (S): 7117303: VM uses non-monotonic time source and complains that it is non-monotonic

[John Cuthbertson]

Hi David,

Thanks for the code review comments - I'll change the comments to what 
you suggest and send out a new webrev.

JohnC

On 12/07/11 21:48, David Holmes wrote:
> Hi John,
>
> On 8/12/2011 3:59 AM, John Cuthbertson wrote:
>> Can I have a couple of volunteers review the changes for this CR? The
>> webrev can be found at:
>> http://cr.openjdk.java.net/~johnc/7117303/webrev.0/.
>>
>> Summary:
>> I replaced the calls to os::javaTimeMillis() in the GC where we expect
>> monotonicity with calls os::javaTimeNanos(), converting the result to
>> milliseconds. os::javaTimeNanos(), at least on some configurations, does
>> guarantee monotonicity and so is a better alternative.
>
> javaTimeNanos is "guaranteed" monotonic if the underlying platform 
> provides a monotonic timesource.
>
> I think this comment:
>
> !   // os::javaTimeMillis() does not guarantee monotonicity.
>
> might be better expressed as:
>
>     // need to use a monotonic time source
>
> or perhaps
>
>    // need a monotonic time in ms but os::javaTimeMillis() does not 
> guarantee monotonicity
>
>> The changes in
>> the os_<*> files are to make use of the named conversion constants I
>> added/moved to globalDefinitions.hpp - we seemed to have multiple names
>> for the same two constants.
>
> Yeah that little cleanup has been on the to-do list for quite a while 
> - thanks.
>
> genCollectedHeap.cpp:
>
> This comment block can be shortened from:
>
> !   // XXX Since javaTimeNanos() mostly guarantees monotonically
> !   // increasing return values, depending upon the underlying
> !   // os-dependent implementation, we still need to guard against
> !   // getting back a time later than now. This should be fixed
> !   // by basing the implementation on something like gethrtime()
> !   // which does guarantee monotonicity.
> !   // Note that cond_wait() is susceptible to a similar problem,
> !   // because its interface is based on absolute time in the form
> !   // of the system time's notion of UCT. See also 4506635 for yet
> !   // another problem of similar nature. XXX
>
> to just
>
>     // javaTimeNanos() is guaranteed to be monotonic non-decreasing
>     // provided the underlying platform provides such a time source
>     // (and it is bug free). So we still have to guard against getting
>     // back a time later than 'now'.
>
> javaTimeNanos _is_ based on gethrtime (on Solaris which is the only 
> place it exists) or on CLOCK_MONOTONIC on Linux (if present). It is as 
> monotonic as the platform provides. The ref to cond_wait is just 
> completely out of place.
>
> David
> -----
>
>> Testing: GC test suite on solaris and Linux, NSK tests on solaris, and
>> jprt.
>>
>> Thanks,
>>
>> JohnC