Permalink
Browse files

Improve performance of iteration in the Array and Content lists.

Add separate tests for simple iterator() as well as listIterator()
instances from List implementations.
  • Loading branch information...
1 parent 16e0008 commit 412170566ebdf8449b442e44f12ed8712d447a19 @rolfl rolfl committed Oct 17, 2011
@@ -442,6 +442,10 @@ private int indexOfDuplicate(Attribute attribute) {
return duplicate;
}
+ @Override
+ public Iterator<Attribute> iterator() {
+ return new ALIterator();
+ }
/**
* Return the number of items in this list
*
@@ -459,4 +463,50 @@ public int size() {
public String toString() {
return super.toString();
}
+
+ /* * * * * * * * * * * * * ContentListIterator * * * * * * * * * * * * * * * */
+ /* * * * * * * * * * * * * ContentListIterator * * * * * * * * * * * * * * * */
+ private final class ALIterator implements Iterator<Attribute> {
+ private int expect = -1;
+ private int cursor = 0;
+ private boolean canremove = false;
+ private ALIterator() {
+ expect = modCount;
+ }
+ @Override
+ public boolean hasNext() {
+ return cursor < size;
+ }
+
+ @Override
+ public Attribute next() {
+ if (modCount != expect) {
+ throw new ConcurrentModificationException("ContentList was " +
+ "modified outside of this Iterator");
+ }
+ if (cursor >= size) {
+ throw new NoSuchElementException("Iterated beyond the end of " +
+ "the ContentList.");
+ }
+ canremove = true;
+ return elementData[cursor++];
+ }
+
+ @Override
+ public void remove() {
+ if (modCount != expect) {
+ throw new ConcurrentModificationException("ContentList was " +
+ "modified outside of this Iterator");
+ }
+ if (!canremove) {
+ throw new IllegalStateException("Can only remove() content " +
+ "after a call to next()");
+ }
+ AttributeList.this.remove(--cursor);
+ expect = modCount;
+ canremove = false;
+ }
+
+ }
+
}
@@ -465,6 +465,11 @@ public Content set(final int index, final Content child) {
public int size() {
return size;
}
+
+ @Override
+ public Iterator<Content> iterator() {
+ return new CLIterator();
+ }
/**
* Return this list as a <code>String</code>
@@ -476,6 +481,52 @@ public String toString() {
return super.toString();
}
+ /* * * * * * * * * * * * * ContentListIterator * * * * * * * * * * * * * * * */
+ /* * * * * * * * * * * * * ContentListIterator * * * * * * * * * * * * * * * */
+ private final class CLIterator implements Iterator<Content> {
+ private int expect = -1;
+ private int cursor = 0;
+ private boolean canremove = false;
+ private CLIterator() {
+ expect = modCount;
+ }
+ @Override
+ public boolean hasNext() {
+ return cursor < size;
+ }
+
+ @Override
+ public Content next() {
+ if (modCount != expect) {
+ throw new ConcurrentModificationException("ContentList was " +
+ "modified outside of this Iterator");
+ }
+ if (cursor >= size) {
+ throw new NoSuchElementException("Iterated beyond the end of " +
+ "the ContentList.");
+ }
+ canremove = true;
+ return elementData[cursor++];
+ }
+
+ @Override
+ public void remove() {
+ if (modCount != expect) {
+ throw new ConcurrentModificationException("ContentList was " +
+ "modified outside of this Iterator");
+ }
+ if (!canremove) {
+ throw new IllegalStateException("Can only remove() content " +
+ "after a call to next()");
+ }
+ canremove = false;
+ ContentList.this.remove(--cursor);
+ expect = modCount;
+ }
+
+
+ }
+
/* * * * * * * * * * * * * FilterList * * * * * * * * * * * * * * * */
/* * * * * * * * * * * * * FilterList * * * * * * * * * * * * * * * */
@@ -891,4 +942,5 @@ public void set(F obj) {
}
}
+
}
@@ -243,6 +243,64 @@ private final void exercise(List<T> list, T...content) {
quickCheck(list, content);
{
+ // Check the iteration code.
+ // the iterator implementation can be different to the ListIterator
+ Iterator<T> it = list.iterator();
+ try {
+ it.remove();
+ fail("Should have failed pre-next() remove()");
+ } catch (IllegalStateException ise) {
+ // good
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("Expected IllegalStateException but got " + e.getClass());
+ }
+ int i = 0;
+ while (i < content.length) {
+ assertTrue(it.hasNext());
+ assertTrue(it.next() == content[i]);
+ it.remove();
+ try {
+ it.remove();
+ fail("Should have failed sequential remove()");
+ } catch (IllegalStateException ise) {
+ // good
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("Expected IllegalStateException but got " + e.getClass());
+ }
+ i++;
+ }
+ assertFalse(it.hasNext());
+ try {
+ it.next();
+ fail("Should have failed end-of-iteration next()");
+ } catch (NoSuchElementException nse) {
+ // good
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("Expected IllegalStateException but got " + e.getClass());
+ }
+
+ try {
+ it.remove();
+ fail("Should have failed after-end remove()");
+ } catch (IllegalStateException ise) {
+ // good
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("Expected IllegalStateException but got " + e.getClass());
+ }
+
+ for (T d : content) {
+ list.add(d);
+ }
+
+ }
+
+ quickCheck(list, content);
+
+ {
//Read-Only List iteration through list contents.
for (int origin = 0; origin <= content.length; origin++) {
// start at every place
@@ -1080,13 +1138,26 @@ public void testConcurrentMod() {
T[] sample = buildSampleContent();
assertTrue("Not enough sample data " + sample.length, sample.length > 2);
+ list.add(sample[0]);
+
ListIterator<T> it = list.listIterator();
+ Iterator<T> si = list.iterator();
+
+ // It is important to add some content, and do a next()
+ // before testing concurrency, because some iterator methods
+ // check for the Iterator state before checking concurrency.
+ // this puts the iterator in to a valid state for remove(), set(), etc.
+ assertTrue(sample[0] == it.next());
+ assertTrue(sample[0] == si.next());
+
// create a concurrent issue.
- list.addAll(Arrays.asList(sample));
+ // we have checked for more than 2 elements so this should be fine.
+ list.add(sample[1]);
+ list.add(sample[2]);
// hasNext() should never throw CME.
assertTrue(it.hasNext());
- assertTrue(0 == it.nextIndex());
+ assertTrue(1 == it.nextIndex());
// hasNext() should never throw CME.
assertTrue(it.hasNext());
@@ -1101,16 +1172,16 @@ public void testConcurrentMod() {
}
- it = list.listIterator(list.size());
- assertTrue(sample[sample.length - 1] == it.previous());
+ it = list.listIterator(3);
+ assertTrue(sample[2] == it.previous());
// create a concurrent issue.
- list.remove(sample.length - 1);
+ list.remove(2);
// hasPrevious() should never throw CME.
assertTrue(it.hasPrevious());
- assertTrue(sample.length - 2 == it.previousIndex());
+ assertTrue(1 == it.previousIndex());
// hasPrevious() should never throw CME.
assertTrue(it.hasPrevious());
@@ -1159,6 +1230,34 @@ public void testConcurrentMod() {
fail ("Iterators should throw ConcurrentModificationException not "
+ e.getClass() + ":" + e.getMessage());
}
+
+
+
+ // Now test the Simple Iterator concurrency
+
+ // hasNext() should never throw CME.
+ assertTrue(si.hasNext());
+
+ try {
+ si.next();
+ fail ("Should have ConcurrentModificationException");
+ } catch (ConcurrentModificationException cme) {
+ // good
+ } catch (Exception e) {
+ fail ("Iterators should throw ConcurrentModificationException not "
+ + e.getClass() + ":" + e.getMessage());
+ }
+
+ try {
+ si.remove();
+ fail ("Should have ConcurrentModificationException");
+ } catch (ConcurrentModificationException cme) {
+ // good
+ } catch (Exception e) {
+ fail ("Iterators should throw ConcurrentModificationException not "
+ + e.getClass() + ":" + e.getMessage());
+ }
+
}
@Test

0 comments on commit 4121705

Please sign in to comment.