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

Reply via email to