Java paint 'stuttering' -list concurrency

Question

One of my applications paints objects to a screen by reading an array List:

simple code summary:

@Override
public synchronized void paintComponent(Graphics g) {
  for(Object gO:paintList) {
    g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
  }
}

The problem is I add more objects every time the user clicks the mouse, so if if the user clicks fast enough, I can cause the program painting to stutter since it cannot read while it is writing (the arrayList is synchronized). What is the common practice developers use to deal with this concurrency issue?

edit: here is the code that calls the repaint:

byte ticks = 0;
    while(true) {
        currentTime = System.nanoTime();
        if(ticks == 25) {
            drawPanel.repaint();
            ticks = 0;
        } else if (ticks%5 == 0) {//if ticks is a multiple of 5 (5,10,15,...)
            drawPanel.operations();
            ticks++;
        } else if(ticks < 25) {
            ticks++;
        }
        try {
            /*
            Refer to: 'http://stackoverflow.com/questions/1036754/difference-between-wait-and-sleep'
            on differences between Thread.sleep() and wait()
            */
            wait(1);//old timings: (long)(refreshRate*1000)
        } catch (InterruptedException ex) {
            Logger.getLogger(DeathWish.class.getName()).log(Level.SEVERE, null, ex);
        }
        //Debugging
        //System.out.println(ticks);
        currentTime = System.nanoTime();

*where operations() calculates changes in 'paintable' object's properties, removes objects that meet certain conditions and adds new objects to the paint list. Logically to me, the adding and writing should be separated? I can post the operations() method, if there isn't enough information, but I'm trying not to post huge sections of code so it is easier to interpret.

Generally, I have single loop that continuously paints the screen, and another loop that handles any events (like clicking to add an object). That way, if the event handler gets stuck on something, the paint loop will continue to run without stuttering. — Jon Egeland, Oct 03 '14 at 17:03
I really doubt that a user can click faster than a lock can be obtained. Lock acquisition is around 25ns. User clicks are around 50ms. — David Ehrmann, Oct 03 '14 at 17:09
@Dimitri No the code doesn't throw any exceptions, it used to throw a concurrentModificationException but I restructured the code a while back. — Bennett Yeo, Oct 03 '14 at 17:27
Did you read my comment below @swap905? I suggested using a CopyOnWriteArrayList — Dimitri, Oct 03 '14 at 17:33
No, I missed it. I think you may have posted that suggestion while I was posting the reply to your other question. I'll look into the CopyOnWriteArrayList though. — Bennett Yeo, Oct 04 '14 at 00:19

Francois Laroche · Accepted Answer · 2014-10-06T06:12:46.670

I guess you're getting things a bit wrong with synchronization :

ArrayList is a list implemented without synchronization
A synchronized method means that only 1 Thread at a time can access the method, but the variables inside your function are not synchronized at all

What you want, is to have your list temporary synchronized.

You could do something like that :

@Override
public void paintComponent(Graphics g) {
  synchronized(paintList) {
    for(Object gO:paintList) {
      g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
    }
  }
}

and in the code adding objects yo your list do somewhat the same.

EDIT from here :

If you want to remove all the concurrency problems between the add thread and the paint Thread, here's how you could do :

in the method to add images :

public synchronized void addImage(...) {
  Something newImage = .....
  List<Something> newPaintList = new ArrayList<>(paintList.size() + 1);
  newPaintList.addAll(paintList);
  newPaintList.add(newImage);
  paintList = newPaintList;
}

And in the paint method, remove the synchronization part.

@Override
public void paintComponent(Graphics g) {
  for(Object gO:paintList) {
    g.drawImage( gO.getObjImage(), gO.getXLoc(), gO.getYLoc(), outer.getTransparent(), null);
  }
}

With this, you won't have any concurrency between the reads and the writes, since the only operation done on paintList is reads.

The addImage should be synchronized to avoid two different Threads adding images at the same time, which could make one addImage ignored.

the issue is the synchronization, the paint thread appears to be waiting while the user is spawning more items in to the list. — Bennett Yeo, Oct 04 '14 at 16:30
This add method is basically the same principal to the `CopyOnWriteArrayList` where a new copy is made every time the collection is mutated, but thanks for the concrete code. — Bennett Yeo, Oct 06 '14 at 18:20

Java paint 'stuttering' -list concurrency

1 Answers1