RFR 8068730: Increase the precision of the implementation of java.time.Clock.systemUTC()

[Daniel Fuchs]

On 09/01/15 19:26, Stephen Colebourne wrote:
> Well that is a nice surprise ;-) And I think the implementation via an
> adjustment is very sensible.

Thanks Stephen :-)


> In java.time.Instant, a readObject() has been added. However, I don't
> believe this will ever be called because Instant has a writeReplace()
> method and so is not deserialized. (There may be some security related
> "evil serialization stream" reason why readObject() should exist, I
> can't say).

Well - I don't know the history of that - so I can't comment.
I have not touched java.time.Instant in this patch.
I added a readObject to Clock.SystemClock to reinitialize the
new transient offset field. It is not strictly required, but for
consistency I believe it's better.

> In java.util.Formatter, there should be no reason to retain use of the
> MILLI_OF_SECOND, as any TemporalAccessor that can return
> NANO_OF_SECOND should also return MILLI_OF_SECOND, however the spec is
> not quite tight enough to guarantee that sadly. As such, I think the
> catch will have to be retained.

OK. Thanks for confirming.


> TestFormatter has some commented out System.out statements that should
> probably be removed.

Right. Done.


> I suspect that this change will break some user code, but it certainly
> doesn't break the spec. As such, I think the change should go in.

Thanks :-)

> I do believe that this change means that a new method should be added
> to Clock however:
>
>      public static Clock tickMillis(ZoneId zone) {
>          return new TickClock(system(zone), NANOS_PER_MILLI);
>      }
>
> While this can be achieved without a new method, the API would feel
> slightly strange without it now better-than-milli clocks exist. I
> recommend raising a separate RFE to track this.

Ah - I see. I didn't think of that. It looks like a sensible
RFE. Agreed.

best regards,

-- daniel

>
> Stephen
>
>
>
> On 9 January 2015 at 16:56, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>> Hi,
>>
>> Please find below a webrev for:
>>
>> 8068730: Increase the precision of the implementation
>>           of java.time.Clock.systemUTC()
>> https://bugs.openjdk.java.net/browse/JDK-8068730
>>
>> The java.time.Clock.system() method (and variants thereof) are
>> specified to "obtain a clock that returns the current instant
>> using best available system clock". However the current
>> implementation of the clock returned is based on
>> System.currentTimeMillis() whereas the underlying native clock
>> used by System.currentTimeMillis() has often a greater precision
>> than milliseconds (for instance, on Linux, System.currentTimeMillis()
>> is based on gettimeofday, which offers microseconds precision).
>>
>> This patch enhances the implementation of the
>> java.time.Clock.SystemClock, so that it offer at least the
>> same precision than the underlying clock available on the system.
>>
>> There is no change in the public API.
>>
>> http://cr.openjdk.java.net/~dfuchs/webrev_8068730/webrev.00/
>>
>> Some more details on the patch:
>>
>> native (hotspot) side:
>>
>>   - adds a new method to the os class:
>>     os::javaTimeSystemUTC(jlong &seconds, jlong &nanos)
>>     which allows to get the time in the form of a number
>>     of seconds and number of nanoseconds
>>     (see os.hpp and various os_<os>.cpp files)
>>
>>   - adds a JVM_GetNanoTimeAdjustment method, which takes
>>     an offset (in seconds) as parameter and returns a
>>     nano time adjustment compared to the offset.
>>     Calls os::javaTimeSystemUTC to compute the adjustment
>>     (see jvm.cpp)
>>
>> java (jdk) side:
>>
>>   - adds a native sun.misc.VM.getNanoTimeAdjustment method
>>     (which is bound to JVM_GetNanoTimeAdjustment)
>>     (see VM.java and VM.c)
>>
>>   - modifies java.time.Clock.SystemClock to call the new
>>     sun.misc.VM.getNanoTimeAdjustment instead of
>>     System.currentTimeMillis.
>>
>>   - fixes java.util.Formatter - which wasn't expecting
>>     greater than millisecond precision.
>>
>> testing:
>>
>>   - A new test is added to test sun.misc.VM.getNanoTimeAdjustment
>>     In particular it checks the edge cases.
>>   - New tests are added to TestClock_System.java to check for
>>     the increased precision.
>>     There is also a test for the edge cases there.
>>   - Some java.time tests where tripped by the increased precision,
>>     and had to be modified to take that into account
>>
>> Note: comparing the new os::javaTimeSystemUTC with os::javaTimeMillis
>>        can help in reviewing the changes.
>>
>> best regards,
>>
>> -- daniel