> On 29 Jan 2016, at 13:43, Hamlin Li <huaming...@oracle.com> wrote:
> 
> Hi Paul,
> 
> Sorry for delayed response, have been occupied by other higher priority task.
> Thanks for your review, I agree with you that your second approach is better.
> New webrev: http://cr.openjdk.java.net/~mli/8076458/webrev.01/
> 

The changes to the data providers look ok.

Would you mind splitting out the tests between StreamTestData<Integer> and 
StreamTestData<Integer>.small as outlined in 2) below. That way for the 
non-eplosive stuff we can still crunch on larger data without much of a slow 
down.


> 
> Below are times cost for different ops:
>  total        :169.996
>  testOps only    :108.988
>  testIntOps only :23.865
>  testLongOps only :22.326
>  testDoubleOps only :16.944
> so, I build small data providers for each of them.
> 

Ok, and i suspect/hope it drops by at least an order of magnitude with your 
changes applied.

Out of curiosity i wonder what the times would be if using parallel GC rather 
than G1.

Paul.

> Thank you
> -Hamlin
> 
> On 2016/1/26 21:18, Paul Sandoz wrote:
>> Hi Hamlin,
>> 
>> Conservatively I would prefer not to remove data sets if at all possible. It 
>> will affect all tests, and leaf tasks for parallel streams should have 
>> enough data to crunch on.
>> 
>> I suspect the problem of the flatMap test is not necessarily due to the 
>> source sizes being of 1000 elements but that there are tests that substitute 
>> an element whose value is m for n elements from 0..m, which can explode 
>> things and generate lots of garbage.
>> 
>> Have you tried executing those kinds tests when the data size is < 1000?
>> 
>> My bet is the FlatMapOpTest will run significantly faster and you will not 
>> need to split it out.
>> 
>> There are two ways we could consider doing this:
>> 
>> 1) Check the size in the test method:
>> 
>> if (data.size() < 1000) {
>>     exerciseOps(data, s -> s.flatMap(mfLt));
>>     exerciseOps(data, s -> s.flatMap(integerRangeMapper));
>>     exerciseOps(data, s -> s.flatMap((Integer e) -> IntStream.range(0, 
>> e).boxed().limit(10)));
>> }
>> 
>> 2) Include a new data provider for smaller data sets
>> 
>> @Test(dataProvider = "StreamTestData<Integer>", dataProviderClass = 
>> StreamTestDataProvider.class)
>> public void testOps(String name, TestData.OfRef<Integer> data) {
>>     Collection<Integer> result = exerciseOps(data, s -> s.flatMap(mfId));
>>     assertEquals(data.size(), result.size());
>> 
>>     result = exerciseOps(data, s -> s.flatMap(mfNull));
>>     assertEquals(0, result.size());
>> 
>>     result = exerciseOps(data, s-> s.flatMap(e -> Stream.empty()));
>>     assertEquals(0, result.size());
>> }
>> 
>> @Test(dataProvider = "StreamTestData<Integer>.small", dataProviderClass = 
>> StreamTestDataProvider.class)
>> public void testOpsX(String name, TestData.OfRef<Integer> data) {
>>     exerciseOps(data, s -> s.flatMap(mfLt));
>>     exerciseOps(data, s -> s.flatMap(integerRangeMapper));
>>     exerciseOps(data, s -> s.flatMap((Integer e) -> IntStream.range(0, 
>> e).boxed().limit(10)));
>> }
>> 
>> I prefer the latter approach (applied to ref and primitive data sets). It’s 
>> more work, but i think the right direction.
>> 
>> Paul.

Reply via email to