Proposal for new interface: TimeSource
Naoto Sato
naoto.sato at oracle.com
Fri May 28 16:53:19 UTC 2021
As I commented on the PR, the test needs to run in othervm mode:
https://github.com/openjdk/jdk/pull/4016#issuecomment-844551175
--- a/test/jdk/java/time/test/TEST.properties
+++ b/test/jdk/java/time/test/TEST.properties
@@ -1,5 +1,5 @@
# java.time tests use TestNG
TestNG.dirs = ..
-othervm.dirs = java/time/chrono java/time/format
+othervm.dirs = java/time
lib.dirs = /test/lib /test/jdk/tools/lib
lib.build = jdk.test.lib.RandomFactory
This will run the test case in java/time directory explicitly in othervm
mode, but other tests in java/time too, which may be unnecessary. I am
not aware of other way to specify it individually.
Naoto
On 5/27/21 5:03 PM, Stephen Colebourne wrote:
> Hi all,
> Is there anything I need to do to progress the CSR and/or PR?
> thanks
> Stephen
>
> On Thu, 13 May 2021 at 22:05, Stephen Colebourne <scolebourne at joda.org> wrote:
>>
>> On Wed, 12 May 2021 at 18:41, Roger Riggs <roger.riggs at oracle.com> wrote:
>>> Will you be posting a PR for the implementation?
>>> It is frequently helpful to evaluate the CSR and the implementation
>>> concurrently.
>>
>> PR: https://github.com/openjdk/jdk/pull/4016
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8266846
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8266847
>>
>> The PR takes a middle ground approach to the implementation.
>>
>> It is not practical to remove the existing package-scoped Clock
>> implementation classes (SystemClock, TickClock, FixedClock,
>> OffsetClock) despite the fact that these would be better expressed as
>> classes that only implement `InstantSource`. However, given that
>> "system" is the 99%+ use case, I do believe it is worth adding a
>> dedicated `SystemInstantSource` class, as per the PR.
>>
>> To achieve this I moved the actual logic using
>> `VM.getNanoAdjustment()` into a static method which can then be called
>> directly from three places - Instant.now(), SystemClock and
>> SystemInstantSource. Previously, every instance of SystemClock
>> performed the VM/offset calculations separately. The new logic
>> performs them once based on a single shared static variable. I have no
>> reason to believe this changes the memory model or performance, but I
>> must flag it up for reviewers.
>>
>> When initially discussing the proposal, I planned to add a new static
>> method `Clock.of(InstantSource, ZoneId)`. When implementing the change
>> I found that the method was adding no value as the instance method
>> `InstantSource.withZone(ZoneId)` achieves the same outcome, so I
>> omitted it.
>>
>> The Mac test failure appears to be unconnected to the change.
>>
>> Thanks for any and all reviews!
>> Stephen
More information about the core-libs-dev
mailing list