On Mon, 13 May 2024 12:45:59 GMT, Jaikiran Pai <[email protected]> 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
