Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource I've made the obvious changes to fix the offset reflection. However it does now mean that the offset is being altered for a singleton where previously it would have only affected Clock.systemUtc(). Is the test class spun up in its own JVM process? Just want to make sure that manipulating the singleton clock won't impact other tests. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource my bad - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource The Error was caused by the movement of the field `offset`, from inner `SystemClock` class to `Clock`. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource It's a side effect of JEP 403, i believe - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne
wrote:
>> 8266846: Add java.time.InstantSource
>
> Stephen Colebourne has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8266846: Add java.time.InstantSource
Another test started failing with your fix. Apparently, the following piece in
java/time/test/java/time/TestClock_System.java throws
ExceptionInInitializerError.
static {
try {
offsetField =
Class.forName("java.time.Clock$SystemClock").getDeclaredField("offset");
offsetField.setAccessible(true);
} catch (ClassNotFoundException | NoSuchFieldException ex) {
throw new ExceptionInInitializerError(ex);
}
}
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Wed, 19 May 2021 08:41:08 GMT, Peter Levart wrote: >> This isn't my logic - it is existing code that has been moved. I'm not a fan >> of infinite retry loops as the can hang the system. But I'm happy to change >> it to work that way if there is a consensus to do so. > > I see the localOffset passed to VM.getNanoTimeAdjustment() has to be in the > range of current time +/- 2^32 seconds. This means the thread would have to > sleep for ~136 years before the exception is triggered. I think this is more > than enough. :-) Exactly. You could hit the 1ns if the operating system clock was concurrently moved backward by a NTP adjustment of ~ 1024 seconds at the same time that the offset is taken but this would be extremely unlikely - I don't believe it deserves any infinite retry loop. That was discussed at the time the enhancement that provides sub-millisecond precision was proposed, and it was rejected in favor of a one time retry. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource test/jdk/java/time/test/java/time/TestInstantSource.java line 33: > 31: import java.time.Duration; > 32: import java.time.Instant; > 33: import java.time.ZoneId; Needs `import java.time.InstantSource;` - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Tue, 18 May 2021 23:14:53 GMT, Stephen Colebourne wrote: >> src/java.base/share/classes/java/time/Clock.java line 487: >> >>> 485: // it more unlikely to hit the 1ns in the future condition. >>> 486: localOffset = System.currentTimeMillis()/1000 - 1024; >>> 487: >> >> Is it possible that after a fresh localOffset is retrieved, the thread is >> preempted and when it is scheduled again after a pause, the >> getNanoTimeAdjustment below returns -1 ? Would it help if instead of >> throwing exception, there was an infinite retry loop? > > This isn't my logic - it is existing code that has been moved. I'm not a fan > of infinite retry loops as the can hang the system. But I'm happy to change > it to work that way if there is a consensus to do so. I see the localOffset passed to VM.getNanoTimeAdjustment() has to be in the range of current time +/- 2^32 seconds. This means the thread would have to sleep for ~136 years before the exception is triggered. I think this is more than enough. :-) - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
On Sun, 16 May 2021 07:39:21 GMT, Peter Levart wrote: >> Stephen Colebourne has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8266846: Add java.time.InstantSource > > src/java.base/share/classes/java/time/Clock.java line 487: > >> 485: // it more unlikely to hit the 1ns in the future condition. >> 486: localOffset = System.currentTimeMillis()/1000 - 1024; >> 487: > > Is it possible that after a fresh localOffset is retrieved, the thread is > preempted and when it is scheduled again after a pause, the > getNanoTimeAdjustment below returns -1 ? Would it help if instead of throwing > exception, there was an infinite retry loop? This isn't my logic - it is existing code that has been moved. I'm not a fan of infinite retry loops as the can hang the system. But I'm happy to change it to work that way if there is a consensus to do so. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v3]
> 8266846: Add java.time.InstantSource Stephen Colebourne has updated the pull request incrementally with one additional commit since the last revision: 8266846: Add java.time.InstantSource - Changes: - all: https://git.openjdk.java.net/jdk/pull/4016/files - new: https://git.openjdk.java.net/jdk/pull/4016/files/7fa9d0ec..425e01a8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4016=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4016=01-02 Stats: 12 lines in 3 files changed: 1 ins; 3 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/4016.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4016/head:pull/4016 PR: https://git.openjdk.java.net/jdk/pull/4016
