Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-08 Thread Naoto Sato
On Wed, 8 Sep 2021 15:35:27 GMT, Stephen Colebourne  
wrote:

> `toEpochMilli()` overflows at large/small values. Thus any attempt to 
> calculate the difference between two very large instants would fail.

Thanks. Fixed.

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-08 Thread Stephen Colebourne
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato  wrote:

>> Please review the fix to the issue. Avoiding overflow by not calling 
>> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference 
>> in nano unit.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added comments for the test.

`toEpochMilli()` overflows at large/small values. Thus any attempt to calculate 
the difference between two very large instants would fail.

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-08 Thread Naoto Sato
On Wed, 8 Sep 2021 13:58:59 GMT, Stephen Colebourne  
wrote:

> This change looks fine, but I think you also need a `millisUntil` private 
> method to fix the identical overflow problem with millis (which might as well 
> be fixed now).

`until()` for millis simply subtracts its `toEpochMilli()` from the end 
Instant's, so I am not sure that would cause the same false 
`ArithmeticException`. Can you please elaborate more?

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-08 Thread Stephen Colebourne
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato  wrote:

>> Please review the fix to the issue. Avoiding overflow by not calling 
>> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference 
>> in nano unit.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added comments for the test.

This change looks fine, but I think you also need a `millisUntil` private 
method to fix the identical overflow problem with millis (which might as well 
be fixed now).

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-08 Thread Roger Riggs
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato  wrote:

>> Please review the fix to the issue. Avoiding overflow by not calling 
>> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference 
>> in nano unit.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added comments for the test.

Marked as reviewed by rriggs (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-07 Thread Joe Wang
On Wed, 8 Sep 2021 00:36:29 GMT, Naoto Sato  wrote:

>> Please review the fix to the issue. Avoiding overflow by not calling 
>> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference 
>> in nano unit.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added comments for the test.

Marked as reviewed by joehw (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5396


Re: RFR: 8273369: Computing micros between two instants unexpectedly overflows for some cases [v5]

2021-09-07 Thread Naoto Sato
> Please review the fix to the issue. Avoiding overflow by not calling 
> nanosUntil() directly, which will overflow beyond Long.MAX_VALUE difference 
> in nano unit.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Added comments for the test.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5396/files
  - new: https://git.openjdk.java.net/jdk/pull/5396/files/bbd6abc3..ccf73bf7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5396=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5396=03-04

  Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5396.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5396/head:pull/5396

PR: https://git.openjdk.java.net/jdk/pull/5396