From 9bdfac0fad10c1947768aec9c1ac3645511f6ced Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Sat, 20 Apr 2013 13:11:14 +0000 Subject: [PATCH] [COLLECTIONS-433] Improve performance of TreeList#addAll and TreeList(Collection). Thanks to Jeffrey Barnes for the patch. git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1470159 13f79535-47bb-0310-9956-ffa450edef68 --- pom.xml | 5 +- src/changes/changes.xml | 4 + .../commons/collections4/list/TreeList.java | 199 +++++++++++++++++- .../collections4/list/TreeListTest.java | 63 +++++- 4 files changed, 266 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index fe38dfa8b..2170f481b 100644 --- a/pom.xml +++ b/pom.xml @@ -124,11 +124,14 @@ Federico Barbieri - Arron Bates + Jeffrey Barnes Nicola Ken Barozzi + + Arron Bates + Sebastian Bazley diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 47093dc78..e0ca85a5e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -22,6 +22,10 @@ + + Improve performance of "TreeList#addAll" and "TreeList(Collection)" by converting + the input collection into an AVL tree and efficiently merge it into the existing tree. + Added methods "CollectionUtils#merge(...)" to merge two sorted Collections into a sorted List using the standard O(n) merge algorithm. diff --git a/src/main/java/org/apache/commons/collections4/list/TreeList.java b/src/main/java/org/apache/commons/collections4/list/TreeList.java index e47fd6294..ea0cf9533 100644 --- a/src/main/java/org/apache/commons/collections4/list/TreeList.java +++ b/src/main/java/org/apache/commons/collections4/list/TreeList.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.ListIterator; import java.util.NoSuchElementException; +import org.apache.commons.collections4.ArrayStack; import org.apache.commons.collections4.OrderedIterator; /** @@ -53,6 +54,7 @@ import org.apache.commons.collections4.OrderedIterator; * @since 3.1 * @version $Id$ */ +@SuppressWarnings("deprecation") // replace ArrayStack by ArrayDeque when moving to Java 1.6 public class TreeList extends AbstractList { // add; toArray; iterator; insert; get; indexOf; remove // TreeList = 1260;7360;3080; 160; 170;3400; 170; @@ -74,14 +76,17 @@ public class TreeList extends AbstractList { } /** - * Constructs a new empty list that copies the specified list. + * Constructs a new empty list that copies the specified collection. * * @param coll the collection to copy * @throws NullPointerException if the collection is null */ public TreeList(final Collection coll) { super(); - addAll(coll); + if (!coll.isEmpty()) { + root = new AVLNode(coll); + size = coll.size(); + } } //----------------------------------------------------------------------- @@ -203,6 +208,29 @@ public class TreeList extends AbstractList { size++; } + /** + * Appends all of the elements in the specified collection to the end of this list, + * in the order that they are returned by the specified collection's Iterator. + *

+ * This method runs in O(n + log m) time, where m is + * the size of this list and n is the size of {@code c}. + * + * @param c the collection to be added to this list + * @return {@code true} if this list changed as a result of the call + * @throws NullPointerException {@inheritDoc} + */ + @Override + public boolean addAll(final Collection c) { + if (c.isEmpty()) { + return false; + } + modCount += c.size(); + final AVLNode cTree = new AVLNode(c); + root = root == null ? cTree : root.addAll(cTree, size); + size += c.size(); + return true; + } + /** * Sets the element at the specified index. * @@ -294,7 +322,7 @@ public class TreeList extends AbstractList { * Constructs a new node with a relative position. * * @param relativePosition the relative position of the node - * @param obj the value for the ndoe + * @param obj the value for the node * @param rightFollower the node with the value following this one * @param leftFollower the node with the value leading this one */ @@ -308,6 +336,58 @@ public class TreeList extends AbstractList { left = leftFollower; } + /** + * Constructs a new AVL tree from a collection. + *

+ * The collection must be nonempty. + * + * @param coll a nonempty collection + */ + private AVLNode(final Collection coll) { + this(coll.iterator(), 0, coll.size() - 1, 0, null, null); + } + + /** + * Constructs a new AVL tree from a collection. + *

+ * This is a recursive helper for {@link #AVLNode(Collection)}. A call + * to this method will construct the subtree for elements {@code start} + * through {@code end} of the collection, assuming the iterator + * {@code e} already points at element {@code start}. + * + * @param iterator an iterator over the collection, which should already point + * to the element at index {@code start} within the collection + * @param start the index of the first element in the collection that + * should be in this subtree + * @param end the index of the last element in the collection that + * should be in this subtree + * @param absolutePositionOfParent absolute position of this node's + * parent, or 0 if this node is the root + * @param prev the {@code AVLNode} corresponding to element (start - 1) + * of the collection, or null if start is 0 + * @param next the {@code AVLNode} corresponding to element (end + 1) + * of the collection, or null if end is the last element of the collection + */ + private AVLNode(final Iterator iterator, final int start, final int end, + final int absolutePositionOfParent, final AVLNode prev, final AVLNode next) { + final int mid = start + (end - start) / 2; + if (start < mid) { + left = new AVLNode(iterator, start, mid - 1, mid, prev, this); + } else { + leftIsPrevious = true; + left = prev; + } + value = iterator.next(); + relativePosition = mid - absolutePositionOfParent; + if (mid < end) { + right = new AVLNode(iterator, mid + 1, end, mid, this, next); + } else { + rightIsNext = true; + right = next; + } + recalcHeight(); + } + /** * Gets the value. * @@ -716,6 +796,119 @@ public class TreeList extends AbstractList { recalcHeight(); } + /** + * Appends the elements of another tree list to this tree list by efficiently + * merging the two AVL trees. This operation is destructive to both trees and + * runs in O(log(m + n)) time. + * + * @param otherTree + * the root of the AVL tree to merge with this one + * @param currentSize + * the number of elements in this AVL tree + * @return the root of the new, merged AVL tree + */ + private AVLNode addAll(AVLNode otherTree, final int currentSize) { + final AVLNode maxNode = max(); + final AVLNode otherTreeMin = otherTree.min(); + + // We need to efficiently merge the two AVL trees while keeping them + // balanced (or nearly balanced). To do this, we take the shorter + // tree and combine it with a similar-height subtree of the taller + // tree. There are two symmetric cases: + // * this tree is taller, or + // * otherTree is taller. + if (otherTree.height > height) { + // CASE 1: The other tree is taller than this one. We will thus + // merge this tree into otherTree. + + // STEP 1: Remove the maximum element from this tree. + final AVLNode leftSubTree = removeMax(); + + // STEP 2: Navigate left from the root of otherTree until we + // find a subtree, s, that is no taller than me. (While we are + // navigating left, we store the nodes we encounter in a stack + // so that we can re-balance them in step 4.) + final ArrayStack> sAncestors = new ArrayStack>(); + AVLNode s = otherTree; + int sAbsolutePosition = s.relativePosition + currentSize; + int sParentAbsolutePosition = 0; + while (s != null && s.height > getHeight(leftSubTree)) { + sParentAbsolutePosition = sAbsolutePosition; + sAncestors.push(s); + s = s.left; + if (s != null) { + sAbsolutePosition += s.relativePosition; + } + } + + // STEP 3: Replace s with a newly constructed subtree whose root + // is maxNode, whose left subtree is leftSubTree, and whose right + // subtree is s. + maxNode.setLeft(leftSubTree, null); + maxNode.setRight(s, otherTreeMin); + if (leftSubTree != null) { + leftSubTree.max().setRight(null, maxNode); + leftSubTree.relativePosition -= currentSize - 1; + } + if (s != null) { + s.min().setLeft(null, maxNode); + s.relativePosition = sAbsolutePosition - currentSize + 1; + } + maxNode.relativePosition = currentSize - 1 - sParentAbsolutePosition; + otherTree.relativePosition += currentSize; + + // STEP 4: Re-balance the tree and recalculate the heights of s's ancestors. + s = maxNode; + while (!sAncestors.isEmpty()) { + final AVLNode sAncestor = sAncestors.pop(); + sAncestor.setLeft(s, null); + s = sAncestor.balance(); + } + return s; + } else { + // CASE 2: This tree is taller. This is symmetric to case 1. + // We merge otherTree into this tree by finding a subtree s of this + // tree that is of similar height to otherTree and replacing it + // with a new subtree whose root is otherTreeMin and whose + // children are otherTree and s. + + otherTree = otherTree.removeMin(); + + final ArrayStack> sAncestors = new ArrayStack>(); + AVLNode s = this; + int sAbsolutePosition = s.relativePosition; + int sParentAbsolutePosition = 0; + while (s != null && s.height > getHeight(otherTree)) { + sParentAbsolutePosition = sAbsolutePosition; + sAncestors.push(s); + s = s.right; + if (s != null) { + sAbsolutePosition += s.relativePosition; + } + } + + otherTreeMin.setRight(otherTree, null); + otherTreeMin.setLeft(s, maxNode); + if (otherTree != null) { + otherTree.min().setLeft(null, otherTreeMin); + otherTree.relativePosition++; + } + if (s != null) { + s.max().setRight(null, otherTreeMin); + s.relativePosition = sAbsolutePosition - currentSize; + } + otherTreeMin.relativePosition = currentSize - sParentAbsolutePosition; + + s = otherTreeMin; + while (!sAncestors.isEmpty()) { + final AVLNode sAncestor = sAncestors.pop(); + sAncestor.setRight(s, null); + s = sAncestor.balance(); + } + return s; + } + } + // private void checkFaedelung() { // AVLNode maxNode = left.max(); // if (!maxNode.rightIsFaedelung || maxNode.right != this) { diff --git a/src/test/java/org/apache/commons/collections4/list/TreeListTest.java b/src/test/java/org/apache/commons/collections4/list/TreeListTest.java index 0a70db06b..e6af9be76 100644 --- a/src/test/java/org/apache/commons/collections4/list/TreeListTest.java +++ b/src/test/java/org/apache/commons/collections4/list/TreeListTest.java @@ -16,6 +16,7 @@ */ package org.apache.commons.collections4.list; +import java.util.ArrayList; import java.util.List; import java.util.ListIterator; @@ -265,6 +266,66 @@ public class TreeListTest extends AbstractListTest { // previous() after remove() should move to // the element before the one just removed assertEquals("A", li.previous()); - } + } + + public void testIterationOrder() { + // COLLECTIONS-433: + // ensure that the iteration order of elements is correct + // when initializing the TreeList with another collection + + for (int size = 1; size < 1000; size++) { + List other = new ArrayList(size); + for (int i = 0; i < size; i++) { + other.add(i); + } + TreeList l = new TreeList(other); + ListIterator it = l.listIterator(); + int i = 0; + while (it.hasNext()) { + Integer val = it.next(); + assertEquals(i++, val.intValue()); + } + + while (it.hasPrevious()) { + Integer val = it.previous(); + assertEquals(--i, val.intValue()); + } + } + } + + public void testIterationOrderAfterAddAll() { + // COLLECTIONS-433: + // ensure that the iteration order of elements is correct + // when calling addAll on the TreeList + + // to simulate different cases in addAll, do different runs where + // the number of elements already in the list and being added by addAll differ + + int size = 1000; + for (int i = 0; i < 100; i++) { + List other = new ArrayList(size); + for (int j = i; j < size; j++) { + other.add(j); + } + TreeList l = new TreeList(); + for (int j = 0; j < i; j++) { + l.add(j); + } + + l.addAll(other); + + ListIterator it = l.listIterator(); + int cnt = 0; + while (it.hasNext()) { + Integer val = it.next(); + assertEquals(cnt++, val.intValue()); + } + + while (it.hasPrevious()) { + Integer val = it.previous(); + assertEquals(--cnt, val.intValue()); + } + } + } }