[9] Review request : JDK-8057719: Develop new tests for LambdaForm Reduction and Caching feature

[Igor Ignatyev]

Paul,

thanks for your review. I'll take care about this change, since 
Konstantin is on vacation.

updated webrev: 
http://cr.openjdk.java.net/~iignatyev/kshefov/8057719/webrev.00/

please see inline answers.

Igor

On 09/11/2014 05:12 PM, Paul Sandoz wrote:
>
> On Sep 5, 2014, at 7:52 PM, Konstantin Shefov <konstantin.shefov at oracle.com> wrote:
>
>> Hello,
>>
>> Please review the new tests for the feature "Lambda Form Reduction and Caching" https://bugs.openjdk.java.net/browse/JDK-8046703
>>
>> JBS task: https://bugs.openjdk.java.net/browse/JDK-8057719
>>
>> Webrev: http://cr.openjdk.java.net/~kshefov/8057719/webrev.00/
>>
>> These tests also depend on testlibrary change: https://bugs.openjdk.java.net/browse/JDK-8057707
>> Webrev of the testlib change: http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/
>>
>
> Generally looks good. It would be interesting see code-coverage results.
>
> Are you sure the LFMultiThreadCachingTest is sufficiently testing the thread-safety of j.l.invoke classes? It might be that more threads need to be generated (== #cores), and the test repeatedly performed in a loop to increase the chance of stuff stomping on each other. (I see you have iterations in the outer loop of runTests, that might be sufficient, but you might need a tighter loop specific to each test in LFMultiThreadCachingTest).
>
good point, I updated LFMultiThreadCachingTest to use more threads, 
however I doubt that we need to add a loop. Writing additional MT-stress 
tests, which can provide more confidence on thread-safety, is out of the 
scope of JDK-8057719.

>
> A general comment, feel free to ignore. It's easy to use EnumSet to obtain a collection of enum values:
>
>    Set<TestMethods> tms = EnumSet.allOf(TestMethods.class)
>
> and filtered:
>
>    Set<TestMethods> tms = EnumSet.complementOf(EnumSet.of(TestMethods.IDENTITY, TestMethods.CONSTANT))
>
> (Quite concise with a static import.)
>
> Change LambdaFormTestCase.runTests to accept a Collection<TestMethods> and you are good to go :-)
done
>
> Paul.
>
>
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>