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>());
 


Reply via email to