Sum of table with threads

Question

I'm trying to sum a table with threads. I'm creating a table with given length and then trying to create a sum with given name of threads.

Every thread is taking a part of table, according to his indeks.

For example:

Table with 12 elements on 3 threads:

0 thread taking [0, 3, 6, 9] elements

1 thread taking [1, 4, 7, 10] elements

2 thread taking [2, 5, 8, 11] elements

Threads are summing that numbers and then returning result. After it, I'm summing it all together, and getting result.

This is my single Callable object implementation:

public class TableSumThread implements Callable<Integer> {

private int indeks;
private int[] table;

public TableSumThread(int indeks, int[] table) {
    this.indeks = indeks;
    this.table = table;
}


@Override
public Integer call() throws Exception {
    int iter = indeks;
    int sum = 0;
    while(iter < table.length) {
        sum += table[iter];
        iter += indeks;
    }
    return sum;
}

}

This is my "Executor":

public class TableSumExecutor {
private int[] table;
private int executors;

public Integer execute() {
    ExecutorService executorService = Executors.newFixedThreadPool(executors);
    List<Future<Integer>> results = new ArrayList<Future<Integer>>(executors);

    for (int i = 0; i < executors; i++) {
        Callable<Integer> task = new TableSumThread(i, table);
        results.add(executorService.submit(task));
    }
    System.out.println("After creating all threads.");
    int suma = sum(results);
    return suma;
}

private int sum(List<Future<Integer>> results) {
    int sum = 0;
    for (int i = 0; i < results.size(); i++) {
        try {
            sum += results.get(i).get();
        } catch (InterruptedException e) {
            e.printStackTrace();
        } catch (ExecutionException e) {
            e.printStackTrace();
        }
    }
    return sum;
}

Main:

public static void main(String[] args) {
    Scanner scanner = new Scanner(System.in);
    System.out.println("Table length: ");
    int nTable = scanner.nextInt();
    System.out.println("Threads number: ");
    int nThreads = scanner.nextInt();

    Random random = new Random();
    int[] tablica = new int[nTable];
    for (int i = 0 ; i < tablica.length ; i++)
        tablica[i] = Math.abs(random.nextInt() % 9 + 1);

    TableSumExecutor tableSumExecutor = new TableSumExecutor(tablica, nThreads);
    int result = tableSumExecutor.execute();

    System.out.println("And the result is: " + result);
}

Everything is fine, threads performing all the tasks, but program is blocking on:

sum += results.get(i).get();

I'm not getting any exception, it's just blocking. I've checking it also on debugger. All task were finished, and result was waiting for final step.

I'm probably not using get() of Future type properly?

EDIT. Ok, I solved a problem. But program is not ending after all. When I'm checking after displaying result in main executorService.isShutdown() It's false. Should I terminate all threads manualy, or it should terminate automatically?

Re, "Can you maybe spot some design mistakes?" Better names, for one: Your class named `TableSumThread` does not implement a thread. It implements a _task_. A task is an object representing some piece of work that needs to be done. A _thread_ is a deep-magic system object that executes code, and an `ExecutorService` (a.k.a., a _thread pool_) is an object that uses one or more threads to _perform_ tasks. — Solomon Slow, Mar 17 '16 at 19:10
...and your `TableSumExecutor` class is not an `Executor`: It is a business-logic-y thing that _uses_ an `ExecutorService`. I would give it a name that says what it does (e.g., `TableSummer`). — Solomon Slow, Mar 17 '16 at 19:13
Your program probably will take more time to run than it would take if it did everything in one thread. Creating threads takes time, and synchronizing threads takes time. I don't know how big the array would have to be before you get an advantage from using several threads, but I'm guessing something like tens of thousands of array elements. (It would be trivially easy to use your program in an experiment to find out the answer.) — Solomon Slow, Mar 17 '16 at 19:17
Your `tableSumExecutor.execute()` method creates a new thread pool each time it is called. In a large-scale or long-running program, you normally would have only one thread pool (or maybe a few specialized thread pools) that are used throughout the life of the program. A class like your `TableSumExecutor` would take a reference to the thread pool that it should use as a constructor argument. — Solomon Slow, Mar 17 '16 at 19:21

score 1 · Accepted Answer · answered Mar 17 '16 at 18:42

1

In this line of code :

Callable<Integer> task = new TableSumThread(i, table);

in first iteration of for loop, "i" is 0. So you create TableSumThread object with indeks = 0. So in this loop :

int iter = indeks;
int sum = 0;
while(iter < table.length) {
    sum += table[iter];
    iter += indeks;
}

you don't increment iter variable and this is infinite loop. That's why your first thread never ends and blocks the execution of your main thread (since get() on future object is blocking operation). You may try to pass two variables - start index, and iteration (in your case - constant 3). Something like new TableSumThread(i, 3, table). Hope this helps.

answered Mar 17 '16 at 18:42

Nikolay Tomitov

947
5
12

Yes, It was a problem. Thank you. It's my first multithreading exercise using Callable interface. Can you maybe spot some design mistakes, thinks that could be made in different, better way? – nowszy94 Mar 17 '16 at 19:02
You may research a CompletionService, to avoid blocking on Future objects. Link here : http://stackoverflow.com/questions/4912228/when-should-i-use-a-completionservice-over-an-executorservice – Nikolay Tomitov Mar 17 '16 at 19:09

Sum of table with threads

1 Answers1