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

[Rémi Forax]

On Wed, 25 Nov 2020 13:09:03 GMT, Jim Laskey <jlaskey at openjdk.org> 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 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.

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 !

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

> 173:         if (props == null) {
> 174:             synchronized (provider) {
> 175:                 if (properties == null) {

same issue with the double check locking

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

> 175:                 if (properties == null) {
> 176:                     try {
> 177:                         Method getProperties = provider.type().getDeclaredMethod("getProperties");

Is it not a better way than using reflection here ?

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

> 178:                         PrivilegedExceptionAction<Map<RandomGeneratorProperty, Object>> getAction = () -> {
> 179:                             getProperties.setAccessible(true);
> 180:                             return (Map<RandomGeneratorProperty, Object>)getProperties.invoke(null);

Given that we have no control over the fact that the method properties() doesn't return a Map of something else, this unsafe cast seems dangerous (from the type system POV).

Having a type representing a reified version of the Map may be a better idea

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

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