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

Staffan Larsen staffan.larsen at oracle.com
Tue Apr 15 12:21:57 UTC 2014 

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

> Hi Staffan,
> 
> For the solaris changes ...
> 
> This comment is not accurate from the original code:
> 
> 1367 // gethrtime can move backwards if read from one cpu and then a different cpu
> 1368 // getTimeNanos is guaranteed to not move backward on Solaris
> 1369 // AssumeMonotonicOSTimers can be used to remove this guarantee.
> 
> can I suggest
> 
> // gethrtime should be monotonic but has had bugs in the past.
> // getTimeNanos must be guaranteed not to move backwards, so we add a
> // check in case gethrtime is buggy.
> // AssumeMonotonicOSTimers can be set to remove this additional check.

Fixed.

> With regard to code deletion this seems to be dead code now as well:
> 
> 350 const int LOCK_BUSY = 1;
> 351 const int LOCK_FREE = 0;
> 352 const int LOCK_INVALID = -1;
> 
> 354 static volatile int max_hrtime_lock = LOCK_FREE;     // Update counter with LSB as lock-in-progress

Fixed.

New version: http://cr.openjdk.java.net/~sla/8040140/webrev.03/

/Staffan

> 
> Thanks,
> David
> 
> 
> On 15/04/2014 9:42 PM, Staffan Larsen wrote:
>> 
>> On 15 apr 2014, at 11:38, David Holmes <david.holmes at oracle.com
>> <mailto: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
>>>> <mailto: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>
>>>>>> <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>
>>>>>>> <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
>>

More information about the hotspot-runtime-dev mailing list