RFR: 8040140 System.nanoTime() is slow and non-monotonic on OS X

[Staffan Larsen]

On 15 apr 2014, at 11:38, David Holmes <david.holmes at oracle.com> wrote:

> On 15/04/2014 7:20 PM, Staffan Larsen wrote:
>> 
>> On 15 apr 2014, at 09:14, David Holmes <david.holmes at oracle.com> wrote:
>> 
>>> Hi Staffan,
>>> 
>>> Generally looks okay.
>>> 
>>> os_bsd.cpp still shows the old URL for Dave Dice's article
>> 
>> I had forgotten to save the file. :-(
>> 
>>> os_solaris.cpp:
>>> 
>>> 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.
>> 
>> 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.
> 
> I'm in favour of removing dead code. :)

Gone!

>>> globals.hpp
>>> 
>>> 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.)
>> 
>> I will update the description to:
>> 
>>   experimental(bool, AssumeMonotonicOSTimers, false,                        \
>>           "Assume that the OS system timers are monotonic "                 \
>>           "(Solaris and OS X)")                                             \
> 
> Sounds good. Will you close 6864866 as a duplicate of this one?

I will include 6864866 in the hg commit message closing both with the same fix.

> 
>>> os.hpp:
>>> 
>>>  #ifdef TARGET_OS_FAMILY_bsd
>>>  # include "jvm_bsd.h"
>>>  # include <setjmp.h>
>>> + # include <mach/mach_time.h>
>>>  #endif
>>> 
>>> I think this include needs to be in a OSX/Apple specific conditional.
>> 
>> Changed it to:
>> 
>> #ifdef TARGET_OS_FAMILY_bsd
>> # include "jvm_bsd.h"
>> # include <setjmp.h>
>> # ifdef __APPLE__
>> #  include <mach/mach_time.h>
>> # endif
> 
> Ok. I wonder if someone could test this on a non-Apple BSD system? ;-)

Updated review here: http://cr.openjdk.java.net/~sla/8040140/webrev.02/

Thanks,
/Staffan

> 
> Thanks,
> David
> 
>> 
>> Thanks,
>> /Staffan
>> 
>>> 
>>> --
>>> 
>>> We should really fix the non-monotonic-clock path in the Linux and Windows implementations too ... but 32-bit is problematic <sigh>
>>> 
>>> Thanks,
>>> David
>>> 
>>> On 15/04/2014 4:00 PM, Staffan Larsen wrote:
>>>> Here is an updated webrev with changes to the comments in os_bsd.cpp and
>>>> os_solaris.cpp.
>>>>  - obs -> obsv
>>>>  - fixed URL to blog entry
>>>> 
>>>> http://cr.openjdk.java.net/~sla/8040140/webrev.01/
>>>> 
>>>> /Staffan
>>>> 
>>>> On 15 apr 2014, at 07:52, Staffan Larsen <staffan.larsen at oracle.com
>>>> <mailto:staffan.larsen at oracle.com>> wrote:
>>>> 
>>>>> 
>>>>> On 14 apr 2014, at 21:08, Aleksey Shipilev
>>>>> <aleksey.shipilev at oracle.com <mailto:aleksey.shipilev at oracle.com>> wrote:
>>>>> 
>>>>>> On 04/14/2014 06:55 PM, Staffan Larsen wrote:
>>>>>>> mach_absolute_time() is essentially a direct call to RDTSC, but with
>>>>>>> conversion factor to offset for any system sleeps and frequency
>>>>>>> changes. The call returns something that can be converted to
>>>>>>> nanoseconds using information from mach_timebase_info(). Calls to
>>>>>>> mach_absolute_time() do not enter the kernel and are very fast. The
>>>>>>> resulting time has nanosecond precision and as good accuracy as one
>>>>>>> can get.
>>>>>> 
>>>>>> Some numbers would be good on the public list :) I know the numbers
>>>>>> already, but others on this list don’t.
>>>>> 
>>>>> I posted the numbers in the bug, but forgot to say so here...
>>>>> 
>>>>>> 
>>>>>>> Since the value from RDTSC can be subject to drifting between CPUs,
>>>>>>> we implement safeguards for this to make sure we never return a lower
>>>>>>> value than the previous values. This adds some overhead to nanoTime()
>>>>>>> but guards us against possible bugs in the OS. For users who are
>>>>>>> willing to trust the OS and need the fastest possible calls to
>>>>>>> System.nanoTime(), we add a flag to disable this safeguard:
>>>>>>> -XX:+AssumeMonotonicOSTimers.
>>>>>> 
>>>>>> I now wonder if this safeguard can produce a stream of exactly the same
>>>>>> timestamps if local clock is lagging behind. But considering the
>>>>>> alternative of answering the retrograde time, and the observation the
>>>>>> current Mac OS X mach_absolute_time() *appears* monotonic, having this
>>>>>> safeguard seems OK.
>>>>>> 
>>>>>>> webrev: http://cr.openjdk.java.net/~sla/8040140/webrev.00/
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8040140
>>>>>> 
>>>>>> This looks good to me.
>>>>> 
>>>>> Thanks.
>>>>> 
>>>>>> And, since this question will inevitably pop up, do we plan to bring it
>>>>>> into 8uX? I think many Mac users will be happy about that.
>>>>> 
>>>>> I would like to do so, but I would also like to have it sit and bake
>>>>> for a while in 9 before that. I think the 8u20 train has left the
>>>>> station, but perhaps 8u40?
>>>>> 
>>>>> /Staffan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140415/103b02f4/attachment-0001.html>