1

I am running a very heavy process under an anonymous SwingWorker thread. In the meantime, I'm reporting progress to the GUI using a progress bar. However, Swing threading is doing me in. It's simply not updating anything in time. I'm not sure how to do it, as I've tried updating the GUI from the SwingWorker thread, and outside, and both refuse to work.

How can I reliably update the Swing UI while a heavy worker thread is running?

Things I've tried

This does not work (with or without wrapping in the invokeLater command).

new LocalCompressor(compressor).execute();

while (!compressionDone) {
    SwingUtilities.invokeLater(new Runnable() {
    
        @Override
        public void run() {
            int percent = compressor.getPercentDone();
            progressBar.setValue(percent);
            statusLabel.setText(percent);
        }
    });
}

Additionally, attempting to update the UI from a concurrent measuring thread does not work:

class LocalCompressor extends SwingWorker<Void, Void> {

    // [...]
    
    public LocalCompressor(Compressor compressor) {
        this.compressor = compressor;
        
        // [...]
    }
    
    @Override
        protected Void doInBackground() {
        
            final Thread t1 = new Thread(new Runnable() {
            
                @Override 
                public void run(){
                    compressor.compress();
                }
            });
            
            final Thread t2 = new Thread(new Runnable() {
            
                @Override
                public void run() {
                
                    t1.start();
                    
                    while (t1.isAlive()) {
                        updateUI(compressor.getPercentDone());
                    }
                }
            
            });
            
            t2.start();
            
            return null;
        }
        
        // [...]
}
Community
  • 1
  • 1
Redandwhite
  • 2,529
  • 4
  • 25
  • 43

5 Answers5

2

You're not really using your SwingWorker. The worker already is a Thread for itself. If you have the possibility to put your long running code into the doInBackground(), put it there. Then just call publish(Integer) with your actual progress and process the chunks you get in the process(List<Integer>)-method. In process() you can update the gui, it's on the EDT.

EDIT: Actually, what you're doing right now is polling in several-while loops, this is kinda power-consuming. That's why I think its better to you events in your algorithm, everytime you got a percent or everytime the loop starts a new round or something like that.

Zhedar
  • 3,480
  • 1
  • 21
  • 44
  • The polling idea isn't a bad one, but I'd at least have a yield or sleep in the listener/consumer thread to help reduce the overhead, IMHO – MadProgrammer Oct 03 '12 at 19:46
  • Well my understanding of polling was an uninterrupted calling of methods. I agree with you, that a sleep(1000) or something like that would be pretty ok. Maybe I misdescribed that :) – Zhedar Oct 03 '12 at 19:49
  • Sorry, my bad, I should take more time to re-read before commenting :P – MadProgrammer Oct 03 '12 at 19:54
1

Expanding on the answers and advice provided here already, here is one way to code it. I'm assuming the compressor itself has no ability to do callbacks but you can ask it for the percent done.

Within the swingworker thread (doInBackground) we start the real compression thread. Then start a polling loop in the background thread, to update the UI a few times a second. To notify the UI thread, call publish. This will cause the overridden method process to be called periodially in the event thread. From here we can safely update the progress bar and status label.

public class LocalCompressor extends SwingWorker<Void, Integer>
{
   private Compressor compressor;

   public LocalCompressor(Compressor compressor)
   {
      this.compressor = compressor;

      // [...]
   }

   @Override
   protected void done()
   {
      System.out.println("Compression is done.  Going to do something with it...");
   }

   @Override
   protected void process(List<Integer> chunks)
   {
      for (Integer percent : chunks)
      {
         progressBar.setValue(percent);
         statusLabel.setText(percent);      
      }
   }

   @Override
   protected Void doInBackground() throws Exception
   {
      final Thread t1 = new Thread(new Runnable()
      {
         @Override
         public void run()
         {
            compressor.compress();
         }
      });

      t1.start();

      while (t1.isAlive())
      {
         int percentDone = compressor.getPercentDone();
         publish(percentDone);
         Thread.sleep(200);
      }
      return null;
   }
}
Guido Simone
  • 7,912
  • 2
  • 19
  • 21
  • This answer won't work for me. It's exactly the same result as before -- I can't update the progress. – Redandwhite Oct 03 '12 at 20:42
  • Additionally -- the `chunks` are actually received by the process method when the `doInBackground()` method returns... – Redandwhite Oct 03 '12 at 20:44
  • Hard to diagnose without more context. Well ... did you get rid of that while loop after you call new LocalCompressor(compressor).execute()? It is not necessary. "process" is supposed to handle all the UI updates. – Guido Simone Oct 03 '12 at 21:16
  • I'm using it simply to prevent the next lines from executing, as they require a result from the worker. Do you of any other ways I can prevent execution of the next few lines? This answer feels like it was the closest yet... – Redandwhite Oct 03 '12 at 21:44
  • You need to rethink the design. You are calling execute and the while loop in the event thread itself. Therefore process can never execute until your while loop completes. You are hung. I'll update with sample with how to add code to execute when compression is complete. You need to override the "done" method. – Guido Simone Oct 03 '12 at 21:48
  • See above sample. Get rid of the while loop. Whatever code was waiting for compression to be done should be put in the "done" override of LocalCompressor. You may need to redesign some stuff so that code inside LocalCompressor can do whatever it needs to do. – Guido Simone Oct 03 '12 at 21:53
  • You are not supposed to spinn of new threads inside doInBackground method. You are already in a separate thread. Take a look at the tutorial [concurrency in Swing](https://docs.oracle.com/javase/tutorial/uiswing/concurrency/). – Avec Mar 17 '18 at 09:16
1

Did you try the very simple and basic way of using a SwingWorker? Like @Zhedar previously said, a SwingWorker already is a Thread for itself. So remove both your inner threads (t1, t2) and just use your time-consuming compress() method in doInBackground().

Something very basic like the following:

class LocalCompressor extends SwingWorker<Void, Integer> {

    // .....
    // Your constructor here
    // .....

    @Override
    protected Void doInBackground() throws Exception {
        compress();
        return null;
    }

    @Override
    protected void process(List<Integer> chunks) {
        for (Integer chunk : chunks) {
            progressBar.setValue(chunk);
            statusLabel.setText(chunk);
        }
    }
}

Now this compress() method should be moved inside the SwingWorker and it must have somewhere a publish(), in your case it might be publish(getPercentDone()) or whatever.

private void compress() {

    // .....

    publish(getPercentDone());

    // .....

}

This is how things are usually done with a SwingWorker.

Rempelos
  • 1,220
  • 10
  • 18
  • Actually I think you are supposed to publish the current progress and not loop it inside process method. Like this: `progressBar.setValue(chunks.get(chunks.size() -1)); // assuming List of Integers` – Avec Mar 17 '18 at 08:53
0

I'm assuming (ya know how that goes) that the call to LocalCompressor.execute() is blocking. If that's the case, your while loop won't run until it's all done, and then you're defeating the purpose of getting a steady stream of updates on your UI.

Give this, or something similar, a shot:

    LocalCompressor comp = new LocalCompressor(compressor);

    SwingUtilities.invokeLater(new Runnable() {

        @Override
        public void run() {
            while (!compressionDone) {
                int percent = compressor.getPercentDone();
                progressBar.setValue(percent);
                statusLabel.setText(percent);
            }
        }
    });
    comp.execute();
}
David
  • 2,602
  • 1
  • 18
  • 32
0

You could employee a producer/consumer pattern...

Here's a really basic concept...

public class ProducerComsumer {

    public static void main(String[] args) {
        new ProducerComsumer();
    }

    public ProducerComsumer() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {

                JPanel panel = new JPanel(new GridBagLayout());
                panel.setBorder(new EmptyBorder(12, 12, 12, 12));

                JProgressBar progressBar = new JProgressBar();
                panel.add(progressBar);

                JFrame frame = new JFrame();
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setLayout(new BorderLayout());
                frame.add(panel);
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);

                Producer producer = new Producer();
                producer.start();

                Consumer consumer = new Consumer(producer, progressBar);
                consumer.start();
            }
        });
    }

    public class Producer extends Thread {

        private volatile float progress;
        private volatile boolean done;

        public Producer() {
            setPriority(NORM_PRIORITY - 1);
            setDaemon(true);
        }

        public float getProgress() {
            return progress;
        }

        public boolean isDone() {
            return done;
        }

        @Override
        public void run() {
            done = false;
            for (int index = 0; index < Integer.MAX_VALUE; index++) {
                progress = (float) index / (float) Integer.MAX_VALUE;
            }
            done = true;
            System.out.println("All done...");
        }
    }

    public class Consumer extends Thread {

        private Producer producer;
        private JProgressBar progressBar;

        public Consumer(Producer producer, JProgressBar progressBar) {
            setDaemon(true);
            setPriority(NORM_PRIORITY - 1);
            this.producer = producer;
            this.progressBar = progressBar;
        }

        public JProgressBar getProgressBar() {
            return progressBar;
        }

        public Producer getProducer() {
            return producer;
        }

        @Override
        public void run() {
            while (!producer.isDone()) {
                updateProgress();
                try {
                    sleep(1000);
                } catch (InterruptedException ex) {
                    Logger.getLogger(ProducerComsumer.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
            updateProgress();
        }

        protected void updateProgress() {
            SwingUtilities.invokeLater(new Runnable() {
                @Override
                public void run() {
                    int progress = Math.round(getProducer().getProgress() * 100f);
                    System.out.println("Update progress to " + progress);
                    getProgressBar().setValue(progress);
                }
            });
        }
    }
}

Have a play around with the Thread.setPriority values and see if it makes any difference

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • Unfortunately, neither of the solution work. My EDT remains unresponsibe. Neither the producer/consumer pattern, nor the priority setting (which as I understand is a hint that the compiler/environment can freely ignore) work. – Redandwhite Oct 03 '12 at 21:14
  • I was about to put a caveat to the answer, if you compressor is consuming all the CPU, there simply may not be anything you can do, short of executing the compressor in a separate process and us the underlying OS's priority features. – MadProgrammer Oct 03 '12 at 21:15
  • From the [Thread Java Docs](http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html) *Every thread has a priority. Threads with higher priority are executed in preference to threads with lower priority*. While I can't find any evidence either way to support either claim :P...BUT, my previous comment would still apply. You consume all the CPU, nothing is going to get time to run (well) – MadProgrammer Oct 03 '12 at 21:30
  • I refactored and moved around my code, and this answer is the closest the got me to a workable solution. Thanks! – Redandwhite Oct 08 '12 at 12:32
  • For future readeres trying to solve threading in Swing. Take a look at the dokumentation [concurrency in Swing] (https://docs.oracle.com/javase/tutorial/uiswing/concurrency/). It is explaining it well. – Avec Mar 17 '18 at 09:01