RFR: 8346107: Generators: testing utility for random value generation

[Emanuel Peter]

On Tue, 7 Jan 2025 08:58:17 GMT, Theo Weidmann <tweidmann at openjdk.org> wrote:

> This PR is a refactoring and partial rewrite of https://github.com/openjdk/jdk/pull/22716 by @eme64. The goals remain the same:
> 
>> For verification testing, it is often critical to generate "interesting" values, to provoke overflows, NaN, etc. And to generate these values in the correct distribution to trigger certain optimizations.
>> 
>> I would like to start a collection of such generators, that can then be used in testing.
>> 
>> The goal is to grow this collection in the future, and add new types. For example byte, char, short, or even Float16.
>> 
>> This will be helpful for the Template framework [JDK-8344942](https://bugs.openjdk.org/browse/JDK-8344942), but also other tests.
>> 
>> Related PR, for value verification: https://github.com/openjdk/jdk/pull/22715
> 
> The refactoring makes use of generics, rendering the generators library more flexible by default, by allowing it work with arbitrary types (with special features for Comparable types), improving the composability of different generators and streamlining the client API for simplicity. This allows test authors to quickly compose their own distributions and generators if necessary. An overview of this functionality is provided in the `Generators` javadoc.

Nice work! I have a few comments in the code.

I am also wondering if we can still do this:
- pick a random int distribution  -> suppose we get mixed special + uniform
- now sample from different int ranges, all from that same underlying distribution:
  - [0, max]
  - [10, 20]
  - ...

I think it is not possible:

gen = Generators.ints();
// gen is not resrictable, sadly... but I would like to do
gen1 = gen.restrict(0, max);
v1 = gen1.next();
gen2 = gen.restrict(10, 20);
v2 = gen2.next()


If that is indeed not possible:
How can we ensure the continuity of the distribution across different range restrictions, if we want to pick a random distribution?

test/hotspot/jtreg/compiler/lib/generators/EmptyGeneratorException.java line 29:

> 27:  * An EmptyGeneratorException is thrown if a generator configuration is requested that would result in an empty
> 28:  * set of values. For example, bounds such as [1, 0] cause an EmptyGeneratorException. Another example would be
> 29:  * restricting a uniform integer generator over the range [0, 1] to [10, 11].

What if I mix distributions, and one of them has no values from that range, but the other does?

test/hotspot/jtreg/compiler/lib/generators/Generator.java line 33:

> 31:      * Returns the next value from the stream.
> 32:      */
> 33:     T next();

Should this not have a `@return`?

Why don't you run something like:
`/oracle-work/jdk-fork0/build/linux-x64-debug/jdk/bin/javadoc -sourcepath test/hotspot/jtreg:./test/lib compiler.lib.generators`
And make sure you don't have any errors/warnings :)

test/hotspot/jtreg/compiler/lib/generators/GeneratorBase.java line 26:

> 24: package compiler.lib.generators;
> 25: 
> 26: abstract class GeneratorBase<T> implements Generator<T> {

Can you add a comment what this is for?

test/hotspot/jtreg/compiler/lib/generators/Generators.java line 39:

> 37:  * optimizations.
> 38:  * <p>
> 39:  * Normally, clients get the default Generators instance by referring to the static variable {@link #G}.

It would be nice to have an example test that uses it as you would expect.

test/hotspot/jtreg/compiler/lib/generators/Generators.java line 68:

> 66:  * </code></pre>
> 67:  * <p>
> 68:  * If there is a single value that is interesting as the to all three parameters, we might even call this

"as the to all"
Looks like a typo?

test/hotspot/jtreg/compiler/lib/generators/Generators.java line 257:

> 255:     }
> 256: 
> 257:     public Generator<Integer> mixedWithSpecialInts(int weightA, int weightB, int rangeSpecial) {

Suggestion:

    public Generator<Integer> uniformMixedWithSpecialInts(int weightA, int weightB, int rangeSpecial) {

Optional, you can also leave it as is.

test/hotspot/jtreg/compiler/lib/generators/RandomnessSource.java line 28:

> 26: /**
> 27:  * Defines the underlying randomness source used by the generators. This is essentially a subset of
> 28:  * {@link java.util.random.RandomGenerator} and the present methods have the same contract.

Why do you need this? For testing? -> Add comment about that.

test/hotspot/jtreg/testlibrary_tests/generators/tests/TestGenerators.java line 78:

> 76:         Asserts.assertEQ(g.next(), 4);
> 77:         Asserts.assertEQ(g.next(), 18);
> 78:     }

It would be nice if you told us / a future person who extends this, what this mocking does, and how it works.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22941#pullrequestreview-2534311495
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905439519
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905441799
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905442788
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905452818
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905455587
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905463832
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905473446
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1905484613