Wierd delay in DefaultListModel.addElement()

[DaedalusAlpha]

Hi, first poster here :)

First let me show the code:

import javax.swing.DefaultListModel;
import javax.swing.JList;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
import javax.swing.text.BadLocationException;

public class SolarSystemSearchField extends JList implements DocumentListener {

	private SolarSystemCollection m_collection;
	private static DefaultListModel m_listModel = new DefaultListModel();
	
	/**
	 * 
	 */
	private static final long serialVersionUID = 1L;
	
	SolarSystemSearchField(SolarSystemCollection collection) {
		super(m_listModel);
		
		m_collection = collection;
		
		for (int i=0; i<m_collection.GetSize(); ++i)
			m_listModel.addElement(m_collection.systems.get(i));
	}
	
	public SolarSystem getTopmost() {
		if (m_listModel.size() > 0)
			return (SolarSystem) m_listModel.get(0);
		else
			return null;
	}

	@Override
	public void changedUpdate(DocumentEvent arg0) {
		searchForHit(getSearchString(arg0));
	}

	@Override
	public void insertUpdate(DocumentEvent arg0) {
		searchForHit(getSearchString(arg0));
	}

	@Override
	public void removeUpdate(DocumentEvent arg0) {
		searchForHit(getSearchString(arg0));
	}
	
	private String getSearchString(DocumentEvent arg0) {
		try {
			return arg0.getDocument().getText(0, arg0.getDocument().getLength());
		} catch (BadLocationException e) {
			return "";
		}
	}
	
	private void searchForHit(String searchStr) {
		m_listModel.clear();
		m_listModel.ensureCapacity(m_collection.GetSize());

		for (int i=0; i<m_collection.GetSize(); ++i) {
			SolarSystem s = m_collection.systems.get(i);
			if (s.name.toLowerCase().contains(searchStr.toLowerCase()))
				m_listModel.addElement(s);
		}
		//if (m_listModel.getSize() > 0)
		//	setSelectedIndex(0);
	}

}

This is a JList extended search list. The idea is that the user can write a couple of letters in a name (in a JTextField with an added document listener connected to this list) and this list will update with only the systems containing those letters, while the user types.

The function I want help with is the last one in the class above; searchForHit(String searchStr).

Now this usually works just as I'd like it to AS LONG AS the last two lines (that are commented out here) are removed. As soon as those are added filling the list that usually takes milliseconds now takes two minutes!

This has made me very confused as there are only around 5400 systems and there is no reason what so ever for it to take so long time. And secondly the two lines in the end should ABSOLUTELY not affect this, but they do!

Using eclipse's debug function I have noticed that each SolarSystem currently in the list model get its toString() function called in m_listModel.addElement(s);. This seems very wierd to me. I have also noticed that the extra delay is coming from that particular line in the code.

The questions:
First, I don't understand why toString() would have to be called at all? it's not called when the two last lines are removed.
Secondly, why is every system currently in the list getting accessed for each addition? I'm specifically calling ensureCapacity() on the list model, so it shouldn't even have to resize the vector internally!

I'm starting to think this is a botched up attempt from the java virtual machine to make the code more efficient.

[camickr]

I have no idea why the commented out lines would cause the problem.

However the code is not the most efficient. Everytime you add an item to the model the model notifies the view to repaint itself, so that is probably why the toString() method is invoked.

Instead of reusing the existing model, a better solution is to simply create a new model and add the items to that model. Then you just use:

list.setModel(...);

This will add the model to the view and the list will only need to be repainted once.

[DaedalusAlpha]

That did it! Thanks alot :)

I didn't know the list worked that way. But when one knows that, it's pretty obvious your solution will work better.

The way I did it, the list updated once for each added item, which naturally would make it very slow with large amounts of items as it was looping through the whole list every time something was changed. O(n!) or something. It also explains the toString() calls, as you say.

Such easy solution, but still hard to know if you're not experienced :)

(Btw, the reason it worked better with the two last lines removed was most likely due to some fancy optimizations from the virtual machine, which couldn't be made if an index in the list were to be selected at the end)

[Webuser]

for (int i=0; i<m_collection.GetSize(); ++i)

JList starts its index with 0 so write
i++
instead of
++i

[JosAH]

JList starts its index with 0 so write
i++
instead of
++i

Please study the basic semantics of a for loop first before you attempt to answer; your answer was utterly senseless.

kind regards,

Jos