commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t.@apache.org
Subject svn commit: r1655060 - in /commons/proper/collections/trunk/src: changes/changes.xml main/java/org/apache/commons/collections4/set/ListOrderedSet.java test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
Date Tue, 27 Jan 2015 15:03:22 GMT
Author: tn
Date: Tue Jan 27 15:03:22 2015
New Revision: 1655060

URL: http://svn.apache.org/r1655060
Log:
[COLLECTIONS-426] 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/set/ListOrderedSet.java
    commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.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=1655060&r1=1655059&r2=1655060&view=diff
==============================================================================
--- commons/proper/collections/trunk/src/changes/changes.xml (original)
+++ commons/proper/collections/trunk/src/changes/changes.xml Tue Jan 27 15:03:22 2015
@@ -22,6 +22,10 @@
   <body>
 
   <release version="4.1" date="TBD" description="">
+    <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.
+    </action>
     <action issue="COLLECTIONS-530" dev="tn" type="fix" due-to="Erik">
       Added a Builder for "PredicatedCollection". Elements added to the builder
       that fail the predicate will not throw an IllegalArgumentException. The builder

Modified: commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java?rev=1655060&r1=1655059&r2=1655060&view=diff
==============================================================================
--- commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
(original)
+++ commons/proper/collections/trunk/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java
Tue Jan 27 15:03:22 2015
@@ -225,27 +225,31 @@ public class ListOrderedSet<E>
         return result;
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * This implementation iterates over the elements of this set, 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 set. 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> collectionRetainAll = new HashSet<Object>();
-        for (final Object next : coll) {
-            if (decorated().contains(next)) {
-                collectionRetainAll.add(next);
-            }
-        }
-        if (collectionRetainAll.size() == decorated().size()) {
+        boolean result = decorated().retainAll(coll);
+        if (result == false) {
             return false;
         }
-        if (collectionRetainAll.size() == 0) {
-            clear();
+        if (decorated().size() == 0) {
+            setOrder.clear();
         } else {
-            for (final Iterator<E> it = iterator(); it.hasNext();) {
-                if (!collectionRetainAll.contains(it.next())) {
+            for (Iterator<E> it = setOrder.iterator(); it.hasNext();) {
+                if (!decorated().contains(it.next())) {
                     it.remove();
                 }
             }
         }
-        return true;
+        return result;
     }
 
     @Override

Modified: commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
URL: http://svn.apache.org/viewvc/commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java?rev=1655060&r1=1655059&r2=1655060&view=diff
==============================================================================
--- commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
(original)
+++ commons/proper/collections/trunk/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java
Tue Jan 27 15:03:22 2015
@@ -204,31 +204,6 @@ public class ListOrderedSetTest<E>
         assertEquals(Integer.valueOf(0), orderedSet.get(4));
     }
 
-    /*
-     * test case for https://issues.apache.org/jira/browse/COLLECTIONS-426
-     */
-    @SuppressWarnings("boxing") // OK in test code
-    public void testRetainAllCollections426() {
-        final int size = 100000;
-        final ListOrderedSet<Integer> set = new ListOrderedSet<Integer>();
-        for (int i = 0; i < size; i++) {
-            set.add(i);
-        }
-        final ArrayList<Integer> list = new ArrayList<Integer>();
-        for (int i = size; i < 2 * size; i++) {
-            list.add(i);
-        }
-
-        final long start = System.currentTimeMillis();
-        set.retainAll(list);
-        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);
-    }
-
     @SuppressWarnings("unchecked")
     public void testDuplicates() {
         final List<E> list = new ArrayList<E>(10);



Mime
View raw message