RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

classic Classic list List threaded Threaded
46 messages Options
123
Reply | Threaded
Open this post in threaded view
|

RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

Jim Laskey-3
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

-------------

Commit messages:
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862; Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
 - ... and 15 more: https://git.openjdk.java.net/jdk/compare/f7517386...2b3e4ed7

Changes: https://git.openjdk.java.net/jdk/pull/1292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8248862
  Stats: 13319 lines in 25 files changed: 11110 ins; 2132 del; 77 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

Remi Forax
Ok, i've taking the time to read some literature about random generators because for me the Mersenne Twister was still the king.

The current API proposed as clearly two levels, you have the user level and the implementation level, at least the implementation level should seen as a SPI

RandomGenerator is the user facing API, for me it should be the sole interface exposed by the API, the others (leap, arbitrary leap and split) should be optional methods.

In term of factory methods, we should have user driven methods:
- getDefaultGenerator() that currently returns a L64X128MixRandom and can be changed in the future
- getFastGenerator() that currently returns a Xoroshiro128PlusPlus and can be changed in the future
- getDefault[Splitable|Leapable|etc]Generator that returns a default generator with the methods splits|leaps|etc defined
- of / getByName that returns a specific generator by its name (but mov ed in a SPI class)

The example in the documentation should use getDefaultGenerator() and not of() to avoid the problem all the programming languages currently have by having over-specified that the default generator is a Mersenne Twister.

All methods that returns a stream of the available implementations should be moved in the SPI package.

Rémi

---
An honest question,
why do we need so many interfaces for the different categories of RandomGenerator ?

My fear is that we are encoding the state of our knowledge of the different kinds of random generators now so it will not be pretty in the future when new categories of random generator are discovered/invented.
If we can take example of the past to predict the future, 20 years ago, what should have been the hierarchy at that time.
Is it not reasonable to think that we will need new kinds of random generator in the future ?

I wonder if it's not better to have one interface and several optional methods like we have with the collections, it means that we are loosing the possibilities to precisely type a method that only works with a precise type of generator but it will be more future proof.

Rémi

----- Mail original -----
> De: "Jim Laskey" <[hidden email]>
> À: "core-libs-dev" <[hidden email]>, "security-dev" <[hidden email]>
> Envoyé: Mercredi 18 Novembre 2020 14:52:56
> Objet: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

> 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
>
> -------------
>
> Commit messages:
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862; Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - 8248862: Implement Enhanced Pseudo-Random Number Generators
> - ... and 15 more: https://git.openjdk.java.net/jdk/compare/f7517386...2b3e4ed7
>
> Changes: https://git.openjdk.java.net/jdk/pull/1292/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8248862
>  Stats: 13319 lines in 25 files changed: 11110 ins; 2132 del; 77 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

Remi Forax
----- Mail original -----
> De: "Jim Laskey" <[hidden email]>
> À: "Remi Forax" <[hidden email]>
> Cc: "Jim Laskey" <[hidden email]>, "core-libs-dev" <[hidden email]>, "security-dev"
> <[hidden email]>
> Envoyé: Lundi 23 Novembre 2020 14:58:50
> Objet: Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators

> Rémi,

Hi Jim,

>
>> On Nov 21, 2020, at 8:03 AM, Remi Forax <[hidden email]> wrote:
>>
>> Ok, i've taking the time to read some literature about random generators because
>> for me the Mersenne Twister was still the king.
>>
>> The current API proposed as clearly two levels, you have the user level and the
>> implementation level, at least the implementation level should seen as a SPI
>
> Yes, We agree. It was decided that we work on the SPI separately from the
> original JEP. IMHO the implementation issues are too complex for a single JEP.
> So, the goal is to release the user level now, and refine the SPI at a later
> date. Only RandomGenerator (with descendant interfaces) and
> RandomGeneratorFactory will be public facing in the first release.
>
>
>>
>> RandomGenerator is the user facing API, for me it should be the sole interface
>> exposed by the API, the others (leap, arbitrary leap and split) should be
>> optional methods.
>
> Fair enough, but if your task requires leapable, you might be disappointed when
> you get an UnsupportedOperationException invoking leap. I personally like the
> "to the quick" ability of using LeapableGenerator for narrowing down the
> search.
>
>    LeapableGenerator leapable = LeapableGenerator.all().findFirst().orElseThrow();
>
> Open for discussion.

For me, it's not different from
   RandomGenerator leapable = RandomGenerator.getLeapableGenerator();

The question is: does this generator will be used directly or will it be sent through several layers of API,
because if it travels a lot, then a specific interface is better, otherwise a generic interface is better because it's easier to use from a user perspective and easier to maintain because you encode less knowledge.

>
>>
>> In term of factory methods, we should have user driven methods:
>> - getDefaultGenerator() that currently returns a L64X128MixRandom and can be
>> changed in the future
>> - getFastGenerator() that currently returns a Xoroshiro128PlusPlus and can be
>> changed in the future
>> - getDefault[Splitable|Leapable|etc]Generator that returns a default generator
>> with the methods splits|leaps|etc defined
>> - of / getByName that returns a specific generator by its name (but mov ed in a
>> SPI class)
>
> I'm concerned that the "can be changed in the future" aspect of your default
> methods will create a false reliance. We sort of have default now with the
> java.util.Random class. If we were to change the underpinnings of Random all
> heck would break loose. We already ran into that aspect during testing - tests
> that relied on sequences from fixed seeds.

The incantation all().findFirst() has the exact same issue.

I believe that the fact that there are several defaults shield you a little because it's understood that those default will change from time to time.

>
> We try to discourage the use of 'of', but there is a class of user (machine
> learning for example) that wants to be able to specify exactly. Often, choosing
> a specific fast prng for testing and then a more sophisticated one for
> production. The main purpose for RandomGenerator is swapability.


If of() is part of the SPI, i think it's fine, because you are explicitly asking for an implementation.
There is still the question about what implementations is provided for a specific jdk.

>
>
>>
>> The example in the documentation should use getDefaultGenerator() and not of()
>> to avoid the problem all the programming languages currently have by having
>> over-specified that the default generator is a Mersenne Twister.
>
> I have a problem with this as well. It would make it difficult to deprecate
> java.util.Random.

As i said above, not really because you are not asking for a specific implementation here but for a good default.
In practice, it will depends if the implementation is changed enough in the next releases of the JDK or not.

>
>>
>> All methods that returns a stream of the available implementations should be
>> moved in the SPI package.
>
> Open for discussion.

Rémi
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Jim Laskey-3
> 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:

  8248862: Implement Enhanced Pseudo-Random Number Generators
 
  Changes to RandomGeneratorFactory requested by @PaulSandoz

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1292/files
  - new: https://git.openjdk.java.net/jdk/pull/1292/files/9d6d1a94..802fa530

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=01-02

  Stats: 54 lines in 1 file changed: 23 ins; 7 del; 24 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Rémi Forax
On Wed, 25 Nov 2020 13:09:03 GMT, Jim Laskey <[hidden email]> 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:
>
>   8248862: Implement Enhanced Pseudo-Random Number Generators
>  
>   Changes to RandomGeneratorFactory requested by @PaulSandoz

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 88:

> 86:  * <pre>{@code
> 87:  *     RandomGeneratorFactory<RandomGenerator> best = RandomGenerator.all()
> 88:  *         .sorted((f, g) -> Integer.compare(g.stateBits(), f.stateBits()))

use Comparator.comparingInt(RandomGenerator::stateBits) instead of the lambda

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 116:

> 114:      * Map of properties for provider.
> 115:      */
> 116:     private volatile Map<RandomGeneratorProperty, Object> properties = null;

`= null` is useless here

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 106:

> 104:      * Map of provider classes.
> 105:      */
> 106:     private static volatile Map<String, Provider<? extends RandomGenerator>> factoryMap;

should be FACTORY_MAP given that it's a static field

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Rémi Forax
In reply to this post by Jim Laskey-3
On Wed, 25 Nov 2020 13:09:03 GMT, Jim Laskey <[hidden email]> 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:
>
>   8248862: Implement Enhanced Pseudo-Random Number Generators
>  
>   Changes to RandomGeneratorFactory requested by @PaulSandoz

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 335:

> 333:                         ctorBytes = tmpCtorBytes;
> 334:                         ctorLong = tmpCtorLong;
> 335:                         ctor = tmpCtor;

This one is a volatile write, so the idea is that ctorBytes and ctorLong does not need to be volatile too, which i think is not true if there is a code somewhere that uses ctorBytes or ctorLong without reading ctor.
This code is too smart for me, i'm pretty sure it is wrong too on non TSO cpu.

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 480:

> 478:         } catch (Exception ex) {
> 479:             // Should never happen.
> 480:             throw new IllegalStateException("Random algorithm " + name() + " is missing a default constructor");

chain the exception ...

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 497:

> 495:             ensureConstructors();
> 496:             return ctorLong.newInstance(seed);
> 497:         } catch (Exception ex) {

this one is very dubious because the result in an exception is thrown is a random generator with the wrong seed

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 517:

> 515:             ensureConstructors();
> 516:             return ctorBytes.newInstance((Object)seed);
> 517:         } catch (Exception ex) {

same as above

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Rémi Forax
In reply to this post by Rémi Forax
On Wed, 25 Nov 2020 15:59:01 GMT, Jim Laskey <[hidden email]> wrote:

>> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 88:
>>
>>> 86:  * <pre>{@code
>>> 87:  *     RandomGeneratorFactory<RandomGenerator> best = RandomGenerator.all()
>>> 88:  *         .sorted((f, g) -> Integer.compare(g.stateBits(), f.stateBits()))
>>
>> use Comparator.comparingInt(RandomGenerator::stateBits) instead of the lambda
>
> Not sure that `.sorted(Comparator.comparingInt(RandomGeneratorFactory<RandomGenerator>::stateBits).reversed())` is simpler than  `.sorted((f, g) -> Integer.compare(g.stateBits(), f.stateBits()))`.

At least, it's more clear that it's reversed, i've initially miss the fact that f and g are swapped.
And :: is able to do inference so, i believe it can be written
  `.sorted(Comparator.comparingInt(RandomGeneratorFactory::stateBits).reversed())`

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Rémi Forax
In reply to this post by Rémi Forax
On Wed, 25 Nov 2020 16:22:34 GMT, Jim Laskey <[hidden email]> wrote:

>> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 151:
>>
>>> 149:         if (fm == null) {
>>> 150:             synchronized (RandomGeneratorFactory.class) {
>>> 151:                 if (RandomGeneratorFactory.factoryMap == null) {
>>
>> if `RandomGeneratorFactory.factoryMap` is not null, it returns null because the value is not stored in fm.
>>
>> Please use the class holder idiom (cf Effective Java)  instead of using the double-check locking pattern.
>
> ? set in 148 and 152, but  class holder idiom +1

If the second `RandomGeneratorFactory.factoryMap` return a non null value ...

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Wed, 25 Nov 2020 13:45:46 GMT, Rémi Forax <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8248862: Implement Enhanced Pseudo-Random Number Generators
>>  
>>   Changes to RandomGeneratorFactory requested by @PaulSandoz
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 157:
>
>> 155:                             .stream()
>> 156:                             .filter(p -> !p.type().isInterface())
>> 157:                             .collect(Collectors.toMap(p -> p.type().getSimpleName().toUpperCase(),
>
> toUpperCase() depends on the Locale !

It was a lame thing to do anyway - removed

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Wed, 25 Nov 2020 13:54:47 GMT, Rémi Forax <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8248862: Implement Enhanced Pseudo-Random Number Generators
>>  
>>   Changes to RandomGeneratorFactory requested by @PaulSandoz
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 235:
>
>> 233:             throws IllegalArgumentException {
>> 234:         Map<String, Provider<? extends RandomGenerator>> fm = getFactoryMap();
>> 235:         Provider<? extends RandomGenerator> provider = fm.get(name.toUpperCase());
>
> again use of toUpperCase() instead of toUpperCase(Locale)

removed

> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 250:
>
>> 248:      * @return Stream of matching Providers.
>> 249:      */
>> 250:     static <T extends RandomGenerator> Stream<RandomGeneratorFactory<T>> all(Class<? extends RandomGenerator> category) {
>
> this signature is weird, T is not used in the parameter, so in case return any type of Stream<RandomGeneratorFactory<T>> from a type POV, should it be
> ` <T extends RandomGenerator> Stream<RandomGeneratorFactory<T>> all(Class<? extends T> category)` instead ?

agree

> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 269:
>
>> 267:      * @throws IllegalArgumentException when either the name or category is null
>> 268:      */
>> 269:     static <T> T of(String name, Class<? extends RandomGenerator> category)
>
> Same issue as above, T is not used as a constraint

agree

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Rémi Forax
On Wed, 25 Nov 2020 14:10:17 GMT, Rémi Forax <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8248862: Implement Enhanced Pseudo-Random Number Generators
>>  
>>   Changes to RandomGeneratorFactory requested by @PaulSandoz
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 497:
>
>> 495:             ensureConstructors();
>> 496:             return ctorLong.newInstance(seed);
>> 497:         } catch (Exception ex) {
>
> this one is very dubious because the result in an exception is thrown is a random generator with the wrong seed

This is explained in the docs.

> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 480:
>
>> 478:         } catch (Exception ex) {
>> 479:             // Should never happen.
>> 480:             throw new IllegalStateException("Random algorithm " + name() + " is missing a default constructor");
>
> chain the exception ...

agree

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Wed, 25 Nov 2020 14:16:20 GMT, Rémi Forax <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8248862: Implement Enhanced Pseudo-Random Number Generators
>>  
>>   Changes to RandomGeneratorFactory requested by @PaulSandoz
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 46:
>
>> 44: import java.util.stream.Stream;
>> 45: import jdk.internal.util.random.RandomSupport.RandomGeneratorProperty;
>> 46:
>
> Instead of calling a method properties to create a Map, it's usually far easier to use an annotation,
> annotation values supports less runtime type so BigInteger is not supported but using a String instead should be OK.

I kind of like the idea - not sure how expressive a BigInteger string is though. I might be able to express as <i, j, k>  BigInteger.ONE.shiftLeft(i).subtract(j).shiftLeft(k). Will ponder.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v6]

Jim Laskey-3
In reply to this post by Jim Laskey-3
> 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:

  8248862: Implement Enhanced Pseudo-Random Number Generators
 
  Propagate exception

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1292/files
  - new: https://git.openjdk.java.net/jdk/pull/1292/files/a6851940..ca29ff74

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=04-05

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v7]

Jim Laskey-3
In reply to this post by Jim Laskey-3
> 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:

  8248862: Implement Enhanced Pseudo-Random Number Generators
 
  Cleanups from Chris H.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1292/files
  - new: https://git.openjdk.java.net/jdk/pull/1292/files/ca29ff74..a8cc0795

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=05-06

  Stats: 13 lines in 1 file changed: 4 ins; 3 del; 6 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v9]

Jim Laskey-3
In reply to this post by Jim Laskey-3
> 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 with a new target base due to a merge or a rebase. The pull request now contains 34 commits:

 - Merge branch 'master' into 8248862
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Added coverage testing
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Cleanups from Chris H.
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Propagate exception
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Override AbstractSplittableGenerator
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Fix extends
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Use Map.of instead of Map.ofEntries
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Changes to RandomGeneratorFactory requested by @PaulSandoz
 - Review changes
   
   @PaulSandoz and @rogermb
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Update package-info.java
 - ... and 24 more: https://git.openjdk.java.net/jdk/compare/f80c63b3...e75cc844

-------------

Changes: https://git.openjdk.java.net/jdk/pull/1292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=08
  Stats: 13525 lines in 26 files changed: 11366 ins; 2040 del; 119 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v11]

Jim Laskey-3
In reply to this post by Jim Laskey-3
> 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 with a new target base due to a merge or a rebase. The pull request now contains 38 commits:

 - Merge branch 'master' into 8248862
 - Use annotation for properties. Add getDefault().
 - Merge branch 'master' into 8248862
 - Introduce RandomGeneratorProperties annotation
 - Merge branch 'master' into 8248862
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Added coverage testing
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Cleanups from Chris H.
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Propagate exception
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Override AbstractSplittableGenerator
 - 8248862: Implement Enhanced Pseudo-Random Number Generators
   
   Fix extends
 - ... and 28 more: https://git.openjdk.java.net/jdk/compare/7e01bc96...cb78fa11

-------------

Changes: https://git.openjdk.java.net/jdk/pull/1292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1292&range=10
  Stats: 13205 lines in 26 files changed: 11043 ins; 2046 del; 116 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v9]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Tue, 5 Jan 2021 02:43:08 GMT, Brett Okken <[hidden email]> wrote:

>> Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 34 commits:
>>
>>  - Merge branch 'master' into 8248862
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Added coverage testing
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Cleanups from Chris H.
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Propagate exception
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Override AbstractSplittableGenerator
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Fix extends
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Use Map.of instead of Map.ofEntries
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Changes to RandomGeneratorFactory requested by @PaulSandoz
>>  - Review changes
>>    
>>    @PaulSandoz and @rogermb
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Update package-info.java
>>  - ... and 24 more: https://git.openjdk.java.net/jdk/compare/f80c63b3...e75cc844
>
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 439:
>
>> 437:      * Fills a user-supplied byte array with generated byte values
>> 438:      * (pseudo)randomly chosen uniformly from the range of values between -128
>> 439:      * (inclusive) and 255 (inclusive).
>
> Should this be
> between -128 (inclusive) and **127** (inclusive)

Thanks. Will update.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Wed, 6 Jan 2021 15:31:32 GMT, Jim Laskey <[hidden email]> wrote:

>> I kind of like the idea - not sure how expressive a BigInteger string is though. I might be able to express as <i, j, k>  BigInteger.ONE.shiftLeft(i).subtract(j).shiftLeft(k). Will ponder.
>
> Done

Also added getDefault and getDefaultFactory to RandomGenerato.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

Brett Okken-2
In reply to this post by Rémi Forax
On Wed, 25 Nov 2020 14:07:04 GMT, Rémi Forax <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8248862: Implement Enhanced Pseudo-Random Number Generators
>>  
>>   Changes to RandomGeneratorFactory requested by @PaulSandoz
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 335:
>
>> 333:                         ctorBytes = tmpCtorBytes;
>> 334:                         ctorLong = tmpCtorLong;
>> 335:                         ctor = tmpCtor;
>
> This one is a volatile write, so the idea is that ctorBytes and ctorLong does not need to be volatile too, which i think is not true if there is a code somewhere that uses ctorBytes or ctorLong without reading ctor.
> This code is too smart for me, i'm pretty sure it is wrong too on non TSO cpu.

The 2 uses call ensureConstructors, which calls this method, which reads ctor.
The memory model guarantees this to be safe, even on non tso hardware.
https://shipilev.net/blog/2016/close-encounters-of-jmm-kind/#pitfall-avoiding

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v12]

Jim Laskey-3
In reply to this post by Jim Laskey-3
On Wed, 6 Jan 2021 16:09:25 GMT, Brett Okken <[hidden email]> wrote:

>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Correct range used by nextBytes
>
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 147:
>
>> 145:      *
>> 146:      * @implNote  Since algorithms will improve over time, there is no
>> 147:      * guarantee that this method will return the same algorithm each time.
>
> is there a guarantee that within a running jvm each invocation will return the same algorithm?

In practice terms yes. The point is that the user should not assume they will be getting the same algorithm each time.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1292
123