RFR: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory

[Jaikiran Pai]

On Mon, 13 May 2024 12:45:59 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> All random number generator algorithms are implemented in module `java.base`. The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` is no longer needed.
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 190:
> 
>> 188:                 if (properties == null) {  // double-checking idiom
>> 189:                     RandomGeneratorProperties props = rgClass.getDeclaredAnnotation(RandomGeneratorProperties.class);
>> 190:                     Objects.requireNonNull(props, rgClass + " missing annotation");
> 
> Hello Raffaello, with the `RandomGenerator` implementations now all residing within the java.base module, I think an additional advantage of that is that, it will now allow us to remove this internal `RandomGeneratorProperties` annotation and thus this reflection code.
> 
> I think one way to do that would be something like this within this `RandomGeneratorFactory` class itself: 
> 
> 
> private record RandomGenEntry(Class<?> randomGenClass, int i, int j,
>                               int k, int equiDistribution, boolean stochastic,
>                               boolean hardware) {
> 
> }
> 
> private static final Map<String, RandomGenEntry> FACTORY_MAP = ... // construct the map
> 
> 
> where the `FACTORY_MAP` will be keyed to the alogrithm and the value will be a record which holds these additional details about the `RandomGenerator`. 
> This current PR is about getting rid of ServiceLoader usage. So if you want to remove the usage of this annotation and reflection is a separate PR that's fine with me. Furthermore, although I don't see the necessity of an annotation for what we are doing here, if you think that the removal of the annotation and reflection isn't worth doing, that is OK too.

Thinking a bit more, I think we can even get rid of the reflection used in `create()` methods implementation, if we wanted to, by doing something like this:


private record RandomGenEntry(Class<? extends RandomGenerator> randomGenClass, int i, int j,
                                  int k, int equiDistribution, boolean stochastic,
                                  boolean hardware) {

        RandomGenerator create() {
            String algo = randomGenClass.getSimpleName();
            return switch (algo) {
                case "Random" -> new Random();
                case "L128X1024MixRandom" -> new L128X1024MixRandom();
                case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus();
                // ... so on for the rest
                default -> throw new InternalError("should not happen");
            };
        }

        RandomGenerator create(long seed) {
            String algo = randomGenClass.getSimpleName();
            return switch (algo) {
                case "Random", "SplittableRandom", "SecureRandom" -> {
                    throw new UnsupportedOperationException("cannot construct with a long seed");
                }
                case "L128X1024MixRandom" -> new L128X1024MixRandom(seed);
                case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus(seed);
                // ... so on for the rest
                default -> throw new InternalError("should not happen");
            };
        }

        RandomGenerator create(byte[] seed) {
            String algo = randomGenClass.getSimpleName();
            return switch (algo) {
                case "Random", "SplittableRandom", "SecureRandom" -> {
                    throw new UnsupportedOperationException("cannot construct with a byte[] seed");
                }
                case "L128X1024MixRandom" -> new L128X1024MixRandom(seed);
                case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus(seed);
                // ... so on for the rest
                default -> throw new InternalError("should not happen");
            };
        }
    }

    
    private static final Map<String, RandomGenEntry> FACTORY_MAP = ... // construct the map

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1598446741