Managing FutureTask and ExecutorService

Question

I have many unrelated tasks to execute and I have to get the result of the task so I use the FutureTask.

I submit all the task to the thread pool and use a map to keep track of the task and the FutureTask to get the result. And two for loops are used:

ExecutorService es = Executors.newCachedThreadPool();
Map<Task, FutureTask<Boolean>> taskResultMap = new HashMap<Task, FutureTask<Boolean>>();
for (int i = 0 ; i < callerList.size(); i++)
{
 Caller caller = callerList.get(i);
 FutureTask<Boolean> futureTask = new FutureTask<Boolean>(caller);
 es.submit(futureTask);
 taskResultMap.put(caller, futureTask);
}
for (int i = 0 ; i < callerList.size(); i++)
{
 Caller caller = callerList.get(i);
 FutureTask<Boolean> task = taskResultMap.get(caller);
 System.out.println(caller.getName() + " result is " + task.get());
}

Is this a good practice about ExecutorService and FutureTask? I'm new in multi-thread programming.

Another two questions:

1. How can I get the execution time for each thread? print in caller iteself or outside caller?
2. When I invoke the FutureTask.get() method in for loop, I think the get() method may be blocked, If a time-consuming task is running, I can't get the result of the next task but to wait. How could I do this?

Answer 1

The official way to support a system where you control multiple Future instances from an ExecutorService is similar to what @ferada has suggested, you use a 'decorator pattern' to wrap the task, and track the results. But, the Java library has this system built in to the ExecutorCompletionService. Using that service you are able to feed tasks to the service, and wait for tasks to complete.

Your code as it is looks functional, and you successfully wait for all tasks to complete. The problem is that you are not waiting for them in the right order (you are waiting in submit-order, not completion-order).

Since the CompletionService returns a Future on submit, and the exact same one on completion, you can use that as a key to a timing Map... using an IdentityHashMap on the Future would be appropriate because it removes any dependency on the actual implementation of Future.

Note that you can use an 'enhanced-for' loop to get the Caller instances from the list.

Consider the following code:

ExecutorService es = Executors.newCachedThreadPool();
ExecutorCompletionService<Boolean> ecs = new ExecutorCompletionService<>(es);
Map<Future,Long> times = new IdentityHashMap<>();
Map<Future,Caller> jobs = new IdentityHashMap<>();
for (Caller caller : callerList) {
 Future<Boolean> f = ecs.submit(caller);
 times.put(f, System.currentTimeMillis());
 jobs.put(f, caller);
}
while (!times.isEmpty()) {
 Future<Boolean> future = ecs.take();
 // note, remove the future from the map.
 long time = System.currentTimeMillis() - times.remove(future);
 System.out.println(jobs.get(future) + " result is " 
 + future.get() + " in " + time + "ms");
}

One important note here, is that I would not actually print the output of the result in the loop, but just record the details. The println is probably slow, and that would make the timing results biased. I would accumulate the completion data in a different way, and print it all after all tasks are done, or, alternatively, in a different thread.

• maaartinus: "correctly implement hashCode() and equals() which are otherwise needed to be a key in a HashMap" - Not implementing them counts as "implementing correctly" and the HashMap and IdentityHashMap work exactly the same. On the opposite, implementing a correct content-based equals would stop the HashMap from doing what you want. So I'd say, your recommendation is right, but the reasoning is not.
• rolfl: @maaartinus - Future is an interface, you can't know whether the actual implementation overrides hashCode/equals, or not. Further, if it does, whether the equals() means the same as what you expect. The IdentityHashMap removes the uncertainty.
• maaartinus: Agreed, but if it does not, then everything is fine. And this is the opposite of what you wrote above. +++ If it does, then it's most probably not what you want. +++ Using IdentityHashMap is definitely right.
• rolfl: @maaartinus - it appears that what I was trying to express was not being expressed well, so I have just removed that reasoning.

Answer 2

1. If you just want to print it (a decorator on the) callee is probably the easiest way. You'll also want to print an identifier for the thread, so you can distinguish between them. If you care about overall time, do it in the caller.
2. Yes, if you need a react to individual tasks you should structure it differently (i.e. push results from the threads to a shared structure and react in the main thread, ). If you can wait till all of them have finished their work I think you can use awaitTermination on the ExecutorService.