Author: tn Date: Tue Jan 27 15:09:02 2015 New Revision: 1655062 URL: http://svn.apache.org/r1655062 Log: [COLLECTIONS-427] Revert performance improvement and add clarifying javadoc wrt runtime complexity.
Modified: commons/proper/collections/trunk/src/changes/changes.xml commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java Modified: commons/proper/collections/trunk/src/changes/changes.xml URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/changes/changes.xml?rev=1655062&r1=1655061&r2=1655062&view=diff ============================================================================== --- commons/proper/collections/trunk/src/changes/changes.xml (original) +++ commons/proper/collections/trunk/src/changes/changes.xml Tue Jan 27 15:09:02 2015 @@ -22,6 +22,10 @@ <body> <release version="4.1" date="TBD" description=""> + <action issue="COLLECTIONS-427" dev="tn" type="fix"> + Reverted performance improvement for "SetUniqueList#retainAll(Collection)" + introduced in 4.0. Added clarifying javadoc wrt runtime complexity instead. + </action> <action issue="COLLECTIONS-426" dev="tn" type="fix"> Reverted performance improvement for "ListOrderedSet#retainAll(Collection)" introduced in 4.0. Added clarifying javadoc wrt runtime complexity instead. Modified: commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java?rev=1655062&r1=1655061&r2=1655062&view=diff ============================================================================== --- commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java (original) +++ commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/list/SetUniqueList.java Tue Jan 27 15:09:02 2015 @@ -25,9 +25,9 @@ import java.util.ListIterator; import java.util.Set; import org.apache.commons.collections4.ListUtils; -import org.apache.commons.collections4.set.UnmodifiableSet; import org.apache.commons.collections4.iterators.AbstractIteratorDecorator; import org.apache.commons.collections4.iterators.AbstractListIteratorDecorator; +import org.apache.commons.collections4.set.UnmodifiableSet; /** * Decorates a <code>List</code> to ensure that no duplicates are present much @@ -251,27 +251,28 @@ public class SetUniqueList<E> extends Ab return result; } + /** + * {@inheritDoc} + * <p> + * This implementation iterates over the elements of this list, checking + * each element in turn to see if it's contained in <code>coll</code>. + * If it's not contained, it's removed from this list. As a consequence, + * it is advised to use a collection type for <code>coll</code> that provides + * a fast (e.g. O(1)) implementation of {@link Collection#contains(Object)}. + */ @Override public boolean retainAll(final Collection<?> coll) { - final Set<Object> setRetainAll = new HashSet<Object>(); - for (final Object next : coll) { - if (set.contains(next)) { - setRetainAll.add(next); - } - } - if (setRetainAll.size() == set.size()) { + boolean result = set.retainAll(coll); + if (result == false) { return false; } - if (setRetainAll.size() == 0) { - clear(); + if (set.size() == 0) { + super.clear(); } else { - for (final Iterator<E> it = iterator(); it.hasNext();) { - if (!setRetainAll.contains(it.next())) { - it.remove(); - } - } + // use the set as parameter for the call to retainAll to improve performance + super.retainAll(set); } - return true; + return result; } @Override Modified: commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java?rev=1655062&r1=1655061&r2=1655062&view=diff ============================================================================== --- commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java (original) +++ commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/list/SetUniqueListTest.java Tue Jan 27 15:09:02 2015 @@ -561,32 +561,6 @@ public class SetUniqueListTest<E> extend assertTrue(uniqueList.contains(Integer.valueOf(8))); } - /* - * test case for https://issues.apache.org/jira/browse/COLLECTIONS-427 - */ - @SuppressWarnings("boxing") // OK in test code - public void testRetainAllCollections427() { - final int size = 50000; - final ArrayList<Integer> list = new ArrayList<Integer>(); - for (int i = 0; i < size; i++) { - list.add(i); - } - final SetUniqueList<Integer> uniqueList = SetUniqueList.setUniqueList(list); - final ArrayList<Integer> toRetain = new ArrayList<Integer>(); - for (int i = size; i < 2*size; i++) { - toRetain.add(i); - } - - final long start = System.currentTimeMillis(); - uniqueList.retainAll(toRetain); - final long stop = System.currentTimeMillis(); - - // make sure retainAll completes under 5 seconds - // TODO if test is migrated to JUnit 4, add a Timeout rule. - // http://kentbeck.github.com/junit/javadoc/latest/org/junit/rules/Timeout.html - assertTrue(stop - start < 5000); - } - public void testSetCollections444() { final SetUniqueList<Integer> lset = new SetUniqueList<Integer>(new ArrayList<Integer>(), new HashSet<Integer>());