How to build a very simple multithreaded pipeline in Java?

Question

I am trying to run some filters on a matrix within a multithreaded pipeline to save some time. Sadly, there are appearent synchronization issues which I fail to fully grasp yet.

What I'm expecting is that I can input a matrix to my step() function and receive a matrix that has all filters on it (delayed by the pipeline lenght).

    volatile private short[][] stage1matrix, stage2matrix;

    public short[][] step(short[][] matrix) {

        short[][] res = stage2matrix; // stage matrix with all applied filters for output
        stage2matrix = stage1matrix; // take matrix with applied stage 1 filters for stage 2
        stage1matrix = matrix; // stage input matrix for stage 1 filters

        Thread stage2 = new Thread(() -> {
            applyArtifactFilter(stage2matrix);
        });
        stage2.setPriority(10);
        stage2.start();

        Thread stage1 = new Thread(() -> {
            applySaltFilter(stage1matrix);
            applyLineFilter(stage1matrix);
            applyMedFilter(stage1matrix);
            applyPreArtifactFilter(stage1matrix);
        });
        stage1.start();

        try {
            stage1.join();
            stage2.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        return res; // output matrix with all applied filters
    }
}

Also, for some additional (and maybe unnecessary) information, I'm using paralellism in the applyMedFilter in a maybe abusive way (I don't really think that this causes the problem but...):

        ArrayList<Integer> rowIndices = new ArrayList<Integer>();
        for (int i = kernel; i < matrix.length + kernel; i++) {
            rowIndices.add(i);
        }
        //   Move window through all elements of the image
        rowIndices.parallelStream().forEach(i -> {
            for (int j = kernel; j < matrix[0].length + kernel; j++) {
                // do some rowwise independent work in parallel
                   doSomeThingWith(matrix[i][j]);
            }
        });
    }

What is happening instead that I irregularly get a matrix from the function that has only some of the filters applied; especially the applyMedFilter was sometimes not applied which shouldn't be possible to happen. The fact that sometimes, everything works right and sometimes not leads me to the conclusion that this might be a cache coherency problem. Also, commenting out the applyArtifactFilter seems to resolve the problem.

Why is this not working as I am expecting and how could I do better?

You seem to have 2 threads modifying the same matrix at the same time. That's bound to cause problems. — Kayaman, Nov 12 '19 at 10:14
...or do you. It's hard to follow since you have 4 different matrix variables, and you seem to be assigning them around weirdly. Also what's with the useless null checks in the threads? — Kayaman, Nov 12 '19 at 10:16
...and lastly, remember that a `volatile` array of variables is not the same as an array of `volatile` variables. — Kayaman, Nov 12 '19 at 10:17
Hopefully not! The intended workflow is: res = stage2matrix -> stage2matrix = filtered(stage1matrix)-> stage1matrix = filtered(inputmatrix) Which should make it a pipeline, or not? Also, in my mental model volatile shouldn't even be necessary, I just added it for safety I suppose :/ — fortuneNext, Nov 12 '19 at 10:22
Well, those assignments are my whole pipelining concept - hard to ask the question without them, maybe that's even the bug! But it's just really 3 assignments - rotating all matrices to the next stage, outputting the last one. Any suggestions for a better structure? — fortuneNext, Nov 12 '19 at 10:56
Well I would group all the assignments together in the beginning for one. The lone assignment between the threads makes it seem odd, as if it *needs* to be there. — Kayaman, Nov 12 '19 at 11:11
Done! :) Reason for that was that I wanted stage2 to begin working as soon as possible (critical path) but I guess it's quite unnecessary at this extent. — fortuneNext, Nov 12 '19 at 11:14
The assignment will take zero time compared to the time it takes to create a new thread. Setting thread priority to 10 won't have any effect either. But that's not important right now since the code doesn't work correctly. I assume the filters only work with their input? (I also understood why you had the null checks now). — Kayaman, Nov 12 '19 at 11:19
Yes, the filters only work on their input, no external data involved. Some create a local copy of the matrix work on that and write to original matrix afterwards if that's of any relevance. [Won't giving your critical path priority ensure that there is always a CPU working on it?] — fortuneNext, Nov 12 '19 at 11:24
You're concentrating too much on performance and too little on correctness. It's easy to write fast multithreaded code that doesn't work. You aren't even using pooled threads, so critical paths should be that last thing in your mind. Your explanation of "filters only work on their input...Some create a local copy of the matrix work on that and write to original matrix afterwards" sounds suspicious too, and that's probably where your problem lies. — Kayaman, Nov 12 '19 at 11:27
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/202201/discussion-between-fortunenext-and-kayaman). — fortuneNext, Nov 12 '19 at 11:36
You are aware that your assignments do not produce new matrices, but instead just make all three variables reference the same `matrix`? — Norwæ, Nov 12 '19 at 12:50
Well how I see it, if I pass a new matrix as input for the step method every time, all three variables should hold different matrices? — fortuneNext, Nov 12 '19 at 13:03

score 0 · Accepted Answer · answered Nov 12 '19 at 13:21

0

The posted code can work correctly; the mistake was the input for the function which was the same matrix every time instead of a copy, so all variables refered to the same.

answered Nov 12 '19 at 13:21

fortuneNext

109
1
14

How to build a very simple multithreaded pipeline in Java?

1 Answers1