[java] Annoying error java.util.ConcurrentModificationException

Started by
18 comments, last by CaptainJester 19 years, 8 months ago
Yes, synchronizing everything might not choke my game at this specific stage, but in the end when the renderqueue's fill up with lotsa objects, and the number of actions modifying/editing these queues grow (running on a different thread), the combine access time for the synchronous methods will probably cause a noticable setback.

It took a little extra work writing the lists as a static array, and I need to set a limit for how many objects that can enter the queues, but in the end it all turns out good. However, I'd like some feedback on the code itself. It is likely to be error-prone, and quite possibly there are several optimization tricks that can be done.

note: dont take synchronous issues in to mind right now. I didnt worry about it yet.


public class RenderList {		public final static int DEFAULT_SIZE = 1000;		private IRender[] objectList;		public int nextIndex;		public int highestIndex;				public RenderList()		{			this(DEFAULT_SIZE);		}		public RenderList(int size)		{			objectList = new IRender[size];					nextIndex=0;			highestIndex = -1;		}				/* registers an object to the list */		public void registerObject(IRender ob)		{			if(	nextIndex>=objectList.length	||				highestIndex>=objectList.length)				return;			if(nextIndex==-1 || nextIndex>highestIndex)			{				objectList[++highestIndex]=ob;				nextIndex=highestIndex+1;			}				else				{					objectList[nextIndex]=ob;					// often the next index might also be free					if(objectList[nextIndex+1]==null)					{						nextIndex++;					}					// otherwise, lets search for it.					else						nextIndex=getNextFreeIndex();				}		}		/* gets the index of an object. -1 if its not in the list */		public int memberArray(IRender ob)		{			for(int i=0;i<(highestIndex+1);i++)			{				if(objectList==null)					continue;				if(objectList.equals(ob))					return i;			}			return -1;		}		/* removes an object from the list */		public void removeObject(IRender ob)		{			int index=memberArray(ob);			if(index==-1)			{				return;			}			objectList[index]=null;			// if this index is less than nextIndex,			// nextIndex becomes this index			if(index<nextIndex || nextIndex==-1)				nextIndex=index;						// if the highest index is this index, or higher			// than this index, reduce highest index			if(highestIndex>=index)			{				while(highestIndex>=0)				{					if(objectList[highestIndex]==null)						highestIndex--;					else					{						if(nextIndex>(highestIndex+1))							nextIndex=(highestIndex+1);						break;					}				}			}		}		/* get a free index where a new object can be  inserted */		public int getNextFreeIndex()		{			for(int i=0;i<highestIndex;i++)			{				if(objectList==null)					return i;			}			return highestIndex+1;		}		/* retrieve an object from the list */		public IRender getObject(int i)		{			if(i<=objectList.length)			{				return objectList;			}			return null;		}		public void render(Graphics2D g)		{						for(int i=0;i<(highestIndex+1);i++)			{				if(objectList==null)					continue;				objectList.render(g);			}		}				public int getListsize()		{			return objectList.length;		}}
Development blog, The Omega Sector -- http://www.omegasector.org
Advertisement
You are doing a lot of work that is already done for you in Arraylist. And ArrayList is also already optimized for you. Plus when you render code iterates over the entire list, it includes empty objects, which eats up cycles. It would be better to try and work your way around the concurrency issue.

import java.util.ArrayList;public class RenderList {    private ArrayList objectList; // containes IRender objects    private ArrayList renderObjects; // containes IRender objects    public RenderList() {		objectList = new ArrayList();		renderObjects = new ArrayList();    }        /* registers an object to the list */    public void registerObject(IRender ob) {		if(!objectList.contains(ob) {			objectList.add(ob);		}    }        /* gets the index of an object. -1 if its not in the list */    public int memberArray(IRender ob) {		int result = -1;		if(objectList.contains(ob)) {			result = objectList.indexOf(ob);		}        return result;    }        /* removes an object from the list */    public void removeObject(IRender ob) {		if(objectList.contains(ob)) {			objectList.remove(ob);		}    }        /* retrieve an object from the list */    public IRender getObject(int i) {		IRender result = null;		if(objectList.size() > i) {			result = (IRender)objectList.get(i);		}        return result;    }    public void render(Graphics2D g) {		renderObjects.clear();		renderObjects.addAll(objectList);        for(Iterator it=renderObjects.iterator();itHasNext();) {			IRender temp = (IRender)it.next();			temp.render(g);        }    }    public int getListsize() {        return objectList.size();    }}[\source]
"None of us learn in a vacuum; we all stand on the shoulders of giants such as Wirth and Knuth and thousands of others. Lend your shoulders to building the future!" - Michael Abrash[JavaGaming.org][The Java Tutorial][Slick][LWJGL][LWJGL Tutorials for NeHe][LWJGL Wiki][jMonkey Engine]
Thanks for the valiant attempt, but I still get the concurrent exception...

And btw, my list never iterates over the entire list, it only iterates between 0..highestIndex, and all the empty elements come after highestIndex. It is possible that gaps form, but they are always cleared up at the next tick.

I decided I hate ArrayList :(
Development blog, The Omega Sector -- http://www.omegasector.org
Quote:Original post by Addictman
Thanks for the valiant attempt, but I still get the concurrent exception...

And btw, my list never iterates over the entire list, it only iterates between 0..highestIndex, and all the empty elements come after highestIndex. It is possible that gaps form, but they are always cleared up at the next tick.

I decided I hate ArrayList :(


Your missing the point. Creating your own RenderList will avoid the ConcurrentModificationException, but you have probably not removed the problem. Instead it will manifest itself by producing other more obscure crashes later. When writing a multithreaded application you need to know all about synchronization, or you will be fucked sooner or later.

To synchronize your code you need to add the following block around every place you use the list, wich includes iterators:
synchronized (renderList) {// use renderList, or its iterator here}


Also be carefull when you iterate the list. You can not modify the original list when using it's iterator. Maybe you should use the get function instead.

Good luck!
I remember having this error. I think I treated it in the same way you did, but I know that it's not a good solution.

Maybe you could just queue up changes and carry them out between iterations?
Can you supply your full source code? I would like to see it in action and if I can get it to work.
"None of us learn in a vacuum; we all stand on the shoulders of giants such as Wirth and Knuth and thousands of others. Lend your shoulders to building the future!" - Michael Abrash[JavaGaming.org][The Java Tutorial][Slick][LWJGL][LWJGL Tutorials for NeHe][LWJGL Wiki][jMonkey Engine]
Quote:Original post by CaptainJester
Yes, except that synchronizing the ArrayList can cause a big performance hit.


That's a rather generic statement. The performance hit from using a synchronized list is highly app dependant, and you have to be making heavy use of the list for the hit to be 'big'. This is a case of 'use what works until profiling says otherwise'.
Quote:Original post by Addictman
Thanks for the valiant attempt, but I still get the concurrent exception...

And btw, my list never iterates over the entire list, it only iterates between 0..highestIndex, and all the empty elements come after highestIndex. It is possible that gaps form, but they are always cleared up at the next tick.

I decided I hate ArrayList :(


Sounds like perhaps you chose the wrong data structure.

Just your description there suggests that a plain old linkedlist could be easier and faster for you (although that won't remove your bugs in your program that are causing CME's)

Why did you choose an AL in the first place? Was it because you didn't know any others?

redmilamber
Quote:Original post by Aldacron
Quote:Original post by CaptainJester
Yes, except that synchronizing the ArrayList can cause a big performance hit.


That's a rather generic statement. The performance hit from using a synchronized list is highly app dependant, and you have to be making heavy use of the list for the hit to be 'big'. This is a case of 'use what works until profiling says otherwise'.


There's a thread on this around here where I make the same point "In general, synchronized data structures offer no noticeable penalties these days" and there's a microbenchmark which demonstrates something like a 10% performance difference between synch and non-synch Collections classes.

http://gamedev.net/community/forums/topic.asp?topic_id=263837

redmilamber
Quote:Original post by Aldacron
Quote:Original post by CaptainJester
Yes, except that synchronizing the ArrayList can cause a big performance hit.


That's a rather generic statement. The performance hit from using a synchronized list is highly app dependant, and you have to be making heavy use of the list for the hit to be 'big'. This is a case of 'use what works until profiling says otherwise'.


That is why I said CAN CAUSE.
"None of us learn in a vacuum; we all stand on the shoulders of giants such as Wirth and Knuth and thousands of others. Lend your shoulders to building the future!" - Michael Abrash[JavaGaming.org][The Java Tutorial][Slick][LWJGL][LWJGL Tutorials for NeHe][LWJGL Wiki][jMonkey Engine]

This topic is closed to new replies.

Advertisement