HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Class managing queuing updates from another thread, Version 2

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
managingqueuingversionthreadanotherfromclassupdates

Problem

This is an updated version of: Class to manage updates coming in from another thread

Managing updates from other threads into Swing is a hard problem. In MVC design, if you don't want to have the Presenter be responsible for Thread safety, you can end up with deadlock issues, and also too many little tasks getting started; not great.

I have written a class designed to manage these updates in the view so that the Presenter is not coupled with the View's threading model. It loads them all into a queue, and then, if a new update is added, it processes the entire queue in case too many updates have come too quickly.

Changes:

  • I removed the race condition, I believe



  • Removed the need for checking for InterruptedException, and therefore a LOG in this class



  • Added a lock to ensure that all updates are properly processed



  • Removed visibility of Runnable by making it an inner class



  • changed name of setObject to postUpdate



  • Removed unnecessary constructor



  • Other minor cosmetic changes



Here's the result:

```
public abstract class SwingUpdater {
private final LinkedBlockingQueue updates = new LinkedBlockingQueue<>();;
private final AtomicBoolean updating = new AtomicBoolean(false);

public final void postUpdate(E object) {
updates.add(object);
synchronized(updating) {
if (!updating.getAndSet(true)) {
EventQueue.invokeLater(new SwingUpdaterRunnable());
}
}
}

private class SwingUpdaterRunnable implements Runnable {
@Override
public void run() {
Deque pending = new LinkedList<>();

do {
updates.drainTo(pending);

while (!pending.isEmpty()) {
doTask(pending.removeFirst());
}
} while (!atomicCheckCanStop());
}

private boolean atomicCheckCanStop() {
synchronized(updating) {
boolean isEmpty = updates.isE

Solution

Synchronization and atomic boolean is overkill. Especially since you only access the boolean inside the synchronized blocks. Choose one or the other.

Draining to a LinkedList and then removeFirst each element is less efficient than reserving enough space in a ArrayList, draining to it and iterating over it.

do {
    ArrayList pending = new ArrayList(updates.size()+3);
    updates.drainTo(pending);

    for(E task : pending){
        doTask(task);
    }

} while (!atomicCheckCanStop());


Having said all that the Java devs have solved this problem before and created sun.swing.AccumulativeRunnable which is used by SwingWorker for it's publish->process mechanism and grouping the setProgress() calls into 1 event. There they don't bother with checking the queue again after draining it.

Code Snippets

do {
    ArrayList<E> pending = new ArrayList<E>(updates.size()+3);
    updates.drainTo(pending);

    for(E task : pending){
        doTask(task);
    }

} while (!atomicCheckCanStop());

Context

StackExchange Code Review Q#103805, answer score: 4

Revisions (0)

No revisions yet.