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

[Jim Laskey]

On Tue, 17 Nov 2020 21:22:28 GMT, Paul Sandoz <psandoz at openjdk.org> 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 40 commits:
>> 
>>  - Merge branch 'master' into 8248862
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Update package-info.java
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Updated RandomGeneratorFactory javadoc.
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Updated documentation for RandomGeneratorFactory.
>>  - Merge branch 'master' into 8248862
>>  - Merge branch 'master' into 8248862
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Move RandomGeneratorProperty
>>  - Merge branch 'master' into 8248862
>>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>>    
>>    Clear up javadoc
>>  - 8248862; Implement Enhanced Pseudo-Random Number Generators
>>    
>>    remove RandomGeneratorProperty from API
>>  - ... and 30 more: https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68
>
> src/java.base/share/classes/java/util/Random.java line 592:
> 
>> 590: 
>> 591:     @Override
>> 592:     public Spliterator.OfInt makeIntsSpliterator(long index, long fence, int origin, int bound) {
> 
> Unsure if this and the other two methods are intended to be public or not, since they are at the end of the class and override methods of a module private class. In principle there is nothing wrong with such `Spliterator` factories, but wonder if they are really needed given the `Stream` returning methods. The arrangement of classes makes it awkward to hide these methods.

Re the properties general comment: I moved properties to RandomSupport based on the notion that the SPI work with come later.

Re makeIntsSpliterator: These methods aren't exposed in the java.util.Random API  I guess no harm done. The only solution I can think of is to create an intermediate implementor, but that leaves the methods exposed as well.

> src/java.base/share/classes/java/util/SplittableRandom.java line 171:
> 
>> 169:      * RandomGenerator properties.
>> 170:      */
>> 171:     static Map<RandomGeneratorProperty, Object> getProperties() {
> 
> With records exiting preview in 16 this map of properties could i think be represented as a record instance, with better type safety, where `RandomSupport.RandomGeneratorProperty` enum values become typed fields declared on the record class. Something to consider after integration perhaps?

Yes.

> src/java.base/share/classes/java/util/SplittableRandom.java line 211:
> 
>> 209:      * http://zimbry.blogspot.com/2011/09/better-bit-mixing-improving-on.html
>> 210:      */
>> 211:     private static long mix64(long z) {
> 
> Usages be replaced with calls to `RandomSupport.mixStafford13`?

We were careful to not change the sequences (from fixed seed) generated by existing prngs. This was an edge case.

> src/java.base/share/classes/module-info.java line 250:
> 
>> 248:     exports jdk.internal.util.xml.impl to
>> 249:         jdk.jfr;
>> 250:     exports jdk.internal.util.random;
> 
> Unqualified export, should this be `to jdk.random`?

I guess you are right. Until we have a defined SPI we should restrict.

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

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