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

[Staffan Larsen]

On 9 okt 2013, at 16:19, Jaroslav Bachorik <jaroslav.bachorik at oracle.com> wrote:

> 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  ...

I think it is a bit late in the release cycle to clean this up in the way it should be cleaned up. I think we should wait until the first 8 update release and do a more thorough job than we have time for right now.

/Staffan


> 
> -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-
>>>>> 
>>> 
>> 
>