Does this non-standard Java synchronization pattern work?
Asked Answered
C

1

7

Let's say I have two threads running like this:

  • Thread A which performs computation while updating pixels of a shared image
  • Thread B periodically reads the image and copies it to the screen

Thread A performs work quickly, say 1 million updates per second, so I suspect it would be a bad idea to lock and unlock on a lock/mutex/monitor that often. But if there is no lock and no way of establishing a happens-before relation from thread A to thread B, then by the Java memory model (JMM spec) thread B is not guaranteed at all to see any of A's updates to the image.

So I was thinking that the minimum solution is for threads A and B to both synchronize periodically on the same shared lock, but not actually perform any work while inside the synchronized block - this is what makes the pattern non-standard and dubious. To illustrate in half-real half-pseudo code:

class ComputationCanvas extends java.awt.Canvas {

    private Object lock = new Object();
    private int[] pixels = new int[1000000];

    public ComputationCanvas() {
        new Thread(this::runThreadA).start();
        new Thread(this::runThreadB).start();
    }

    private void runThreadA() {
        while (true) {
            for (1000 steps) {
                update pixels directly
                without synchornization
            }
            synchronized(lock) {}    // Blank
        }
    }

    private void runThreadB() {
        while (true) {
            Thread.sleep(100);
            synchronized(lock) {}    // Blank
            this.repaint();
        }
    }

    @Override
    public void paint(Graphics g) {
        g.drawImage(pixels, 0, 0);
    }
}

Does adding empty synchronization blocks in this way correctly achieve the effect of transferring data from thread A to thread B? Or is there some other solution I failed to imagine?

Commissionaire answered 30/1, 2017 at 2:22 Comment(11)
Why wouldn't an atomic boolean do the trick? Synchronizing on nothing still leaves the work out of synchronizationGoodness
Why not use volatile?Caton
Possibly related: #17109041Greaser
@Caton I would think that volatile will cause the cache to become invalidated on every write operation leading to much slower updates in thread a.Greaser
It's still possible for the canvas to read from the pixels array while it's been updated by AVilma
I don't care about "page tearing"; this is a visualization into a messy processCommissionaire
This works. An cheaper solution is for thread A to write to a volatile variable, and thread B to read from it. A full synchronization may force a thread to both invalidate its read cache and flush its write buffer. A volatile read/write only does one of the two.Chairmanship
Possible alternative: java.util.concurrent.atomic.AtomicIntegerArray.lazySet()Commissionaire
Graphics doesn’t have a drawImage method that takes an int[] array. So a fundamental step is missing in this example. Further, there are three threads involved and third one, the event dispatch thread being the one actually reading the array (if there was a drawImage method accepting int[]) is never using synchronized. Since repaint does nothing but posting some kind of event to the queue (but with some gotchas), you could get rid of the thread B and the synchronized and simply post a custom event that solves all issues.Delacruz
@Delacruz My intention is that Thread A should focus on pure computation. It should not worry about the output frame rate, copying buffers, posting events, etc.Commissionaire
That synchronized block is already outside the pure computation focus. Why should any other approach that also boils down to a single line of code be worse than that?Delacruz
P
1

Yes it works. But it works horribly.

Happens before only works when the release of the writer happens before the acquire of the reader. Your implementation assumes that whatever you're writing will complete before the subsequent reading/updating from ThreadB. Causing your data to be flushed all the time by synchronized will cause performance problems, although to what extent I cannot say for sure. Sure, you've made your synchronization finer grained, have you tested it yet?

A better solution might use a singleton/transfer SPSC (single producer/single consumer) queue to store the current snapshot of the writing thread and use that whenever you update.

int[] data = ...
Queue<int[]> queue = new ...

// Thread A
while (true) {
    for (1000 iterations or so) {
        ...
    }
    queue.add(data);
}

// Thread B
while (true) {
    int[] snapshot = queue.take(); 
    this.repaint();
}

The advantage of this is that you don't need to busywait, you can just wait for the queue to block or until the next write. You can skip writes that you don't have time to update. You don't need to depend on the arbitrary thread scheduler to plan data flushes for you.

Remember that thread-safe data structures are great for passing data between threads.

Edit: oops, forgot to say that depending on how your updates go, you might want to use an array copy to prevent your data from being garbled from random writes that aren't cached.

Prospect answered 30/1, 2017 at 5:11 Comment(7)
repaint doesn't take any parametersShayn
@DavidConrad you're right, was thinking about the paint method below.Prospect
One could also pass the pixel data through an AtomicReference, but a queue is superior for reasons you noted - no busy wait or scheduling issues. I have used this pattern many times with great success.Incommensurable
I would limit the size of the queue to two. There is no need to accumulate thousands of arrays waiting to be repainted. Just like a double bufferGoodness
@efekctive I mentioned before my answer that one can use a TransferQueue or a singleton queue.Prospect
Then it becomes expensive: a queue of one element to "avoid" synchronization. I just wanted to point out that by keeping the size small you can even put the producer thread to sleep while the other repaints. No need to waste cpu cycles in arrays that would be thrown awayGoodness
@efekctive right again, but it depends on what the author wants. You also assume that the producer is faster than the consumer, in which case there are other factors that come into play, the producer blocking might be a bad thing, who knows? But again, nothing you've said is wrong.Prospect

© 2022 - 2024 — McMap. All rights reserved.