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

[Emanuel Peter]

On Wed, 18 Dec 2024 08:09:41 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

>> 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
>
> test/hotspot/jtreg/compiler/lib/generators/LongGenerator.java line 55:
> 
>> 53:      * Fill it with longs from the generators distribution.
>> 54:      */
>> 55:     public void fill(MemorySegment ms, long lo, long hi) {
> 
> Why are the two methods above final and both `fill` are not?

Looks like an oversight.

> test/hotspot/jtreg/compiler/lib/generators/MixedIntGenerator.java line 34:
> 
>> 32:  * Mixed results between UniformIntGenerator and SpecialIntGenerator.
>> 33:  */
>> 34: public final class MixedIntGenerator extends IntGenerator {
> 
> If you make IntGenerator and LongGenerator Interfaces (you can still have the method definitions as default implementations) you could move the shared code between MixedIntGenerator and MixedLongGenerator into a common superclass MixedGenerator. (There might be other trade-offs to this approach though.)

Hmm. I know there is a bit of code duplication. Generics with primitive types would have been nice.

I don't think I want generators of different types to share too much, it would probably create some complicated dependencies that would make extending this to other types harder.

> test/hotspot/jtreg/compiler/lib/generators/SpecialDoubleGenerator.java line 32:
> 
>> 30:  * Provide a double distribution picked from a list of special values, including NaN, zero, int, etc.
>> 31:  */
>> 32: public final class SpecialDoubleGenerator extends DoubleGenerator {
> 
> Same as for MixedLong/IntGenerator: Special*Generator could share common logic in a superclass if Double/FloatGenerator are interfaces.

Again: I'm not sure if doing this with more complicated and more generic code is really easier than a bit of code duplication. Not that I'm in general an advocate for code duplication 🙈 😅

> test/hotspot/jtreg/compiler/lib/generators/SpecialDoubleGenerator.java line 56:
> 
>> 54:     private int specialCountDown;
>> 55: 
>> 56:     public SpecialDoubleGenerator(DoubleGenerator backgroundGenerator, int specialMinFrequency, int specialMaxFrequency) {
> 
> I think it would be nice to a brief description of what the specialMin/MaxFrequency parameters mean exactly. Also for the float version.

Ok, will do.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22716#discussion_r1890001444
PR Review Comment: https://git.openjdk.org/jdk/pull/22716#discussion_r1889999200
PR Review Comment: https://git.openjdk.org/jdk/pull/22716#discussion_r1890000866
PR Review Comment: https://git.openjdk.org/jdk/pull/22716#discussion_r1890001964