jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

[Jaroslav Bachorik]

On 9.10.2013 16:10, Staffan Larsen wrote:
> There is now an awful amount of different timestamps in the Management class. Can they be consolidated somehow? At least _begin_vm_creation_time and the new _begin_vm_creation_ns.
>
> This discussion also implies that the "elapsed time" we print in the hs_err file is also wrong. It uses os::elapsedTime() which uses os::elapsed_counter().
>
> And I guess the same thing for the VM.uptime Diagnostic Command (class VMUptimeDCmd) which also relies on os::elapsed_counter().

Also the reported GC pauses duration might be wrong since it uses 
Management::timestamp().

On the first sight the change looks rather trivial. But, honestly, I'm 
not sure which other parts could for whatever reason break once the 
time-of-day timestamp is replaced with a monotonic equivalent. One would 
think that it shouldn't matter but one never knows ...

Staffan, do you think this kind of change is suitable for the current 
phase of JDK release cycle? I think I could improve the patch in few 
days and then it should probably be able to pass the review before ZBB. 
But, it's only P3  ...

-JB-

>
> /Staffan
>
>
> On 9 okt 2013, at 13:26, Jaroslav Bachorik <jaroslav.bachorik at oracle.com> wrote:
>
>> On 8.10.2013 23:46, David Holmes wrote:
>>> On 8/10/2013 10:36 PM, Jaroslav Bachorik wrote:
>>>> On 8.10.2013 09:34, David Holmes wrote:
>>>>> Jaroslav,
>>>>>
>>>>> On 2/10/2013 6:47 PM, Jaroslav Bachorik wrote:
>>>>>> Hello,
>>>>>>
>>>>>> currently the JVM uptime reported by the RuntimeMXBean is based on
>>>>>> System.currentTimeMillis() which makes it susceptible to changes of the
>>>>>> OS time (eg. changing timezone, NTP synchronization etc.). The uptime
>>>>>> should not depend on the system time and should be calculated using a
>>>>>> monotonic clock source.
>>>>>>
>>>>>> There is already the way to get the actual JVM uptime in ticks. It is
>>>>>> accessible as Management::timestamp() and the ticks are convertible to
>>>>>> milliseconds using Management::ticks_to_ms(ts_ticks) thus making it
>>>>>> very
>>>>>> easy to switch to the monotonic clock based uptime.
>>>>>
>>>>> Maybe I'm missing something but TiumeStamp updates using
>>>>> os::elapsed_counter() which on Linux uses gettimeofday so is not a
>>>>> monotonic clock source.
>>>>
>>>> Hm, yes. I wasn't aware of this linux/bsd specific.
>>>>
>>>> Is there any reason why a non monotonic clock source is used for
>>>> timestamping except of the historical one? os::javaTimeNanos() uses
>>>> montonic clock when available - why can't be the same used for
>>>> os::elapsed_counter() especially when a counter based on "gettimeofday"
>>>> is not really a counter?
>>>
>>> It is all historical. These elapsed_counters and elapsed_timers make me
>>> cringe. But changing it has a lot of potential consequences because of
>>> the way these are used in logging etc. Certainly not something to be
>>> contemplated at this stage of JDK 8.
>>>
>>> Perhaps a simpler fix here is to expose a startUpTimeNanos that can then
>>> be used for the uptime.
>>
>> My attempt at this is at http://cr.openjdk.java.net/~jbachorik/6523160/webrev.01/hotspot
>> I am using os::javaTimeNanos() to get the monotonic ticks where possible.
>>
>> The JDK part stays the same as for webrev.00
>>
>> -JB-
>>
>>>
>>> David
>>>
>>>> -JB-
>>>>
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>>
>>>>>> The patch consists of the hotspot and jdk parts.
>>>>>>
>>>>>> For the hotspot a new constant needs to be introduced in
>>>>>> src/share/vm/services/jmm.h and the actual logic to obtain the
>>>>>> uptime in
>>>>>> milliseconds is added in src/share/vm/services/management.cpp.
>>>>>>
>>>>>> For the jdk the changes comprise of adding the necessary JNI bridging
>>>>>> methods in order to get the new uptime, introducing the same constant
>>>>>> that is used in hotspot and changes to mapfile-vers files in order to
>>>>>> properly build the native library.
>>>>>>
>>>>>> Issue:   https://bugs.openjdk.java.net/browse/JDK-6523160
>>>>>> Webrev:  http://cr.openjdk.java.net/~jbachorik/6523160/webrev.00
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -JB-
>>>>
>>
>