Why my JAVA threadpool is not saving time?

Question

I have this simple code and it does not save time. The running time is the same as using the single thread. What is wrong? Could you help? I am new to this and I hope the question does not annoy you if it is too simple. Thanks!

import java.util.Random;
import java.util.concurrent.Callable;
public class Work implements Callable<int[]>
{
    int id;
    int y;

    public Work(int i)
    {
        id=i;
    }

    public int[] doit(){
        Random rand = new Random();
        y=rand.nextInt(10);
        try {
            Thread.sleep(y*1000);                 //1000 milliseconds is one second.
        } catch(InterruptedException ex) {
            Thread.currentThread().interrupt();
            System.out.println(ex.getMessage());
        }
        int x[] = new int[2];
        x[0]=id;
        x[1]=y;

        return x;
    }


    @Override
    public int[] call() throws Exception
    {
        return doit();
    }
}
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
public class Driver
{

    ArrayList<int[]>  result=new  ArrayList<int[]>() ;
    public Driver()
    {
    }

    public void doCalc(int n)
    {

        for (int i=0; i<n; i++) {
            Work w = new Work(i);
            int[] y = w.doit();


            if (y != null) {
                result.add(y);
            }
        }
        int total=0;
        for (int i=0; i< result.size(); i++) {
            System.out.println("id = " + result.get(i)[0] + "; random number = "+ result.get(i)[1]);
            total+=result.get(i)[1];
        }
        System.out.println("total = " + total);
    }
    public void doCalcThread(int n)
    {
        ExecutorService executor = Executors.newFixedThreadPool(5);
        for (int i=0; i<n; i++) {
            Work w = new Work(i);
            Future<int[]> future =executor.submit(w);
            int[] y;
            try
            {
                y = future.get();
                if (y != null) {
                    result.add(y);
                }
            } catch (InterruptedException | ExecutionException e)
            {
                e.printStackTrace();
            }

        }
        executor.shutdown();
        while (!executor.isTerminated()) {
        }
        System.out.println("Finished all threads");

        int total=0;
        for (int i=0; i< result.size(); i++) {
            System.out.println("id = " + result.get(i)[0] + "; random number = "+ result.get(i)[1]);
            total+=result.get(i)[1];
        }
        System.out.println("total = " + total);
    }
    /**
     * @param args
     */
    public static void main(String[] args)
    {
        Driver dr = new Driver();
        Calendar cal = Calendar.getInstance();
        SimpleDateFormat sdf = new SimpleDateFormat("HH:mm:ss.SSS");
        cal = Calendar.getInstance();
        System.out.println( "before  " + sdf.format(cal.getTime()) );

        //dr.doCalc(5);
        dr.doCalcThread(5);

        cal = Calendar.getInstance();
        System.out.println( "after  " + sdf.format(cal.getTime()) );
    }
}

Answer 1

Like Svetlin Zarev mentioned your future.get() is blocking, but if you are using java 8 I would recommend looking at CompleteableFuture as you can see all of your threads to run, and use CompleteableFuture.allOf() to check that they are all completed and add the results once the done

Answer 2

future.get(); is a blocking operation that waits for the computation to complete. So basically you are starting one thread at a time in your loop -> no parallelism whatsoever. Here's the faulty code:

for (int i=0; i<n; i++) {
            Work w = new Work(i);
            Future<int[]> future =executor.submit(w);
            int[] y;
            try
            {
                y = future.get();
                if (y != null) {
                    result.add(y);
                }
            } catch (InterruptedException | ExecutionException e)
            {
                e.printStackTrace();
            }

        }

So basically you need to store in the list all Future<>s and after you've submitted all of them start to call their get() methods. This would allow the tasks to run in parallel.