On 10 okt 2013, at 13:02, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> 
wrote:

> On 10.10.2013 05:44, David Holmes wrote:
>> On 10/10/2013 4:12 AM, Staffan Larsen wrote:
>>> 
>>> On 9 okt 2013, at 16:19, Jaroslav Bachorik
>>> <jaroslav.bacho...@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.
>> 
>> I second that. The elapsed_counter/elpased_timer APIs and
>> implementations are a tangled mess. But part of the problem has been
>> that people want/expect monotonic time-of-day based timestamps (yes a
>> contradiction - though some people make sure TOD does not get modified
>> on their production systems). The use of timestamps in logging has to be
>> examined carefully - mainly GC logging. I recall a "simple" attempted
>> change in the past that resulted in trying to compare a nanoTime based
>> timestamp with the TOD. :(
> 
> Actually, if I'm reading the sources right for Solaris and Win the monotonic 
> clock source is used to provide elapsed_counter() value. It falls back to TOD 
> when the monotonic clock source is not available.
> For Linux/BSD the TOD is used directly.
> 
> This makes me wonder if changing the linux/bsd implementation to follow the 
> same logic would be really that disruptive.

Good point. I would like a world where elapsed_counter is monotonic (where 
possible). Still a bit scary this late in the release, but an interesting 
experiment.

/Staffan




> 
> -JB-
>> 
>> David
>> -----
>> 
>>> /Staffan
>>> 
>>> 
>>>> 
>>>> -JB-
>>>> 
>>>>> 
>>>>> /Staffan
>>>>> 
>>>>> 
>>>>> On 9 okt 2013, at 13:26, Jaroslav Bachorik
>>>>> <jaroslav.bacho...@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-
>>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
> 

Reply via email to