Is it a good way to use java.util.concurrent.FutureTask ?

Question

First of all, I must say that I am quite new to the API java.util.concurrent, so maybe what I am doing is completely wrong.

What do I want to do?

I have a Java application that basically runs 2 separate processing (called myFirstProcess, mySecondProcess), but these processing must be run at the same time.

So, I tried to do that:

public void startMyApplication() {
    ExecutorService executor = Executors.newFixedThreadPool(2);
    FuturTask<Object> futureOne = new FutureTask<Object>(myFirstProcess);
    FuturTask<Object> futureTwo = new FutureTask<Object>(mySecondProcess);
    executor.execute(futureOne);
    executor.execute(futureTwo);
    while (!(futureOne.isDone() && futureTwo.isDone())) {
        try {
            // I wait until both processes are finished.
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
    logger.info("Processing finished");
    executor.shutdown();
    // Do some processing on results
    ...
}

myFirstProcess and mySecondProcess are classes that implements Callable<Object>, and where all their processing is made in the call() method.

It is working quite well but I am not sure that it is the correct way to do that. Is a good way to do what I want? If not, can you give me some hints to enhance my code (and still keep it as simple as possible).

Answer 1

You'd be better off using the get() method.

futureOne.get();
futureTwo.get();

Both of which wait for notification from the thread that it finished processing, this saves you the busy-wait-with-timer you are now using which is not efficient nor elegant.

As a bonus, you have the API get(long timeout, TimeUnit unit) which allows you to define a maximum time for the thread to sleep and wait for a response, and otherwise continues running.

See the Java API for more info.

Answer 2

You may want to use a CyclicBarrier if you are interested in starting the threads at the same time, or waiting for them to finish and then do some further processing. See the javadoc for more information.

Answer 3

Yuval's solution is fine. As an alternative you can also do this:

ExecutorService executor = Executors.newFixedThreadPool();
FuturTask<Object> futureOne = new FutureTask<Object>(myFirstProcess);
FuturTask<Object> futureTwo = new FutureTask<Object>(mySecondProcess);
executor.execute(futureOne);
executor.execute(futureTwo);
executor.shutdown();
try {
  executor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
} catch (InterruptedException e) {
  // interrupted
}

What is the advantage of this approach? There's not a lot of difference really except that this way you stop the executor accepting any more tasks (you can do that the other way too). I tend to prefer this idiom to that one though.

Also, if either get() throws an exception you may end up in a part of your code that assumes both tasks are done, which might be bad.

Answer 4

The uses of FutureTask above are tolerable, but definitely not idiomatic. You're actually wrapping an extra FutureTask around the one you submitted to the ExecutorService. Your FutureTask is treated as a Runnable by the ExecutorService. Internally, it wraps your FutureTask-as-Runnable in a new FutureTask and returns it to you as a Future<?>.

Instead, you should submit your Callable<Object> instances to a CompletionService. You drop two Callables in via submit(Callable<V>), then turn around and call CompletionService#take() twice (once for each submitted Callable). Those calls will block until one and then the other submitted tasks are complete.

Given that you already have an Executor in hand, construct a new ExecutorCompletionService around it and drop your tasks in there. Don't spin and sleep waiting; CompletionService#take() will block until either one of your tasks are complete (either finished running or canceled) or the thread waiting on take() is interrupted.