Improve TimeEfficientImplementation performances.

Using a SplFixedArray seems to reduce memory allocation (-90% memory peak usage) and have greater lookup speed (+15%) than the two-dimension dynamic array storing the LCS matrix. It also seems that a SplFixedArray is wiped earlier by the PHP garbage collector.

An other optimization: avoid usage of "array_unshift" (Pushing into the result array and reversing it at the end is faster in most cases).

If this patch is OK, the "Differ->calculateEstimatedFootprint()" should be also adjusted, because the SplFixedArray memory cost seems to be completely different than the one of (partially filled) arrayXarray matrix.

(+ Removed unclear comment in the TimeEfficientImplementationTest case.)

Author: Guillaume Crico

src/LCS/TimeEfficientLongestCommonSubsequenceImplementation.php

@@ -66,24 +66,26 @@ class TimeEfficientImplementation implements LongestCommonSubsequence
     public function calculate(array $from, array $to)
     {
         $common     = array();
-        $matrix     = array();
         $fromLength = count($from);
         $toLength   = count($to);
+        $width      = $fromLength + 1;
+        $matrix     = new \SplFixedArray($width * ($toLength + 1));
 
         for ($i = 0; $i <= $fromLength; ++$i) {
-            $matrix[$i][0] = 0;
+            $matrix[$i] = 0;
         }
 
         for ($j = 0; $j <= $toLength; ++$j) {
-            $matrix[0][$j] = 0;
+            $matrix[$j * $width] = 0;
         }
 
         for ($i = 1; $i <= $fromLength; ++$i) {
             for ($j = 1; $j <= $toLength; ++$j) {
-                $matrix[$i][$j] = max(
-                    $matrix[$i-1][$j],
-                    $matrix[$i][$j-1],
-                    $from[$i-1] === $to[$j-1] ? $matrix[$i-1][$j-1] + 1 : 0
+                $o = ($j * $width) + $i;
+                $matrix[$o] = max(
+                    $matrix[$o - 1],
+                    $matrix[$o - $width],
+                    $from[$i - 1] === $to[$j - 1] ? $matrix[$o - $width - 1] + 1 : 0
                 );
             }
         }
@@ -93,16 +95,19 @@ public function calculate(array $from, array $to)
 
         while ($i > 0 && $j > 0) {
             if ($from[$i-1] === $to[$j-1]) {
-                array_unshift($common, $from[$i-1]);
+                $common[] = $from[$i-1];
                 --$i;
                 --$j;
-            } elseif ($matrix[$i][$j-1] > $matrix[$i-1][$j]) {
-                --$j;
             } else {
-                --$i;
+                $o = ($j * $width) + $i;
+                if ($matrix[$o - $width] > $matrix[$o - 1]) {
+                    --$j;
+                } else {
+                    --$i;
+                }
             }
         }
 
-        return $common;
+        return array_reverse($common);
     }
 }

tests/LCS/TimeEfficientImplementationTest.php

@@ -191,21 +191,12 @@ public function testSingleElementSubsequenceAtEnd()
 
     public function testReversedSequences()
     {
-        // The values are unique and the second sequence is the reverse
-        // of the first sequence.
-        // The LCS must return a single value: the first value of the
-        // first sequence (which is the same than the last value of the
-        // second sequence).
-        //
-        // I don't know --yet-- if it is really important!
-        // As the values are unique, both results are "longest common
-        // sequence"! In fact, any value from teh set would be valid!
-        //
-        // And that too bad, because if we were allowed to return the
-        // first value of the first sequence, it would be much faster!
-        // (by building the matrix backward and gathering the common 
-        // items in the original direction.)
-        //
+        $from     = array('A', 'B');
+        $to       = array('B', 'A');
+        $expected = array('A');
+        $common   = $this->implementation->calculate($from, $to);
+        $this->assertEquals($expected, $common);
+
         foreach ($this->stress_sizes as $size) {
             $from   = range(1, $size);
             $to     = array_reverse($from);