RFR(S) : 8243429 : use reproducible random in :vmTestbase_nsk_stress

[Igor Ignatyev]

Hi David,

> On Apr 30, 2020, at 7:34 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Igor,
> 
> On 1/05/2020 3:15 am, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev/8243429/webrev.00
>>> 82 lines changed: 41 ins; 4 del; 37 mod;
> 
> The patch files in your webrevs are not generating properly due to the use of mq. can you fix that?
unfortunately, I've already squashed all the patches together, so I was able to upload only the final version --  http://cr.openjdk.java.net/~iignatyev/8243429/webrev.00/8243429.patch
> 
>> Hi all,
>> could you please review this small patch?
>> from JBS:
>>> this subtask is to use j.t.l.Utils.getRandomInstance() as a random number generator, where applicable, in :vmTestbase_nsk_stress test group and marking the tests which make use of "randomness" with a proper k/w.
> 
> I'm a bit confused about the strategy here:
> 
> vmTestbase/nsk/stress/except/except012.java
> 
> -            Random random = new Random(0);
> +            Random random = new Random(42);
> 
> but:
> 
> vmTestbase/nsk/stress/jni/jnistress002.java
> 
> -    static Random myRandom = new Random();
> +    Random myRandom = new Random(Utils.getRandomInstance().nextLong());
> 
> why are these random instances seeded completely differently?

because they were different before my patch, except012 used a fixed seed (so it doesn't qualify to be marked w/ randomness k/w), and jnistress002 used a "random" seed; after the patch except012 still uses a fixed (but a "well-known" and easier to grep) seed value, and jnistress002 uses random, yet reproducible/configurable seed. I've explained that in my previous RFRs for hotspot tests outside of vmTestbase, e.g. https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/037828.html <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/037828.html> 

also, in jnistress002, we can't use Utils.getRandomInstance() as there would multiple threads (JNIter002) using the same RNG. we could change it to use LocalRandom, I must admit I was doing different parts of these patches on different days, and given their tediousness I might have became less consistent in my choices b/w using LocalRandom and using a "threadlocal" instance of Random. I, personally, think we need to switch to LocalRandom in all such cases, or even better to replace Utils.getRandomInstance() w/ something akin to LocalRandom, but such change is somewhat orthogonal to the series of 'use reproducible random' patches, so I'd prefer to do it separately. 

> 
> And looking at:
> 
> vmTestbase/nsk/stress/jni/gclocker/gcl001.java
> 
> you changed it to use the LocalRandom utility rather than creating its own random instance. Why was that applicable here but not in other tests?
> 
> The changes from Math.random to a local RNG from getRandomInstance() seem okay- but again why that versus LocalRandom? Do we even need LocalRandom?

(although I guess you've already figured that out from other patches, I'll answer here for completeness) 

vmTestbase/nsk/stress/jni/gclocker/gcl001.java used the same instance of RNG in multiple threads, hence it would be really reproducible if we just switched to getRandomInstance(), LocalRandom is needed exactly for that -- provide reproducible random in multi-thread use cases.

-- Igor 

> 
> Thanks,
> David
> 
>> testing: : vmTestbase_nsk_stress test group
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8243429
>> webrevs:
>> - code changes: http://cr.openjdk.java.net/~iignatyev//8243429/webrev.00.code
>>> 58 lines changed: 41 ins; 4 del; 13 mod;
>> - adding k/w: http://cr.openjdk.java.net/~iignatyev//8243429/webrev.00.kw
>>> 11 lines changed: 0 ins; 0 del; 11 mod;
>> - full: http://cr.openjdk.java.net/~iignatyev//8243429/webrev.00
>>> 82 lines changed: 41 ins; 4 del; 37 mod;
>> Thanks,
>> -- Igor