> On 29 Jan 2016, at 13:43, Hamlin Li <[email protected]> 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.
