Question

I'm working on this project and I want to utilize multi-thread in my code. So I developed this little piece of code and tested it but it turned out that it only uses one of the threads in my computer. Can someone please tell me what's wrong with it and how I can improve it?

public static int choiceCount(List<Character> charlist) throws InterruptedException, ExecutionException {

    int coreCount = 8;
    ExecutorService e1 = Executors.newFixedThreadPool(coreCount);
    Integer total = 0;
    for (int i = 0; i < coreCount; i++) {
        Future<Integer> result = e1.submit(new Count(coreCount, i, charlist));
        total += result.get();
    }
    e1.shutdown();
    return total;
}

And here's the Callable

class Count implements Callable<Integer> {
//where the processing code is
}

So when I run this program, it only uses 12.5% of my CPU which is one thread only... Ideas guys?

Thanks

Was it helpful?

Solution

The problem is in your loop:

for (int i = 0; i < coreCount; i++) {
    Future<Integer> result = e1.submit(new Count(coreCount, i, charlist));
    total += result.get();
}

What this does, is:

  • submit a calculation
  • call get() on the Future object, which waits for the calculation to finish
  • then do the next iteration of the loop

So, in each iteration, your code is waiting for the calculation to be finished before you submit the next one.

You should create two loops, one to submit the calculations, which stores all the Future objects in a collection, and then a second loop which calls get() on each of the Future objects.

OTHER TIPS

You have to save the Future object rather than wait for each one before submitting the next.

public static int choiceCount(List<Character> charlist) throws InterruptedException, ExecutionException {

    int coreCount = Runtime.getRuntime().availableProcessors();
    ExecutorService e1 = Executors.newFixedThreadPool(coreCount);
    int total = 0;
    List<Future<Integer>> futures = new ArrayList<>();
    // start all the tasks, before
    for (int i = 0; i < coreCount; i++) 
        futures.add(e1.submit(new Count(coreCount, i, charlist)));
    // notify the executor to stop when finished in case get() throws an exception
    e1.shutdown(); 
    // collecting the results.
    for (Future<Integer> future: futures)
        total += future.get();
    return total;
}

You should create a list of List<Callable<Integer>> and then use invokeAll on the executor which will start all your computation threads.

List<Callable<Integer>> callables = new ArrayList<Callable<Integer>>();
for (int i = 0; i < coreCount; i++) {
    callables.add(new Count(coreCount, i, charlist));
}
List<Future<Integer>> futures = executor.invokeAll(callables); //why is it e1?
//Then you can wait for all computations to  finish by calling
for (Future<Integer> future : futures) {
    Integer result = future.get(); //blocks for computation to finish.
    //do something with result
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top