<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Staffan,<div><br></div><div>Looks good.</div><div><br></div><div>Thank you for separating out the issues to get the Mac OS X fixes in right away.</div><div><br></div><div>thanks,</div><div>Karen</div><div><br><div><div>On Apr 22, 2014, at 4:06 AM, Staffan Larsen wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">I have removed the AssumeMonotonicOSTime flag from this change set and updated the comments in the solaris code.<div><br></div><div>Newest webrev: <a href="http://cr.openjdk.java.net/~sla/8040140/webrev.04/">http://cr.openjdk.java.net/~sla/8040140/webrev.04/</a></div><div><br></div><div><div>Thanks,</div><div>/Staffan</div><div><br></div><div><br><div><div>On 18 apr 2014, at 16:43, Karen Kinnear <<a href="mailto:karen.kinnear@oracle.com">karen.kinnear@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Staffan,<div><br></div><div>thank you for your patience.</div><div><br></div><div>I am happy with the fix to Mac OS X in terms of adding the new code and the guarantee about</div><div>time not moving backward.</div><div><br></div><div>The cleanup in the Solaris code also looks good.</div><div><br></div><div>What I would request please is to split this back into two parts - I would like further discussion of the flag</div><div>in a separate thread, to allow the OS X changes to go back right away.</div><div><br></div><div>And for the comment in the Solaris code - the one thing we do know is that Solaris does not actually</div><div>guarantee that time will not go backward. They currently have issues running on virtual machines unless</div><div>bound to a specific cpu, and in  the past had issues with non-invariant TSC, which tells us that they do not have</div><div>logic with specific guarantees handling changes across cores. So if there is a way to phrase this - it is not</div><div>about Solaris having had bugs in the past, it is about Solaris not actually guaranteeing time not moving</div><div>backward so we have to do it.</div><div><br></div><div>thank you,</div><div>Karen</div><div><br></div><div><div><div>On Apr 15, 2014, at 8:27 AM, Staffan Larsen wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On 15 apr 2014, at 14:00, Karen Kinnear <<a href="mailto:karen.kinnear@oracle.com">karen.kinnear@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Staffan,<br><div><br></div><div>Have you heard back from the Solaris folks yet on how to tell that we are on a virtual machine, and</div><div>where they tell us  gethhrtime can move backward?</div></div></blockquote><div><br></div>I have not. It would be interesting to know this.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Is there any thought that in cases where we know there is a problem we might not want to</div><div>follow a script's flag recommendation?</div></div></blockquote><div><br></div><div>The default with my change is to guard against time moving backwards. If you want the “scary” behavior you have to say -XX:+UnlockExperimentalFeatures -XX:+AssumeMonotonicOSTimers”. </div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>I have been in too many situations in which scripts for starting java applications are inherited on</div><div>systems that are not appropriate, by folks who didn't write them and don't know the history</div><div>of the flag settings.</div><div><br></div><div>I am concerned about subtle unexpected behaviors here.</div><div><br></div><div>Since I haven't had time (yet - sorry) to walk through all the callers of getTimeNanos - could</div><div>you possibly let me know if there is a java call that ultimately uses this and if so, the impact</div><div>on java code of time moving backward?</div></div></blockquote><div><br></div><div>Well.. the most obvious case is System.nanoTime(). It must not move backwards. But with the current code (before my change) it can do so on OS X (!).</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Same for callers in the vm please? I believe Ramki fixed at least one of the garbage collectors</div><div>that had problems if time moved backward. Are the vm internal callers ok if time moves backwards?</div></div></blockquote><div><br></div><div>Again, There should be no moving backwards unless you run with experimental features.</div><div><br></div><div>Thanks,</div><div>/Staffan</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Or I can do that research when I get the time - and I can get back to you on that tomorrow - sorry,</div><div>another day of back-to-back meetings.</div><div><br></div><div>thanks,</div><div>Karen</div><div><br><div><div>On Apr 15, 2014, at 7:42 AM, Staffan Larsen wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On 15 apr 2014, at 11:38, David Holmes <<a href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On 15/04/2014 7:20 PM, Staffan Larsen wrote:<br><blockquote type="cite"><br>On 15 apr 2014, at 09:14, David Holmes <<a href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>> wrote:<br><br><blockquote type="cite">Hi Staffan,<br><br>Generally looks okay.<br><br>os_bsd.cpp still shows the old URL for Dave Dice's article<br></blockquote><br>I had forgotten to save the file. :-(<br><br><blockquote type="cite">os_solaris.cpp:<br><br>In the Solaris changes there is a lot of old code with inaccurate comments, but I suppose cleaning that up (oldgetTimeNanos()) is out of scope. You only added the check for AssumeMonotonicOSTimers in the supports_cx8 path, but the other path is now dead code.<br></blockquote><br>Since supports_cx8() returns true (because SUPPORTS_NATIVE_CX8 is defined) on both solaris sparc and solaris x86, it looks like oldgetTimeNanos() is really dead code. I can remove it as part of this change if that’s ok.<br></blockquote><br>I'm in favour of removing dead code. :)<br></div></blockquote><div><br></div><div>Gone!</div><br><blockquote type="cite"><div style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite"><blockquote type="cite">globals.hpp<br><br>Do we need to document this only affects OSX and Solaris? (Though implicitly this acts as-if true on Linux and Windows in the common case.)<br></blockquote><br>I will update the description to:<br><br>  experimental(bool, AssumeMonotonicOSTimers, false,                        \<br>          "Assume that the OS system timers are monotonic "                 \<br>          "(Solaris and OS X)")                                             \<br></blockquote><br>Sounds good. Will you close 6864866 as a duplicate of this one?<br></div></blockquote><div><br></div><div>I will include 6864866 in the hg commit message closing both with the same fix.</div><br><blockquote type="cite"><div style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite"><blockquote type="cite">os.hpp:<br><br> #ifdef TARGET_OS_FAMILY_bsd<br> # include "jvm_bsd.h"<br> # include <setjmp.h><br>+ # include <mach/mach_time.h><br> #endif<br><br>I think this include needs to be in a OSX/Apple specific conditional.<br></blockquote><br>Changed it to:<br><br>#ifdef TARGET_OS_FAMILY_bsd<br># include "jvm_bsd.h"<br># include <setjmp.h><br># ifdef __APPLE__<br>#  include <mach/mach_time.h><br># endif<br></blockquote><br>Ok. I wonder if someone could test this on a non-Apple BSD system? ;-)</div></blockquote><div><br></div><div>Updated review here: <a href="http://cr.openjdk.java.net/~sla/8040140/webrev.02/">http://cr.openjdk.java.net/~sla/8040140/webrev.02/</a></div><div><br></div><div>Thanks,</div><div>/Staffan</div><br><blockquote type="cite"><div style="font-size: 16px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>Thanks,<br>David<br><br><blockquote type="cite"><br>Thanks,<br>/Staffan<br><br><blockquote type="cite"><br>--<br><br>We should really fix the non-monotonic-clock path in the Linux and Windows implementations too ... but 32-bit is problematic <sigh><br><br>Thanks,<br>David<br><br>On 15/04/2014 4:00 PM, Staffan Larsen wrote:<br><blockquote type="cite">Here is an updated webrev with changes to the comments in os_bsd.cpp and<br>os_solaris.cpp.<br> - obs -> obsv<br> - fixed URL to blog entry<br><br><a href="http://cr.openjdk.java.net/~sla/8040140/webrev.01/">http://cr.openjdk.java.net/~sla/8040140/webrev.01/</a><br><br>/Staffan<br><br>On 15 apr 2014, at 07:52, Staffan Larsen <<a href="mailto:staffan.larsen@oracle.com">staffan.larsen@oracle.com</a><br><<a href="mailto:staffan.larsen@oracle.com">mailto:staffan.larsen@oracle.com</a>>> wrote:<br><br><blockquote type="cite"><br>On 14 apr 2014, at 21:08, Aleksey Shipilev<br><<a href="mailto:aleksey.shipilev@oracle.com">aleksey.shipilev@oracle.com</a> <<a href="mailto:aleksey.shipilev@oracle.com">mailto:aleksey.shipilev@oracle.com</a>>> wrote:<br><br><blockquote type="cite">On 04/14/2014 06:55 PM, Staffan Larsen wrote:<br><blockquote type="cite">mach_absolute_time() is essentially a direct call to RDTSC, but with<br>conversion factor to offset for any system sleeps and frequency<br>changes. The call returns something that can be converted to<br>nanoseconds using information from mach_timebase_info(). Calls to<br>mach_absolute_time() do not enter the kernel and are very fast. The<br>resulting time has nanosecond precision and as good accuracy as one<br>can get.<br></blockquote><br>Some numbers would be good on the public list :) I know the numbers<br>already, but others on this list don’t.<br></blockquote><br>I posted the numbers in the bug, but forgot to say so here...<br><br><blockquote type="cite"><br><blockquote type="cite">Since the value from RDTSC can be subject to drifting between CPUs,<br>we implement safeguards for this to make sure we never return a lower<br>value than the previous values. This adds some overhead to nanoTime()<br>but guards us against possible bugs in the OS. For users who are<br>willing to trust the OS and need the fastest possible calls to<br>System.nanoTime(), we add a flag to disable this safeguard:<br>-XX:+AssumeMonotonicOSTimers.<br></blockquote><br>I now wonder if this safeguard can produce a stream of exactly the same<br>timestamps if local clock is lagging behind. But considering the<br>alternative of answering the retrograde time, and the observation the<br>current Mac OS X mach_absolute_time() *appears* monotonic, having this<br>safeguard seems OK.<br><br><blockquote type="cite">webrev: <a href="http://cr.openjdk.java.net/~sla/8040140/webrev.00/">http://cr.openjdk.java.net/~sla/8040140/webrev.00/</a><br>bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8040140">https://bugs.openjdk.java.net/browse/JDK-8040140</a><br></blockquote><br>This looks good to me.<br></blockquote><br>Thanks.<br><br><blockquote type="cite">And, since this question will inevitably pop up, do we plan to bring it<br>into 8uX? I think many Mac users will be happy about that.<br></blockquote><br>I would like to do so, but I would also like to have it sit and bake<br>for a while in 9 before that. I think the 8u20 train has left the<br>station, but perhaps 8u40?<br><br>/Staffan</blockquote></blockquote></blockquote></blockquote></div></blockquote></div><br></div></blockquote></div><br></div></div></blockquote></div><br></div></blockquote></div><br></div></div></blockquote></div><br></div></div></div></blockquote></div><br></div></body></html>