Browse Source

Merge pull request #5587 from cakephp/3.0-faster-collections

3.0 faster collections
Mark Story 11 years ago
parent
commit
51031076dd

+ 44 - 24
src/Collection/CollectionTrait.php

@@ -15,7 +15,7 @@
 namespace Cake\Collection;
 
 use AppendIterator;
-use ArrayObject;
+use ArrayIterator;
 use Cake\Collection\Collection;
 use Cake\Collection\Iterator\BufferedIterator;
 use Cake\Collection\Iterator\ExtractIterator;
@@ -28,6 +28,7 @@ use Cake\Collection\Iterator\SortIterator;
 use Cake\Collection\Iterator\StoppableIterator;
 use Cake\Collection\Iterator\TreeIterator;
 use Cake\Collection\Iterator\UnfoldIterator;
+use Iterator;
 use LimitIterator;
 use RecursiveIteratorIterator;
 
@@ -45,7 +46,7 @@ trait CollectionTrait
      */
     public function each(callable $c)
     {
-        foreach ($this as $k => $v) {
+        foreach ($this->_unwrap() as $k => $v) {
             $c($v, $k);
         }
         return $this;
@@ -63,7 +64,7 @@ trait CollectionTrait
                 return (bool)$v;
             };
         }
-        return new FilterIterator($this, $c);
+        return new FilterIterator($this->_unwrap(), $c);
     }
 
     /**
@@ -73,7 +74,7 @@ trait CollectionTrait
      */
     public function reject(callable $c)
     {
-        return new FilterIterator($this, function ($key, $value, $items) use ($c) {
+        return new FilterIterator($this->_unwrap(), function ($key, $value, $items) use ($c) {
             return !$c($key, $value, $items);
         });
     }
@@ -84,7 +85,7 @@ trait CollectionTrait
      */
     public function every(callable $c)
     {
-        foreach ($this as $key => $value) {
+        foreach ($this->_unwrap() as $key => $value) {
             if (!$c($value, $key)) {
                 return false;
             }
@@ -98,7 +99,7 @@ trait CollectionTrait
      */
     public function some(callable $c)
     {
-        foreach ($this as $key => $value) {
+        foreach ($this->_unwrap() as $key => $value) {
             if ($c($value, $key) === true) {
                 return true;
             }
@@ -112,7 +113,7 @@ trait CollectionTrait
      */
     public function contains($value)
     {
-        foreach ($this as $v) {
+        foreach ($this->_unwrap() as $v) {
             if ($value === $v) {
                 return true;
             }
@@ -127,7 +128,7 @@ trait CollectionTrait
      */
     public function map(callable $c)
     {
-        return new ReplaceIterator($this, $c);
+        return new ReplaceIterator($this->_unwrap(), $c);
     }
 
     /**
@@ -137,7 +138,7 @@ trait CollectionTrait
     public function reduce(callable $c, $zero)
     {
         $result = $zero;
-        foreach ($this as $k => $value) {
+        foreach ($this->_unwrap() as $k => $value) {
             $result = $c($result, $value, $k);
         }
         return $result;
@@ -150,7 +151,7 @@ trait CollectionTrait
      */
     public function extract($matcher)
     {
-        return new ExtractIterator($this, $matcher);
+        return new ExtractIterator($this->_unwrap(), $matcher);
     }
 
     /**
@@ -159,8 +160,7 @@ trait CollectionTrait
      */
     public function max($callback, $type = SORT_NUMERIC)
     {
-        $sorted = new SortIterator($this, $callback, SORT_DESC, $type);
-        return $sorted->top();
+        return (new SortIterator($this->_unwrap(), $callback, SORT_DESC, $type))->first();
     }
 
     /**
@@ -169,8 +169,7 @@ trait CollectionTrait
      */
     public function min($callback, $type = SORT_NUMERIC)
     {
-        $sorted = new SortIterator($this, $callback, SORT_ASC, $type);
-        return $sorted->top();
+        return (new SortIterator($this->_unwrap(), $callback, SORT_ASC, $type))->first();
     }
 
     /**
@@ -179,7 +178,7 @@ trait CollectionTrait
      */
     public function sortBy($callback, $dir = SORT_DESC, $type = SORT_NUMERIC)
     {
-        return new Collection(new SortIterator($this, $callback, $dir, $type));
+        return new SortIterator($this->_unwrap(), $callback, $dir, $type);
     }
 
     /**
@@ -225,7 +224,7 @@ trait CollectionTrait
         $reducer = function ($values, $key, $mr) {
             $mr->emit(count($values), $key);
         };
-        return new Collection(new MapReduce($this, $mapper, $reducer));
+        return new Collection(new MapReduce($this->_unwrap(), $mapper, $reducer));
     }
 
     /**
@@ -249,7 +248,7 @@ trait CollectionTrait
      */
     public function shuffle()
     {
-        $elements = iterator_to_array($this);
+        $elements = $this->toArray();
         shuffle($elements);
         return new Collection($elements);
     }
@@ -269,7 +268,7 @@ trait CollectionTrait
      */
     public function take($size = 1, $from = 0)
     {
-        return new Collection(new LimitIterator($this, $from, $size));
+        return new Collection(new LimitIterator($this->_unwrap(), $from, $size));
     }
 
     /**
@@ -307,9 +306,10 @@ trait CollectionTrait
      */
     public function append($items)
     {
+        $items = $items instanceof Iterator ? $items : new Collection($items);
         $list = new AppendIterator;
         $list->append($this);
-        $list->append(new Collection($items));
+        $list->append($items->_unwrap());
         return new Collection($list);
     }
 
@@ -349,7 +349,7 @@ trait CollectionTrait
             $mapReduce->emit($result, $key);
         };
 
-        return new Collection(new MapReduce($this, $mapper, $reducer));
+        return new Collection(new MapReduce($this->_unwrap(), $mapper, $reducer));
     }
 
     /**
@@ -374,7 +374,7 @@ trait CollectionTrait
         $reducer = function ($values, $key, $mapReduce) use (&$parents, $isObject) {
             if (empty($key) || !isset($parents[$key])) {
                 foreach ($values as $id) {
-                    $parents[$id] = $isObject ? $parents[$id] : new ArrayObject($parents[$id]);
+                    $parents[$id] = $isObject ? $parents[$id] : new ArrayIterator($parents[$id], 1);
                     $mapReduce->emit($parents[$id]);
                 }
                 return;
@@ -388,7 +388,7 @@ trait CollectionTrait
         $collection = new MapReduce($this, $mapper, $reducer);
         if (!$isObject) {
             $collection = (new Collection($collection))->map(function ($value) {
-                return (array)$value;
+                return $value->getArrayCopy();
             });
         }
 
@@ -402,7 +402,7 @@ trait CollectionTrait
      */
     public function insert($path, $values)
     {
-        return new InsertIterator($this, $path, $values);
+        return new InsertIterator($this->_unwrap(), $path, $values);
     }
 
     /**
@@ -411,6 +411,11 @@ trait CollectionTrait
      */
     public function toArray($preserveKeys = true)
     {
+        $iterator = $this->_unwrap();
+        if ($iterator instanceof ArrayIterator) {
+            $items = $iterator->getArrayCopy();
+            return $preserveKeys ? $items : array_values($items);
+        }
         return iterator_to_array($this, $preserveKeys);
     }
 
@@ -420,7 +425,7 @@ trait CollectionTrait
      */
     public function toList()
     {
-        return iterator_to_array($this, false);
+        return $this->toArray(false);
     }
 
     /**
@@ -502,4 +507,19 @@ trait CollectionTrait
             )
         );
     }
+
+    /**
+     * Returns the closest nested iterator that can be safely traversed without
+     * losing any possible transformations.
+     *
+     * @return \Iterator
+     */
+    protected function _unwrap()
+    {
+        $iterator = $this;
+        while (get_class($iterator) === 'Cake\Collection\Collection') {
+            $iterator = $iterator->getInnerIterator();
+        }
+        return $iterator;
+    }
 }

+ 9 - 1
src/Collection/Iterator/ReplaceIterator.php

@@ -31,6 +31,13 @@ class ReplaceIterator extends Collection
     protected $_callback;
 
     /**
+     * A reference to the internal iterator this object is wrapping.
+     *
+     * @var \Iterator
+     */
+    protected $_innerIterator;
+
+    /**
      * Creates an iterator from another iterator that will modify each of the values
      * by converting them using a callback function.
      *
@@ -45,6 +52,7 @@ class ReplaceIterator extends Collection
     {
         $this->_callback = $callback;
         parent::__construct($items);
+        $this->_innerIterator = $this->getInnerIterator();
     }
 
     /**
@@ -56,6 +64,6 @@ class ReplaceIterator extends Collection
     public function current()
     {
         $callback = $this->_callback;
-        return $callback(parent::current(), $this->key(), $this->getInnerIterator());
+        return $callback(parent::current(), $this->key(), $this->_innerIterator);
     }
 }

+ 13 - 93
src/Collection/Iterator/SortIterator.php

@@ -14,8 +14,8 @@
  */
 namespace Cake\Collection\Iterator;
 
-use Cake\Collection\ExtractTrait;
-use SplHeap;
+use Cake\Collection\Collection;
+use Cake\Collection\CollectionInterface;
 
 /**
  * An iterator that will return the passed items in order. The order is given by
@@ -37,40 +37,9 @@ use SplHeap;
  *
  * This iterator does not preserve the keys passed in the original elements.
  */
-class SortIterator extends SplHeap
+class SortIterator extends Collection
 {
 
-    use ExtractTrait;
-
-    /**
-     * Original items passed to this iterator
-     *
-     * @var array|\Traversable
-     */
-    protected $_items;
-
-    /**
-     * The callback used to extract the column or property from the elements
-     *
-     * @var callable
-     */
-    protected $_callback;
-
-    /**
-     * The direction in which the elements should be sorted. The constants
-     * `SORT_ASC` and `SORT_DESC` are the accepted values
-     *
-     * @var string
-     */
-    protected $_dir;
-
-    /**
-     * The type of sort comparison to perform.
-     *
-     * @var string
-     */
-    protected $_type;
-
     /**
      * Wraps this iterator around the passed items so when iterated they are returned
      * in order.
@@ -90,70 +59,21 @@ class SortIterator extends SplHeap
      */
     public function __construct($items, $callback, $dir = SORT_DESC, $type = SORT_NUMERIC)
     {
-        $this->_items = $items;
-        $this->_dir = $dir;
-        $this->_type = $type;
-        $this->_callback = $this->_propertyExtractor($callback);
-    }
-
-    /**
-     * The comparison function used to sort the elements
-     *
-     * @param mixed $a an element in the list
-     * @param mixed $b an element in the list
-     * @return int
-     */
-    public function compare($a, $b)
-    {
-        if ($this->_dir === SORT_ASC) {
-            list($a, $b) = [$b, $a];
-        }
-
-        $callback = $this->_callback;
-        $a = $callback($a);
-        $b = $callback($b);
-
-        if ($this->_type === SORT_NUMERIC) {
-            return $a - $b;
+        if ($items instanceof CollectionInterface) {
+            $items = $items->toList();
         }
 
-        if ($this->_type === SORT_NATURAL) {
-            return strnatcmp($a, $b);
+        $callback = $this->_propertyExtractor($callback);
+        $results = [];
+        foreach ($items as $key => $value) {
+            $results[$key] = $callback($value);
         }
 
-        if ($this->_type === SORT_STRING) {
-            return strcmp($a, $b);
-        }
-
-        return strcoll($a, $b);
-    }
-
-    /**
-     * Returns the top of the heap. Rewinds the iterator if the heap is empty.
-     *
-     * @return mixed
-     */
-    public function top()
-    {
-        if ($this->isEmpty()) {
-            $this->rewind();
-        }
-        if ($this->isEmpty()) {
-            return null;
-        }
-        return parent::top();
-    }
+        $dir === SORT_DESC ? arsort($results, $type) : asort($results, $type);
 
-    /**
-     * SplHeap removes elements upon iteration. Implementing rewind so that
-     * this iterator can be reused, at least at a cost.
-     *
-     * @return void
-     */
-    public function rewind()
-    {
-        foreach ($this->_items as $item) {
-            $this->insert($item);
+        foreach (array_keys($results) as $key) {
+            $results[$key] = $items[$key];
         }
+        parent::__construct($results);
     }
 }

+ 9 - 1
src/Collection/Iterator/StoppableIterator.php

@@ -35,6 +35,13 @@ class StoppableIterator extends Collection
     protected $_condition;
 
     /**
+     * A reference to the internal iterator this object is wrapping.
+     *
+     * @var \Iterator
+     */
+    protected $_innerIterator;
+
+    /**
      * Creates an iterator that can be stopped based on a condition provided by a callback.
      *
      * Each time the condition callback is executed it will receive the value of the element
@@ -50,6 +57,7 @@ class StoppableIterator extends Collection
     {
         $this->_condition = $condition;
         parent::__construct($items);
+        $this->_innnerIterator = $this->getInnerIterator();
     }
 
     /**
@@ -67,6 +75,6 @@ class StoppableIterator extends Collection
         $current = $this->current();
         $key = $this->key();
         $condition = $this->_condition;
-        return !$condition($current, $key, $this);
+        return !$condition($current, $key, $this->_innerIterator);
     }
 }

+ 9 - 1
src/Collection/Iterator/UnfoldIterator.php

@@ -37,6 +37,13 @@ class UnfoldIterator extends IteratorIterator implements RecursiveIterator
     protected $_unfolder;
 
     /**
+     * A reference to the internal iterator this object is wrapping.
+     *
+     * @var \Iterator
+     */
+    protected $_innerIterator;
+
+    /**
      * Creates the iterator that will generate child iterators from each of the
      * elements it was constructed with.
      *
@@ -49,6 +56,7 @@ class UnfoldIterator extends IteratorIterator implements RecursiveIterator
     {
         $this->_unfolder = $unfolder;
         parent::__construct($items);
+        $this->_innerIterator = $this->getInnerIterator();
     }
 
     /**
@@ -74,6 +82,6 @@ class UnfoldIterator extends IteratorIterator implements RecursiveIterator
         $key = $this->key();
         $unfolder = $this->_unfolder;
 
-        return new NoChildrenIterator($unfolder($current, $key, $this));
+        return new NoChildrenIterator($unfolder($current, $key, $this->_innerIterator));
     }
 }

+ 6 - 6
tests/TestCase/Collection/CollectionTest.php

@@ -118,7 +118,7 @@ class CollectionTest extends TestCase
         $items = ['a' => 1, 'b' => 2, 'c' => 3];
         $collection = new Collection($items);
         $result = $collection->reject(function ($v, $k, $items) use ($collection) {
-            $this->assertSame($collection, $items);
+            $this->assertSame($collection->getInnerIterator(), $items);
             return $v > 2;
         });
         $this->assertEquals(['a' => 1, 'b' => 2], iterator_to_array($result));
@@ -244,7 +244,7 @@ class CollectionTest extends TestCase
         $items = ['a' => 1, 'b' => 2, 'c' => 3];
         $collection = new Collection($items);
         $map = $collection->map(function ($v, $k, $it) use ($collection) {
-            $this->assertSame($collection, $it);
+            $this->assertSame($collection->getInnerIterator(), $it);
             return $v * $v;
         });
         $this->assertInstanceOf('Cake\Collection\Iterator\ReplaceIterator', $map);
@@ -306,11 +306,11 @@ class CollectionTest extends TestCase
         $map = $collection->sortBy('a.b.c');
         $this->assertInstanceOf('Cake\Collection\Collection', $map);
         $expected = [
-            2 => ['a' => ['b' => ['c' => 10]]],
-            1 => ['a' => ['b' => ['c' => 6]]],
-            0 => ['a' => ['b' => ['c' => 4]]],
+            ['a' => ['b' => ['c' => 10]]],
+            ['a' => ['b' => ['c' => 6]]],
+            ['a' => ['b' => ['c' => 4]]],
         ];
-        $this->assertEquals($expected, iterator_to_array($map));
+        $this->assertEquals($expected, $map->toList());
     }
 
     /**

+ 45 - 63
tests/TestCase/Collection/Iterator/SortIteratorTest.php

@@ -37,12 +37,12 @@ class SortIteratorTest extends TestCase
             return $a;
         };
         $sorted = new SortIterator($items, $identity);
-        $expected = array_combine(range(4, 0), range(5, 1));
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $expected = range(5, 1);
+        $this->assertEquals($expected, $sorted->toList());
 
         $sorted = new SortIterator($items, $identity, SORT_ASC);
-        $expected = array_combine(range(4, 0), range(1, 5));
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $expected = range(1, 5);
+        $this->assertEquals($expected, $sorted->toList());
     }
 
     /**
@@ -57,12 +57,12 @@ class SortIteratorTest extends TestCase
             return $a * -1;
         };
         $sorted = new SortIterator($items, $callback);
-        $expected = array_combine(range(4, 0), [1, 2, 3, 4, 5]);
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $expected = range(1, 5);
+        $this->assertEquals($expected, $sorted->toList());
 
         $sorted = new SortIterator($items, $callback, SORT_ASC);
-        $expected = array_combine(range(4, 0), [5, 4, 3, 2, 1]);
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $expected = range(5, 1);
+        $this->assertEquals($expected, $sorted->toList());
     }
 
     /**
@@ -83,21 +83,21 @@ class SortIteratorTest extends TestCase
         };
         $sorted = new SortIterator($items, $callback, SORT_DESC, SORT_NUMERIC);
         $expected = [
-            3 => ['foo' => 13, 'bar' => 'a'],
-            2 => ['foo' => 10, 'bar' => 'a'],
-            1 => ['foo' => 2, 'bar' => 'a'],
-            0 => ['foo' => 1, 'bar' => 'a'],
+            ['foo' => 13, 'bar' => 'a'],
+            ['foo' => 10, 'bar' => 'a'],
+            ['foo' => 2, 'bar' => 'a'],
+            ['foo' => 1, 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $this->assertEquals($expected, $sorted->toList());
 
         $sorted = new SortIterator($items, $callback, SORT_ASC, SORT_NUMERIC);
         $expected = [
-            3 => ['foo' => 1, 'bar' => 'a'],
-            2 => ['foo' => 2, 'bar' => 'a'],
-            1 => ['foo' => 10, 'bar' => 'a'],
-            0 => ['foo' => 13, 'bar' => 'a'],
+            ['foo' => 1, 'bar' => 'a'],
+            ['foo' => 2, 'bar' => 'a'],
+            ['foo' => 10, 'bar' => 'a'],
+            ['foo' => 13, 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $this->assertEquals($expected, $sorted->toList());
     }
 
     /**
@@ -118,22 +118,22 @@ class SortIteratorTest extends TestCase
         };
         $sorted = new SortIterator($items, $callback, SORT_DESC, SORT_NATURAL);
         $expected = [
-            3 => ['foo' => 'foo_13', 'bar' => 'a'],
-            2 => ['foo' => 'foo_10', 'bar' => 'a'],
-            1 => ['foo' => 'foo_2', 'bar' => 'a'],
-            0 => ['foo' => 'foo_1', 'bar' => 'a'],
+            ['foo' => 'foo_13', 'bar' => 'a'],
+            ['foo' => 'foo_10', 'bar' => 'a'],
+            ['foo' => 'foo_2', 'bar' => 'a'],
+            ['foo' => 'foo_1', 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $this->assertEquals($expected, $sorted->toList());
 
         $sorted = new SortIterator($items, $callback, SORT_ASC, SORT_NATURAL);
         $expected = [
-            3 => ['foo' => 'foo_1', 'bar' => 'a'],
-            2 => ['foo' => 'foo_2', 'bar' => 'a'],
-            1 => ['foo' => 'foo_10', 'bar' => 'a'],
-            0 => ['foo' => 'foo_13', 'bar' => 'a'],
+            ['foo' => 'foo_1', 'bar' => 'a'],
+            ['foo' => 'foo_2', 'bar' => 'a'],
+            ['foo' => 'foo_10', 'bar' => 'a'],
+            ['foo' => 'foo_13', 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
-        $this->assertEquals($expected, iterator_to_array($sorted), 'Iterator should rewind');
+        $this->assertEquals($expected, $sorted->toList());
+        $this->assertEquals($expected, $sorted->toList(), 'Iterator should rewind');
     }
 
     /**
@@ -151,22 +151,22 @@ class SortIteratorTest extends TestCase
         ]);
         $sorted = new SortIterator($items, 'foo', SORT_DESC, SORT_NATURAL);
         $expected = [
-            3 => ['foo' => 'foo_13', 'bar' => 'a'],
-            2 => ['foo' => 'foo_10', 'bar' => 'a'],
-            1 => ['foo' => 'foo_2', 'bar' => 'a'],
-            0 => ['foo' => 'foo_1', 'bar' => 'a'],
+            ['foo' => 'foo_13', 'bar' => 'a'],
+            ['foo' => 'foo_10', 'bar' => 'a'],
+            ['foo' => 'foo_2', 'bar' => 'a'],
+            ['foo' => 'foo_1', 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
+        $this->assertEquals($expected, $sorted->toList());
 
         $sorted = new SortIterator($items, 'foo', SORT_ASC, SORT_NATURAL);
         $expected = [
-            3 => ['foo' => 'foo_1', 'bar' => 'a'],
-            2 => ['foo' => 'foo_2', 'bar' => 'a'],
-            1 => ['foo' => 'foo_10', 'bar' => 'a'],
-            0 => ['foo' => 'foo_13', 'bar' => 'a'],
+            ['foo' => 'foo_1', 'bar' => 'a'],
+            ['foo' => 'foo_2', 'bar' => 'a'],
+            ['foo' => 'foo_10', 'bar' => 'a'],
+            ['foo' => 'foo_13', 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
-        $this->assertEquals($expected, iterator_to_array($sorted), 'Iterator should rewind');
+        $this->assertEquals($expected, $sorted->toList());
+        $this->assertEquals($expected, $sorted->toList(), 'Iterator should rewind');
     }
 
     /**
@@ -184,29 +184,11 @@ class SortIteratorTest extends TestCase
         ]);
         $sorted = new SortIterator($items, 'foo.bar', SORT_ASC, SORT_NUMERIC);
         $expected = [
-            3 => ['foo' => ['bar' => 1], 'bar' => 'a'],
-            2 => ['foo' => ['bar' => 2], 'bar' => 'a'],
-            1 => ['foo' => ['bar' => 10], 'bar' => 'a'],
-            0 => ['foo' => ['bar' => 12], 'bar' => 'a'],
+            ['foo' => ['bar' => 1], 'bar' => 'a'],
+            ['foo' => ['bar' => 2], 'bar' => 'a'],
+            ['foo' => ['bar' => 10], 'bar' => 'a'],
+            ['foo' => ['bar' => 12], 'bar' => 'a'],
         ];
-        $this->assertEquals($expected, iterator_to_array($sorted));
-    }
-
-    /**
-     * Tests top
-     *
-     * @return void
-     */
-    public function testTop()
-    {
-        $items = new ArrayObject([3, 5, 1, 2, 4]);
-        $identity = function ($a) {
-            return $a;
-        };
-        $sorted = new SortIterator($items, $identity);
-        $this->assertEquals(5, $sorted->top());
-
-        $sorted = new SortIterator([], $identity);
-        $this->assertNull($sorted->top());
+        $this->assertEquals($expected, $sorted->toList());
     }
 }

+ 1 - 1
tests/TestCase/ORM/Behavior/TranslateBehaviorTest.php

@@ -839,7 +839,7 @@ class TranslateBehaviorTest extends TestCase
         $query = $table->find()->where(['Comments.id' => 6]);
         $query2 = $table->find()->where(['Comments.id' => 5]);
         $query->union($query2);
-        $results = $query->sortBy('id')->toArray();
+        $results = $query->sortBy('id', SORT_ASC)->toList();
         $this->assertCount(2, $results);
 
         $this->assertEquals('First Comment for Second Article', $results[0]->comment);