Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Sat, 13 Feb 2021 21:30:12 GMT, Andrey Turbanov
wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added table of available algorithms.
>
> test/jdk/java/util/Random/RandomTestBsi1999.java line 227:
>
>> 225: static int checkRunStats(int[] stats) {
>> 226:int runFailure = 0;
>> 227:runFailure |= ((2267 <= stats[1]) || (stats[1] <= 2733)) ? 0
>> : 1;
>
> 1. confusing formatting.
> 2. This condition is always `true`. Looks like `&&` should be used instead of
> `||`
Correct. Changed.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Tue, 16 Feb 2021 15:54:08 GMT, Rémi Forax wrote: >> The interface method is a default method, so not technically an override. > > It's not an override but @Override has a broader semantics than just > overriding an existing method. > Given that it helps to understand that this method is part of a larger > algorithm, i think this method should be tagged with @Override Okay - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Tue, 16 Feb 2021 14:03:56 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
>> 1548:
>>
>>> 1546: * @return a stream of (pseudo)randomly chosen {@code int}
>>> values
>>> 1547: */
>>> 1548:
>>
>> Is `@Override` intentionally omitted?
>
> The interface method is a default method, so not technically an override.
It's not an override but @Override has a broader semantics than just overriding
an existing method.
Given that it helps to understand that this method is part of a larger
algorithm, i think this method should be tagged with @Override
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Sat, 13 Feb 2021 15:03:53 GMT, Andrey Turbanov
wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added table of available algorithms.
>
> src/java.base/share/classes/java/util/Random.java line 29:
>
>> 27:
>> 28: import java.io.*;
>> 29: import java.math.BigInteger;
>
> This import is not actually used
good
> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
> 115:
>
>> 113: static final String BAD_BOUND = "bound must be positive";
>> 114: static final String BAD_FLOATING_BOUND = "bound must be finite and
>> positive";
>> 115: static final String BAD_RANGE = "bound must be greater than origin";
>
> Unused fields in Random class now can be cleaned up:
> `java.util.Random#BadRange`, `java.util.Random#BadSize`
Good
> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
> 149:
>
>> 147: */
>> 148: public static void checkJumpDistance(double distance) {
>> 149: if (!(distance > 0.0 && distance < Float.POSITIVE_INFINITY
>
> I wounder if this usage of `Float.POSITIVE_INFINITY` intentional here? Looks
> a bit suspicious for comparison with `double` argument
Turns out the method is no longer used.
> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
> 1548:
>
>> 1546: * @return a stream of (pseudo)randomly chosen {@code int}
>> values
>> 1547: */
>> 1548:
>
> Is `@Override` intentionally omitted?
The interface method is a default method, so not technically an override.
> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
> 1943:
>
>> 1941:
>> 1942: // IllegalArgumentException messages
>> 1943: static final String BadLogDistance = "logDistance must be
>> non-negative";
>
> seems unused
Good.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Thu, 11 Feb 2021 16:02:03 GMT, Jim Laskey wrote:
>> This PR is to introduce a new random number API for the JDK. The primary API
>> is found in RandomGenerator and RandomGeneratorFactory. Further description
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>>
>> javadoc can be found at
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>>
>> old PR: https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Added table of available algorithms.
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
149:
> 147: */
> 148: public static void checkJumpDistance(double distance) {
> 149: if (!(distance > 0.0 && distance < Float.POSITIVE_INFINITY
I wounder if this usage of `Float.POSITIVE_INFINITY` intentional here? Looks a
bit suspicious for comparison with `double` argument
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
1548:
> 1546: * @return a stream of (pseudo)randomly chosen {@code int}
> values
> 1547: */
> 1548:
Is `@Override` intentionally omitted?
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
1943:
> 1941:
> 1942: // IllegalArgumentException messages
> 1943: static final String BadLogDistance = "logDistance must be
> non-negative";
seems unused
test/jdk/java/util/Random/RandomTestBsi1999.java line 227:
> 225: static int checkRunStats(int[] stats) {
> 226:int runFailure = 0;
> 227:runFailure |= ((2267 <= stats[1]) || (stats[1] <= 2733)) ? 0
> : 1;
1. confusing formatting.
2. This condition is always `true`. Looks like `&&` should be used instead of
`||`
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Thu, 11 Feb 2021 16:02:03 GMT, Jim Laskey wrote: >> This PR is to introduce a new random number API for the JDK. The primary API >> is found in RandomGenerator and RandomGeneratorFactory. Further description >> can be found in the JEP https://openjdk.java.net/jeps/356 . >> >> javadoc can be found at >> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html >> >> old PR: https://github.com/openjdk/jdk/pull/1273 > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Added table of available algorithms. src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 115: > 113: static final String BAD_BOUND = "bound must be positive"; > 114: static final String BAD_FLOATING_BOUND = "bound must be finite and > positive"; > 115: static final String BAD_RANGE = "bound must be greater than origin"; Unused fields in Random class now can be cleaned up: `java.util.Random#BadRange`, `java.util.Random#BadSize` - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
On Thu, 11 Feb 2021 16:02:03 GMT, Jim Laskey wrote: >> This PR is to introduce a new random number API for the JDK. The primary API >> is found in RandomGenerator and RandomGeneratorFactory. Further description >> can be found in the JEP https://openjdk.java.net/jeps/356 . >> >> javadoc can be found at >> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html >> >> old PR: https://github.com/openjdk/jdk/pull/1273 > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Added table of available algorithms. src/java.base/share/classes/java/util/Random.java line 29: > 27: > 28: import java.io.*; > 29: import java.math.BigInteger; This import is not actually used - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v17]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Added table of available algorithms. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/1c17ad35..f1e3699d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1292=16 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1292=15-16 Stats: 181 lines in 3 files changed: 140 ins; 0 del; 41 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
