> On 18 Apr 2016, at 14:35, Tagir F. Valeev <amae...@gmail.com> wrote:
>
> Hello!
>
> Thank you for review!
>
> PS> 913 UnorderedSliceSpliterator(T_SPLITR s, long skip, long limit)
> {
> PS> 914 this.s = s;
> PS> 915 this.unlimited = limit < 0;
> PS> 916 this.skipThreshold = limit >= 0 ? limit : 0;
> PS> 917 this.chunkSize = limit >= 0 ? (int)Math.min(CHUNK_SIZE,
> PS> 918 (skip + limit) /
> PS> ForkJoinPool.getCommonPoolParallelism() + 1) : CHUNK_SIZE;
> PS> 919 this.permits = new AtomicLong(limit >= 0 ? skip + limit
> : skip);
> PS> 920 }
> PS> 921
>
> PS> Note the common pool parallelism can never be 0. I dunno if you
> PS> added 1 for that or another reason.
>
> It's actually
> ((skip + limit) / ForkJoinPool.getCommonPoolParallelism()) + 1
> Not
> (skip + limit) / (ForkJoinPool.getCommonPoolParallelism() + 1)
>
> Probably I should add explicit parentheses to make this clear. One is
> added exactly to make chunkSize at least 1.
>
Doh. I think i need glasses…
A comment on the range might help too.
> PS> Did you consider:
>
> PS> (skip + limit) / AbstractTask.LEAF_TARGET
>
> PS> ?
>
> It should not make drastic changes in my test, but I will try.
>
Ok.
> PS> What if chunkSize is zero? should it be a minimum of 1?
>
> PS> Testing wise i think our existing tests cover things ok.
>
> PS> Performance-wise looks good. Probable primes are my favourite way
> PS> of easily increasing Q (cost per op) :-)
>
> PS> Can you run the stream tests and the perf tests with parallelism disabled:
>
> PS> -Djava.util.concurrent.ForkJoinPool.common.parallelism=1
>
> Ok. I think I should also test the performance for some high-N low-Q
> task to check whether it not degrades. Will perform all the tests
> later this week.
>
> By the way is these some special place to commit/store JMH tests
> (except CodeReview server), so they could be reused later?
>
Not that i am aware of. The best thing for the moment is to place ‘em on
cr.openjdk as you have done.
Paul.
