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

[John Cuthbertson]

Hi Ramki,

Thanks for looking at the code changes and for the suggestions.

Extending the os API is a good idea. Not sure about the name though - 
how about: javaMonotonicTimeMillis? Also I believe os_solaris.cpp 
already has an internal monotonic version of javaTimeMillis() so there 
are alternatives to using javaTimeNanos() in the implementation.

Thanks for the heads up about the other CR. I'll check.

Thanks,

JohnC

On 12/11/11 01:32, Srinivas Ramakrishna wrote:
> Hi John -- Looks fine. Two minor comments:
>
> (1) by defining an os::javaTimeNanosInMillis() you may be able to 
> consolidate the divide by NANOS_IN_MILLISEC to one place instead of it 
> appearing at each use.
> (2) you might consolidate the commentary about monotonicity into 
> os::javaTimeNanosInMillis(), then at each client simly say "Protect 
> against possible nonmonotonicity"
>
> That will reduce the number of repeated lines while still providing 
> all of the commentary in all the relevant places. (By the way, you 
> might want to
> check if there;s another CR with the same content which might be 
> closed as a dup of this... just a very vague recollection.)
>
> reviewed.
> -- ramki (opendjk: ysr)
>
> On Fri, Dec 9, 2011 at 11:30 AM, John Cuthbertson 
> <john.cuthbertson at oracle.com <mailto:john.cuthbertson at oracle.com>> wrote:
>
>     Hi Everyone,
>
>     I have updated the comments based upon feedback from David Holmes.
>     A new webrev can be found at:
>     http://cr.openjdk.java.net/~johnc/7117303/webrev.1/
>     <http://cr.openjdk.java.net/%7Ejohnc/7117303/webrev.1/>
>
>     Thanks,
>
>     JohnC
>
>
>     On 12/7/2011 9:59 AM, John Cuthbertson wrote:
>
>         Hi Everyone,
>
>         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/
>         <http://cr.openjdk.java.net/%7Ejohnc/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. 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.
>
>         Testing: GC test suite on solaris and Linux, NSK tests on
>         solaris, and jprt.
>
>         Thanks,
>
>         JohnC
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20111212/d56512a2/attachment.htm>