RFR 8010293 <was> Re: Potential issue with CHM.toArray

[Remi Forax]

On 09/06/2013 04:56 PM, Alan Bateman wrote:
> On 06/09/2013 12:08, Paul Sandoz wrote:
>> On Sep 2, 2013, at 3:24 PM, Paul Sandoz<Paul.Sandoz at oracle.com>  wrote:
>>
>> :
>>
>>> Here is the fix in the lambda repo which can be applied to tl:
>>>
>>> http://hg.openjdk.java.net/lambda/lambda/jdk/rev/b73937e96ae0
>>> http://hg.openjdk.java.net/lambda/lambda/jdk/raw-rev/b73937e96ae0
>>>
>> Anyone up for reviewing this?
>>
> The comments are very educational as the resizing is difficult to 
> completely grok without going through examples on a whiteboard. 
> Anyway, I don't see anything obviously wrong after going through it. 
> The test case is useful although creating the list of threads is quite 
> a mouth full to take in.
>
> -Alan.

I agree with Alan,
that the test case can be rewritten to be simpler.
List<Thread> workers = IntStream.range(0, nWorkers).
.map(w -> w * sizePerWorker)
.mapToObj(w -> (Runnable) () -> workFunction.accept(w, w + sizePerWorker))
                                                               
.map(Thread::new).collect(toList());

the first call to map is the functional way to introduce a local variable,
it's a little too much for Java, the cast to Runnable is useless if the 
creation of the thread is done at the same time,
so

List<Thread> workers = IntStream.range(0, nWorkers).
.map(w -> {
int begin = w * sizePerWorker;
int end = begin + sizePerWorker;
return new Thread(() -> workFunction.accept(begin, end));
                                                                }
.collect(toList());

or in an imperative way:
   ArrayList<Thread> workers = new ArrayList<>();
   for(int i=0; i<nWorkers; i++) {
     int begin = i * sizePerWorker;
     int end = begin + sizePerWorker;
     workers.add(new Thread(() -> workFunction.accept(begin, end)));
   }

which is not that bad :)

Also
   workers.stream().forEach(Thread::start);
(and respectively workers.stream().forEach(toArray::join); )
should be replaced by
   workers.forEach(Thread::start);
which is more efficient.

BTW the whole code can also be written using only the fluent API:
IntStream.range(0, nWorkers).
                   .map(w -> w * sizePerWorker)
                   .mapToObj(begin -> new Thread(() -> 
workFunction.accept(begin, begin + sizePerWorker)))
                   .tee(Thread::start)
                   .collect(toList())
                   .forEach(toArray::join);

sometimes I feel like Pandora.

cheers,
Rémi