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 <scolebou...@joda.org> wrote: > > On Wed, 12 May 2021 at 18:41, Roger Riggs <roger.ri...@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