RFR: 8346107: Generators: testing utility for random value generation [v17]

[Christian Hagedorn]

On Tue, 14 Jan 2025 15:35:07 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.
>
> Theo Weidmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove unnecessary type postfixes

Great work! This will be a nice improvement for writing tests with randomized values.

I have a few comments but otherwise, it looks good to me, too.

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

> 279:      * values as powers of two.
> 280:      */
> 281:     public RestrictableGenerator<Integer> specialInts(int range) {

IIUC, this will generate MIN and MAX and values around them but only with `range = 31`. Would it make sense to have a separate method that creates a generator with values including MIN, MAX, zero and values around them? We could also combine them with some power of twos. When calling this new method `specialInts`, we could rename this method to `powerOfTwos` or something like that. This might make it easier to understand the intent.

Same for longs.

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

> 394: 
> 395:     /**
> 396:      * Returns a mixed generator that mixes the provided background generator and SPECIAL_DOUBLES with the provided

Suggestion:

     * Returns a mixed generator that mixes the provided background generator and {@link #SPECIAL_DOUBLES} with the provided

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

> 402: 
> 403:     /**
> 404:      * Returns a restrictable mixed generator that mixes the provided background generator and SPECIAL_DOUBLES with the provided

Suggestion:

     * Returns a restrictable mixed generator that mixes the provided background generator and {@link #SPECIAL_DOUBLES} with the provided

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

> 426: 
> 427:     /**
> 428:      * Returns a mixed generator that mixes the provided background generator and SPECIAL_FLOATS with the provided

Suggestion:

     * Returns a mixed generator that mixes the provided background generator and {@link #SPECIAL_FLOATS} with the provided

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

> 434: 
> 435:     /**
> 436:      * Returns a restrictable mixed generator that mixes the provided background generator and SPECIAL_FLOATS with the provided

Suggestion:

     * Returns a restrictable mixed generator that mixes the provided background generator and {@link #SPECIAL_FLOATS} with the provided

test/hotspot/jtreg/compiler/lib/generators/RandomnessSourceAdapter.java line 32:

> 30:  * See RandomnessSource for more information.
> 31:  */
> 32: public class RandomnessSourceAdapter implements RandomnessSource {

Is this a class a user must be aware of or could it also be package-private?

test/hotspot/jtreg/testlibrary_tests/generators/tests/ExampleTest.java line 39:

> 37: 
> 38: 
> 39: public class ExampleTest {

Might be good to have more concrete examples how to use the various generators to then point users to them. But could be something for a README later at some point. Either way, should not block this PR.

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

> 44: 
> 45: 
> 46: public class TestGenerators {

Nice tests :-)

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

PR Review: https://git.openjdk.org/jdk/pull/22941#pullrequestreview-2549946899
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915097680
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915127893
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915128556
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915124918
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915125535
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915137463
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915163951
PR Review Comment: https://git.openjdk.org/jdk/pull/22941#discussion_r1915164468