Multithread the variant of matrix multiplication

This is matrix multiplication. Big matrix split on the submatrix and multiplication in parallel, result matrices merge to result. How i can make this code better?

```
public class MatrixMultiplication implements Callable {
private static int LENGTH_OF_SIDE = 1000;
private int taskCount = 4;
private int[][] matrixA;
private int[][] matrixB;

public MatrixMultiplication(int[][] matrixA, int[][] matrixB) {
this.matrixA = matrixA;
this.matrixB = matrixB;
}

public void setTaskCount(int taskCount) {
this.taskCount = taskCount;
}

@Override
public int[][] call() {
int n = matrixA.length;
int m = matrixA[0].length;
int p = matrixB[0].length;
try {
if (n >= LENGTH_OF_SIDE || m >= LENGTH_OF_SIDE || p >= LENGTH_OF_SIDE) {
return splitAndMerge(matrixA, matrixB);
} else {
return MultiplicationMatrixSingleThread.multiplicationMatrices(matrixA, matrixB);
}
} catch (InterruptedException | ExecutionException e) {
e.printStackTrace();
return null;
}
}

public int[][] splitAndMerge(int[][] matrixA, int[][] matrixB) throws ExecutionException,
InterruptedException {
ExecutorService service = Executors.newCachedThreadPool();
int n = matrixA.length;
int m = matrixA[0].length;
int k = matrixB[0].length;
int bound = n / taskCount;
int start;
int end;
Future future;
List> matrices = new ArrayList<>();
for (int task = 0; task {
private int[][] matrixA;
private int[][] matrixB;
private int start;
private int end;

private MultiplicationSubMatrixN(int[][] matrixA, int[][] matrixB, int start, int end) {
this.matrixA = matrixA;
this.matrixB = matrixB;
this.start = start;
this.end = end;
}

@Override
public int[][] call() throws Exception {
int n = matrixA.length;
int m = matrixA[0].length;
int k = matrixB[0].length;
int[][] matrixC = new int[n / taskCoun

Let's start with the simplification changes:

-
Declare variables and fields as final wherever possible. This drastically simplifies reasoning about mutation behaviour in your code. Since matrixA and matrixB are not supposed to change for the lifetime of the Object, they should be declared final.

-
Make use of caching when it makes sense: Currently your code will always recompute the result if the call method is executed, regardless of whether it has already been or not. That's basically a waste of computing power you could prevent by just computing the result once and allowing future calls to just lookup that result.

-
Keep the scope of variables as small as possible. Double check that introducing a variable actually makes sense before doing that. The code currently declares almost all variables it uses upfront. That reminds me of old vba programs. In either case: it's easier for you (and the runtime) when variables are in the smallest scope possible

future, start and end can be declared inside the respective loop body. in call you only use n, m and k to decide whether you splitAndMerge or run the calculation on a single thread. Those variables are basically just fluff. Inline them to prevent confusion when reading the code.

-
You seem to have an aversion to whitespace. It's hard to subidivide the code into logical sections because there's no distinction between blocks. This makes grasping the code significantly harder.

There's a large possibility for simplification in your design. Currently the code attempts to break the problem into "subresult" computations by "tasks". The problem I see with this is that matrices make this harder than it needs to be. To compute the result at \$(i,j)\$ you need the left hand sides \$i\$th row and the right hand sides \$j\$th column.

Since it's a bit unclear to me how the matrices are organized in your code, I'll not get into the details, but I agree with oopexpert when he comments:


If that is the case and there is nothing special about the matrix multiplication I would follow a totally other approach. I would forget about any thread handling and formulate the problem in a way that I am able to use parallel streams

Now to make myself sufficiently clear: I'm not proposing you reformulate this as a solution using Streams. I haven't quite found a way to make that look nice. Instead I'd strive to calculate each entry independently. This will allow you to trivially control how much CPU-power you want to throw at the problem (instead of this somewhat messy solution).

I think you could have a good chance at reformulating this using a simplification. Since you only read from the input matrices and result matrix entries are fully independent of one another you can have great success enqueueing a separate Task for each entry that task only needs matrixA, matrixB as well as a row and column to perform it's duties. You can control the level of parallelism by setting a limit on how many Threads your executor pool may use.

Overall that solution strikes me as a heavy simplification over the current code, because it doesn't rely on subdividing the matrices into submatrices. That makes it much more understandable: one task for one result.

StackExchange Code Review Q#155012, answer score: 3