]> granicus.if.org Git - php/commitdiff
run-tests: add skip cache
authorMax Semenik <maxsem.wiki@gmail.com>
Thu, 11 Feb 2021 17:33:03 +0000 (20:33 +0300)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 23 Feb 2021 14:05:49 +0000 (15:05 +0100)
Currently every --SKIPIF-- section in every test file results in 1
extra execution of PHP, every --EXTENSIONS-- section - in 2 executions.
This is quite wasteful, as skip checking code is extremely repetitive
and extensions are fixed for every binary/ini/command parameters
combination.

This patch adds caching to all such checks.

On my machine, the gains are quite noticeable: 36s instead of 43s
with -j16, 292s instead of 337s without concurrency. Cache stats are
3780 hits, 1247 misses in the latter case. In the future, tests could
be adjusted to have more uniform skip checks to improve performance even
more.

Closes GH-6681.

run-tests.php

index 1dcd43268272f83a01278120bf494adfc884b9fb..f6819f95d81aedc0c85fb1f64ef10ac8c4a81d04 100755 (executable)
@@ -1832,6 +1832,11 @@ function run_test(string $php, $file, array $env): string
     /** @var JUnit */
     global $junit;
 
+    static $skipCache;
+    if (!$skipCache) {
+        $skipCache = new SkipCache($cfg['keep']['skip']);
+    }
+
     $temp_filenames = null;
     $org_file = $file;
 
@@ -2155,9 +2160,8 @@ TEST $file
         $ext_params = [];
         settings2array($ini_overwrites, $ext_params);
         $ext_params = settings2params($ext_params);
-        $ext_dir = `$php $pass_options $extra_options $ext_params $no_file_cache -d display_errors=0 -r "echo ini_get('extension_dir');"`;
         $extensions = preg_split("/[\n\r]+/", trim($section_text['EXTENSIONS']));
-        $loaded = explode(",", `$php $pass_options $extra_options $ext_params $no_file_cache -d display_errors=0 -r "echo implode(',', get_loaded_extensions());"`);
+        [$ext_dir, $loaded] = $skipCache->getExtensions("$php $pass_options $extra_options $ext_params $no_file_cache");
         $ext_prefix = IS_WINDOWS ? "php_" : "";
         foreach ($extensions as $req_ext) {
             if (!in_array($req_ext, $loaded)) {
@@ -2217,7 +2221,6 @@ TEST $file
     if (array_key_exists('SKIPIF', $section_text)) {
         if (trim($section_text['SKIPIF'])) {
             show_file_block('skip', $section_text['SKIPIF']);
-            save_text($test_skipif, $section_text['SKIPIF'], $temp_skipif);
             $extra = !IS_WINDOWS ?
                 "unset REQUEST_METHOD; unset QUERY_STRING; unset PATH_TRANSLATED; unset SCRIPT_FILENAME; unset REQUEST_METHOD;" : "";
 
@@ -2229,8 +2232,9 @@ TEST $file
             $junit->startTimer($shortname);
 
             $startTime = microtime(true);
-            $output = system_with_timeout("$extra $php $pass_options $extra_options -q $orig_ini_settings $no_file_cache -d display_errors=1 -d display_startup_errors=0 \"$test_skipif\"", $env);
-            $output = trim($output);
+            $commandLine = "$extra $php $pass_options $extra_options -q $orig_ini_settings $no_file_cache -d display_errors=1 -d display_startup_errors=0";
+            $output = $skipCache->checkSkip($commandLine, $section_text['SKIPIF'], $test_skipif, $temp_skipif, $env);
+
             $time = microtime(true) - $startTime;
 
             $junit->stopTimer($shortname);
@@ -2245,10 +2249,6 @@ TEST $file
                 ];
             }
 
-            if (!$cfg['keep']['skip']) {
-                @unlink($test_skipif);
-            }
-
             if (!strncasecmp('skip', $output, 4)) {
                 if (preg_match('/^skip\s*(.+)/i', $output, $m)) {
                     show_result('SKIP', $tested, $tested_file, "reason: $m[1]", $temp_filenames);
@@ -2256,10 +2256,6 @@ TEST $file
                     show_result('SKIP', $tested, $tested_file, '', $temp_filenames);
                 }
 
-                if (!$cfg['keep']['skip']) {
-                    @unlink($test_skipif);
-                }
-
                 $message = !empty($m[1]) ? $m[1] : '';
                 $junit->markTestAs('SKIP', $shortname, $tested, null, $message);
                 return 'SKIPPED';
@@ -3716,6 +3712,79 @@ class JUnit
     }
 }
 
+class SkipCache
+{
+    private bool $keepFile;
+
+    private array $skips = [];
+    private array $extensions = [];
+
+    private int $hits = 0;
+    private int $misses = 0;
+    private int $extHits = 0;
+    private int $extMisses = 0;
+
+    public function __construct(bool $keepFile)
+    {
+        $this->keepFile = $keepFile;
+    }
+
+    public function checkSkip(string $php, string $code, string $checkFile, string $tempFile, array $env): string
+    {
+        // Extension tests frequently use something like <?php require 'skipif.inc';
+        // for skip checks. This forces us to cache per directory to avoid pollution.
+        $dir = dirname($checkFile);
+        $key = "$php => $dir";
+
+        if (isset($this->skips[$key][$code])) {
+            $this->hits++;
+            if ($this->keepFile) {
+                save_text($checkFile, $code, $tempFile);
+            }
+            return $this->skips[$key][$code];
+        }
+
+        save_text($checkFile, $code, $tempFile);
+        $result = trim(system_with_timeout("$php \"$checkFile\"", $env));
+        $this->skips[$key][$code] = $result;
+        $this->misses++;
+
+        if (!$this->keepFile) {
+            @unlink($checkFile);
+        }
+
+        return $result;
+    }
+
+    public function getExtensions(string $php): array
+    {
+        if (isset($this->extensions[$php])) {
+            $this->extHits++;
+            return $this->extensions[$php];
+        }
+
+        $extDir = `$php -d display_errors=0 -r "echo ini_get('extension_dir');"`;
+        $extensions = explode(",", `$php -d display_errors=0 -r "echo implode(',', get_loaded_extensions());"`);
+
+        $result = [$extDir, $extensions];
+        $this->extensions[$php] = $result;
+        $this->extMisses++;
+
+        return $result;
+    }
+
+//    public function __destruct()
+//    {
+//        echo "Skips: {$this->hits} hits, {$this->misses} misses.\n";
+//        echo "Extensions: {$this->extHits} hits, {$this->extMisses} misses.\n";
+//        echo "Cache distribution:\n";
+//
+//        foreach ($this->skips as $php => $cache) {
+//            echo "$php: " . count($cache) . "\n";
+//        }
+//    }
+}
+
 class RuntestsValgrind
 {
     protected $version = '';