Benchmark for JUnitTestCaseSorter::uniqueByTestFile (#1177)

* Sample of JUnit test timing from a real project

* Implement bucket sort

Co-Authored-By: Théo FIDRY <theo.fidry@gmail.com>

devTools/phpstan-src.neon

@@ -32,10 +32,6 @@ parameters:
             path: '../src/Container.php'
             message: '#^Parameter \#1 \$processHandler of class Infection\\Process\\Runner\\ParallelProcessRunner constructor expects Closure\(Infection\\Process\\Runner\\ProcessBearer\)\: void\, Closure\(Infection\\Process\\MutantProcess\): void given\.$#'
             count: 1
-        -
-            path: '../src/TestFramework/Coverage/JUnit/JUnitTestCaseSorter.php'
-            message: '#array_key_exists expects int\|string, string\|null given#'
-            count: 1
         -
             path: '../src/FileSystem/DummyFileSystem.php'
             message: '#Infection\\FileSystem\\DummyFileSystem#'

src/Process/Runner/MutationTestingRunner.php

@@ -90,6 +90,7 @@ public function run(iterable $mutations, string $testFrameworkExtraOptions): voi
                 return $this->mutantFactory->create($mutation);
             })
             ->filter(function (Mutant $mutant) {
+                // It's a proxy call to Mutation, can be done one stage up
                 if ($mutant->isCoveredByTest()) {
                     return true;
                 }

src/TestFramework/Coverage/JUnit/JUnitTestCaseSorter.php

@@ -36,6 +36,7 @@
 namespace Infection\TestFramework\Coverage\JUnit;
 
 use function array_key_exists;
+use function count;
 use function current;
 use Infection\AbstractTestFramework\Coverage\TestLocation;
 use function Safe\usort;
@@ -45,44 +46,85 @@
  */
 final class JUnitTestCaseSorter
 {
+    /**
+     * Expected average number of buckets. Exposed for testing purposes.
+     *
+     * @var int[]
+     */
+    public const BUCKETS_COUNT = 25;
+
+    /**
+     * For 25 buckets QS becomes theoretically less efficient on average at and after 15 elements.
+     * Exposed for testing purposes.
+     */
+    public const USE_BUCKET_SORT_AFTER = 15;
+
     /**
      * @param TestLocation[] $tests
      *
-     * @return string[]
+     * @return iterable<string>
      */
     public function getUniqueSortedFileNames(array $tests): iterable
     {
         $uniqueTestLocations = $this->uniqueByTestFile($tests);
 
-        if (count($uniqueTestLocations) === 1) {
+        $numberOfTestLocation = count($uniqueTestLocations);
+
+        if ($numberOfTestLocation === 1) {
             // Around 5% speed up compared to when without this optimization.
             /** @var TestLocation $testLocation */
             $testLocation = current($uniqueTestLocations);
 
-            $filePath = $testLocation->getFilePath();
+            /*
+             * TestLocation gets its file path and timings from TestFileTimeData.
+             * Path for TestFileTimeData is not optional. It is never a null.
+             * Therefore we don't need to make any type checks here.
+             */
 
-            if ($filePath !== null) {
-                yield $filePath;
-            }
+            /** @var string $filePath */
+            $filePath = $testLocation->getFilePath();
 
-            return;
+            return [$filePath];
         }
 
         /*
+         * We need to sort tests to run the fastest first.
+         *
          * Two tests per file are also very frequent. Yet it doesn't make sense
          * to sort them by hand: usort does that just as good.
          */
 
-        // sort tests to run the fastest first
-        usort(
-            $uniqueTestLocations,
-            static function (TestLocation $a, TestLocation $b) {
-                return $a->getExecutionTime() <=> $b->getExecutionTime();
-            }
+        if ($numberOfTestLocation < self::USE_BUCKET_SORT_AFTER) {
+            usort(
+                $uniqueTestLocations,
+                static function (TestLocation $a, TestLocation $b) {
+                    return $a->getExecutionTime() <=> $b->getExecutionTime();
+                }
+            );
+
+            return self::sortedLocationsGenerator($uniqueTestLocations);
+        }
+
+        /*
+         * For large number of tests use a more efficient algorithm.
+         */
+        return self::sortedLocationsGenerator(
+            TestLocationBucketSorter::bucketSort($uniqueTestLocations)
         );
+    }
+
+    /**
+     * @param iterable<TestLocation> $sortedTestLocations
+     *
+     * @return iterable<string>
+     */
+    private static function sortedLocationsGenerator(iterable $sortedTestLocations): iterable
+    {
+        foreach ($sortedTestLocations as $testLocation) {
+            /** @var string $filePath */
+            $filePath = $testLocation->getFilePath();
 
-        foreach ($uniqueTestLocations as $testLocation) {
-            yield $testLocation->getFilePath();
+            yield $filePath;
         }
     }
 
@@ -98,9 +140,11 @@ private function uniqueByTestFile(array $testLocations): array
         $uniqueTests = [];
 
         foreach ($testLocations as $testLocation) {
+            /** @var string $filePath */
             $filePath = $testLocation->getFilePath();
 
-            if (!array_key_exists($filePath, $usedFileNames)) {
+            // isset() is 20% faster than array_key_exists() as of PHP 7.3
+            if (!isset($usedFileNames[$filePath])) {
                 $uniqueTests[] = $testLocation;
                 $usedFileNames[$filePath] = true;
             }

src/TestFramework/Coverage/JUnit/TestLocationBucketSorter.php

@@ -0,0 +1,98 @@
+<?php
+/**
+ * This code is licensed under the BSD 3-Clause License.
+ *
+ * Copyright (c) 2017, Maks Rafalko
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice, this
+ *   list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of the copyright holder nor the names of its
+ *   contributors may be used to endorse or promote products derived from
+ *   this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+declare(strict_types=1);
+
+namespace Infection\TestFramework\Coverage\JUnit;
+
+use Infection\AbstractTestFramework\Coverage\TestLocation;
+use function Safe\ksort;
+
+/**
+ * @internal
+ */
+final class TestLocationBucketSorter
+{
+    /**
+     * Pre-sort first buckets, optimistically assuming that most projects
+     * won't have tests longer than a second.
+     */
+    private const INIT_BUCKETS = [
+        0 => [],
+        1 => [],
+        2 => [],
+        3 => [],
+        4 => [],
+        5 => [],
+        6 => [],
+        7 => [],
+    ];
+
+    private function __construct()
+    {
+    }
+
+    /**
+     * Sorts tests to run the fastest first. Exposed for benchmarking purposes.
+     *
+     * @param TestLocation[] $uniqueTestLocations
+     *
+     * @return iterable<TestLocation>
+     */
+    public static function bucketSort(array $uniqueTestLocations): iterable
+    {
+        $buckets = self::INIT_BUCKETS;
+
+        foreach ($uniqueTestLocations as $location) {
+            // @codeCoverageIgnoreStart
+            // This is a very hot path. Factoring here another method just to test this math may not be as good idea.
+
+            // Quick drop off lower bits, reducing precision to 8th of a second
+            $msTime = $location->getExecutionTime() * 1024 >> 7; // * 1024 / 128
+
+            // For anything above 4 seconds reduce precision to 4 seconds
+            if ($msTime > 32) {
+                $msTime = $msTime >> 5 << 5; // 7 + 5 = 12 bits
+            }
+            // @codeCoverageIgnoreEnd
+
+            $buckets[$msTime][] = $location;
+        }
+
+        ksort($buckets);
+
+        foreach ($buckets as $bucket) {
+            yield from $bucket;
+        }
+    }
+}