Hi David --<br><br><div class="gmail_quote">On Mon, Dec 12, 2011 at 9:28 PM, David Holmes <span dir="ltr"><<a href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 13/12/2011 6:06 AM, John Cuthbertson wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks for looking at the code changes and for the suggestions.<br>
<br>
Extending the os API is a good idea. Not sure about the name though -<br>
how about: javaMonotonicTimeMillis? Also I believe os_solaris.cpp<br>
already has an internal monotonic version of javaTimeMillis() so there<br>
are alternatives to using javaTimeNanos() in the implementation.<br>
</blockquote>
<br></div>
I'm somewhat opposed to extending the API simply because there are already too many time functions. What about a TO_MILLIS macro?<br>
<br>
#define TO_MILLIS(nanos) ((nanos)/NANOS_PER_MILLISEC<br>
<br>
jlong start = TO_MILLIS(javaTimeNanos());<br></blockquote><div><br>While the macro reduces the # characters, that was not the intent of my suggestion...<br> <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
BTW the "java" in javaTimeMillis and javaTimeNanos identifies these methods as being part of the Java API (System.currentTimeMillis and System.nanoTime respectively) - so javaMonotonicTimeMillis would not be appropriate.<br>
</blockquote><div><br>Then, how about os::monotonicTimeMillis() defined as an inline method in os:: ? One of the reasons for wanting the consolidation<br>into one place was also so that the commentary would be in one place, including the part about monotonicity only if and when the<br>
underlying platform guarantees it, which isn't true today on all the supported platforms.<br><br>Anyway, do whatever makes sense wrt proper naming and reducing duplication, clutter or confusion as the case may be...<br>
thanks.<br>-- ramki<br> <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
John: you have a recurrent typo in the comments: monton instead of monoton (and I mistyped it myself writing that!)<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks for the heads up about the other CR. I'll check.<br>
</blockquote>
<br></div>
See 4741166. It wanted to make javaTimeMillis monotonic, which it can not be. Pretty much all internal uses of javaTimeMillis (those not part of implementing System.currentTimeMillis()) should really be using a monotonic time source.<br>
<br>
David<br>
-----<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
Thanks,<br>
<br>
JohnC<br>
<br>
On 12/11/11 01:32, Srinivas Ramakrishna wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
Hi John -- Looks fine. Two minor comments:<br>
<br>
(1) by defining an os::javaTimeNanosInMillis() you may be able to<br>
consolidate the divide by NANOS_IN_MILLISEC to one place instead of it<br>
appearing at each use.<br>
(2) you might consolidate the commentary about monotonicity into<br>
os::javaTimeNanosInMillis(), then at each client simly say "Protect<br>
against possible nonmonotonicity"<br>
<br>
That will reduce the number of repeated lines while still providing<br>
all of the commentary in all the relevant places. (By the way, you<br>
might want to<br>
check if there;s another CR with the same content which might be<br>
closed as a dup of this... just a very vague recollection.)<br>
<br>
reviewed.<br>
-- ramki (opendjk: ysr)<br>
<br>
On Fri, Dec 9, 2011 at 11:30 AM, John Cuthbertson<br></div><div class="im">
<<a href="mailto:john.cuthbertson@oracle.com" target="_blank">john.cuthbertson@oracle.com</a> <mailto:<a href="mailto:john.cuthbertson@oracle.com" target="_blank">john.cuthbertson@<u></u>oracle.com</a>>> wrote:<br>
<br>
Hi Everyone,<br>
<br>
I have updated the comments based upon feedback from David Holmes.<br>
A new webrev can be found at:<br>
<a href="http://cr.openjdk.java.net/%7Ejohnc/7117303/webrev.1/" target="_blank">http://cr.openjdk.java.net/~<u></u>johnc/7117303/webrev.1/</a><br></div>
<<a href="http://cr.openjdk.java.net/%7Ejohnc/7117303/webrev.1/" target="_blank">http://cr.openjdk.java.net/%<u></u>7Ejohnc/7117303/webrev.1/</a>><div class="im"><br>
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
<br>
On 12/7/2011 9:59 AM, John Cuthbertson wrote:<br>
<br>
Hi Everyone,<br>
<br>
Can I have a couple of volunteers review the changes for this<br>
CR? The webrev can be found at:<br>
<a href="http://cr.openjdk.java.net/%7Ejohnc/7117303/webrev.0/" target="_blank">http://cr.openjdk.java.net/~<u></u>johnc/7117303/webrev.0/</a><br></div>
<<a href="http://cr.openjdk.java.net/%7Ejohnc/7117303/webrev.0/" target="_blank">http://cr.openjdk.java.net/%<u></u>7Ejohnc/7117303/webrev.0/</a>>.<div class="im"><br>
<br>
Summary:<br>
I replaced the calls to os::javaTimeMillis() in the GC where<br>
we expect monotonicity with calls os::javaTimeNanos(),<br>
converting the result to milliseconds. os::javaTimeNanos(), at<br>
least on some configurations, does guarantee monotonicity and<br>
so is a better alternative. The changes in the os_<*> files<br>
are to make use of the named conversion constants I<br>
added/moved to globalDefinitions.hpp - we seemed to have<br>
multiple names for the same two constants.<br>
<br>
Testing: GC test suite on solaris and Linux, NSK tests on<br>
solaris, and jprt.<br>
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
<br>
</div></blockquote>
<br>
</blockquote>
</blockquote></div><br>