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.san...@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
