Simplyfied whitespace checking

While finding blocks we have todo a whitespace check of part of the old sentence, this is done loads of times. This is done by refrerencing the oldWords array, and temporarely making a string from part of that array, and then checking if that string is only whitespace.

I have replaced this by a loop that iterates over the part of the sentence item by item, and when one of the items is not a space (usually the first item), it immediately reports false and caching the result, speeding up the algorithm in some cases by up to 50%

Author: Sven Hagemann

lib/Caxy/HtmlDiff/HtmlDiff.php

@@ -734,26 +734,24 @@ protected function matchingBlocks()
     }
 
     /**
-     * @param int   $startInOld
-     * @param int   $endInOld
-     * @param int   $startInNew
-     * @param int   $endInNew
-     * @param array $matchingBlocks
+     * @param MatchingBlock[] $matchingBlocks
      */
-    protected function findMatchingBlocks($startInOld, $endInOld, $startInNew, $endInNew, &$matchingBlocks)
+    protected function findMatchingBlocks(int $startInOld, int $endInOld, int $startInNew, int $endInNew, array &$matchingBlocks) : void
     {
         $match = $this->findMatch($startInOld, $endInOld, $startInNew, $endInNew);
 
-        if ($match !== null) {
-            if ($startInOld < $match->startInOld && $startInNew < $match->startInNew) {
-                $this->findMatchingBlocks($startInOld, $match->startInOld, $startInNew, $match->startInNew, $matchingBlocks);
-            }
+        if ($match === null) {
+            return;
+        }
 
-            $matchingBlocks[] = $match;
+        if ($startInOld < $match->startInOld && $startInNew < $match->startInNew) {
+            $this->findMatchingBlocks($startInOld, $match->startInOld, $startInNew, $match->startInNew, $matchingBlocks);
+        }
 
-            if ($match->endInOld() < $endInOld && $match->endInNew() < $endInNew) {
-                $this->findMatchingBlocks($match->endInOld(), $endInOld, $match->endInNew(), $endInNew, $matchingBlocks);
-            }
+        $matchingBlocks[] = $match;
+
+        if ($match->endInOld() < $endInOld && $match->endInNew() < $endInNew) {
+            $this->findMatchingBlocks($match->endInOld(), $endInOld, $match->endInNew(), $endInNew, $matchingBlocks);
         }
     }
 
@@ -766,70 +764,71 @@ protected function stripTagAttributes($word)
     {
         $space = $this->stringUtil->strpos($word, ' ', 1);
 
-        if ($space) {
+        if ($space > 0) {
             return '<' . $this->stringUtil->substr($word, 1, $space) . '>';
         }
 
         return trim($word, '<>');
     }
 
-    /**
-     * @param int $startInOld
-     * @param int $endInOld
-     * @param int $startInNew
-     * @param int $endInNew
-     *
-     * @return MatchingBlock|null
-     */
-    protected function findMatch($startInOld, $endInOld, $startInNew, $endInNew)
+    protected function findMatch(int $startInOld, int $endInOld, int $startInNew, int $endInNew) : ?MatchingBlock
     {
         $groupDiffs     = $this->isGroupDiffs();
         $bestMatchInOld = $startInOld;
         $bestMatchInNew = $startInNew;
         $bestMatchSize = 0;
-        $matchLengthAt = array();
+        $matchLengthAt = [];
 
         for ($indexInOld = $startInOld; $indexInOld < $endInOld; ++$indexInOld) {
-            $newMatchLengthAt = array();
+            $newMatchLengthAt = [];
+
             $index = $this->oldWords[ $indexInOld ];
-            if ($this->isTag($index)) {
+
+            if ($this->isTag($index) === true) {
                 $index = $this->stripTagAttributes($index);
             }
-            if (!isset($this->wordIndices[ $index ])) {
+
+            if (isset($this->wordIndices[$index]) === false) {
                 $matchLengthAt = $newMatchLengthAt;
+
                 continue;
             }
-            foreach ($this->wordIndices[ $index ] as $indexInNew) {
+
+            foreach ($this->wordIndices[$index] as $indexInNew) {
                 if ($indexInNew < $startInNew) {
                     continue;
                 }
+
                 if ($indexInNew >= $endInNew) {
                     break;
                 }
 
-                $newMatchLength = (isset($matchLengthAt[ $indexInNew - 1 ]) ? $matchLengthAt[ $indexInNew - 1 ] : 0) + 1;
-                $newMatchLengthAt[ $indexInNew ] = $newMatchLength;
+                $newMatchLength =
+                    (isset($matchLengthAt[$indexInNew - 1]) === true ? ($matchLengthAt[$indexInNew - 1] + 1) : 1);
+
+                $newMatchLengthAt[$indexInNew] = $newMatchLength;
 
                 if ($newMatchLength > $bestMatchSize ||
                     (
-                        $groupDiffs &&
+                        $groupDiffs === true &&
                         $bestMatchSize > 0 &&
-                        $this->isOnlyWhitespace($this->array_slice_cached($this->oldWords, $bestMatchInOld, $bestMatchSize))
+                        $this->oldTextIsOnlyWhitespace($bestMatchInOld, $bestMatchSize) === true
                     )
                 ) {
                     $bestMatchInOld = $indexInOld - $newMatchLength + 1;
                     $bestMatchInNew = $indexInNew - $newMatchLength + 1;
-                    $bestMatchSize = $newMatchLength;
+                    $bestMatchSize  = $newMatchLength;
                 }
             }
+
             $matchLengthAt = $newMatchLengthAt;
         }
 
         // Skip match if none found or match consists only of whitespace
         if ($bestMatchSize !== 0 &&
             (
-                !$groupDiffs ||
-                !$this->isOnlyWhitespace($this->array_slice_cached($this->oldWords, $bestMatchInOld, $bestMatchSize))
+                $groupDiffs === false ||
+                $this->oldTextIsOnlyWhitespace($bestMatchInOld, $bestMatchSize) === false
             )
         ) {
             return new MatchingBlock($bestMatchInOld, $bestMatchInNew, $bestMatchSize);
@@ -838,40 +837,16 @@ protected function findMatch($startInOld, $endInOld, $startInNew, $endInNew)
         return null;
     }
 
-    /**
-     * @param string $str
-     *
-     * @return bool
-     */
-    protected function isOnlyWhitespace($str)
+    protected function oldTextIsOnlyWhitespace(int $startingAtWord, int $wordCount) : bool
     {
-        //  Slightly faster then using preg_match
-        return $str !== '' && trim($str) === '';
-    }
+        $isWhitespace = true;
 
-    /**
-     * Special array_slice function that caches its last request.
-     *
-     * The diff algorithm seems to request the same information many times in a row.
-     * by returning the previous answer the algorithm preforms way faster.
-     *
-     * The result is a string instead of an array, this way we safe on the amount of
-     * memory intensive implode() calls.
-     *
-     * @param array         &$array
-     * @param integer       $offset
-     * @param integer|null  $length
-     *
-     * @return string
-     */
-    protected function array_slice_cached(&$array, $offset, $length = null)
-    {
-        static $lastOffset = null;
-        static $lastLength = null;
-        static $cache      = null;
+        // oldTextIsWhitespace get called consecutively by findMatch, with the same parameters.
+        // by caching the previous result, we speed up the algorithm by more then 50%
+        static $lastStartingWordOffset = null;
+        static $lastWordCount          = null;
+        static $cache                  = null;
 
-        // PHP has no support for by-reference comparing.
-        // to prevent false positive hits, reset the cache when the oldWords or newWords is changed.
         if ($this->resetCache === true) {
             $cache = null;
 
@@ -880,16 +855,28 @@ protected function array_slice_cached(&$array, $offset, $length = null)
 
         if (
             $cache !== null &&
-            $lastLength === $length &&
-            $lastOffset === $offset
+            $lastWordCount === $wordCount &&
+            $lastStartingWordOffset === $startingAtWord
         ) { // Hit
             return $cache;
         } // Miss
 
-        $lastOffset = $offset;
-        $lastLength = $length;
+        for ($index = $startingAtWord; $index < ($startingAtWord + $wordCount); $index++) {
+            // Assigning the oldWord to a variable is slightly faster then searching by reference twice
+            // in the if statement
+            $oldWord = $this->oldWords[$index];
+
+            if ($oldWord !== '' && trim($oldWord) !== '') {
+                $isWhitespace = false;
+
+                break;
+            }
+        }
+
+        $lastWordCount          = $wordCount;
+        $lastStartingWordOffset = $startingAtWord;
 
-        $cache = implode('', array_slice($array, $offset, $length));
+        $cache = $isWhitespace;
 
         return $cache;
     }