From 6201acf9566864d40d10c40de99d464653192e1e Mon Sep 17 00:00:00 2001 From: Sushma Kyasaralli Thimmappa Date: Wed, 17 Jan 2018 17:55:58 -0800 Subject: [PATCH] Mark reference arguments at the call site HHVM 3.24 requires the reference arguments to be marked at the call site, following which the current oss-performance code throws errors when "hh_client" is run or the workload is run using HHVM runtime. This patch fixes the issue by marking the reference arguments at the call time. However, this patch also mandates the HHVM version to be at least 3.24 and makes it incompatible with the earlier versions as the change is not supported in earlier versions of HHVM Fixes https://github.com/hhvm/oss-performance/issues/89 --- base/DatabaseInstaller.php | 4 ++-- base/HHVMDaemon.php | 2 +- base/NginxDaemon.php | 2 +- base/PHP5Daemon.php | 4 ++-- base/PerfOptions.php | 4 ++-- base/PerfRunner.php | 2 +- base/PerfTarget.php | 2 +- base/Process.php | 4 ++-- base/SiegeStats.php | 4 ++-- composer.json | 3 +++ targets/magento1/Magento1Target.php | 2 +- 11 files changed, 18 insertions(+), 15 deletions(-) diff --git a/base/DatabaseInstaller.php b/base/DatabaseInstaller.php index 4d52923..19c1906 100644 --- a/base/DatabaseInstaller.php +++ b/base/DatabaseInstaller.php @@ -90,8 +90,8 @@ public function installDatabase(): bool { Utils::EscapeCommand( Vector {'mysql', '-h', $dbHost.'', $db, '-u', $db, '-p'.$db}, ), - $output, - $ret, + &$output, + &$ret, ); if ($ret !== 0) { diff --git a/base/HHVMDaemon.php b/base/HHVMDaemon.php index d790ead..9e3c15e 100644 --- a/base/HHVMDaemon.php +++ b/base/HHVMDaemon.php @@ -32,7 +32,7 @@ public function __construct(private PerfOptions $options) { if ($options->traceSubProcess) { fprintf(STDERR, "%s\n", $check_command); } - exec($check_command, $output); + exec($check_command, &$output); $checks = json_decode(implode("\n", $output), /* as array = */ true); invariant($checks, 'Got invalid output from hhvm_config_check.php'); if (array_key_exists('HHVM_VERSION', $checks)) { diff --git a/base/NginxDaemon.php b/base/NginxDaemon.php index 5980276..3998001 100644 --- a/base/NginxDaemon.php +++ b/base/NginxDaemon.php @@ -177,7 +177,7 @@ private static function GetPercentiles( Vector $times, ): Map { $count = count($times); - sort($times); + sort(&$times); return Map { 'Nginx P50 time' => $times[(int) ($count * 0.5)], 'Nginx P90 time' => $times[(int) ($count * 0.9)], diff --git a/base/PHP5Daemon.php b/base/PHP5Daemon.php index faa73ca..a504879 100644 --- a/base/PHP5Daemon.php +++ b/base/PHP5Daemon.php @@ -32,7 +32,7 @@ public function __construct(private PerfOptions $options) { if ($options->traceSubProcess) { fprintf(STDERR, "%s\n", $check_command); } - exec($check_command, $output); + exec($check_command, &$output); $check = array_search('Opcode Caching => Up and Running', $output, true); invariant($check, 'Got invalid output from php-fpm -i'); } else { @@ -51,7 +51,7 @@ public function __construct(private PerfOptions $options) { if ($options->traceSubProcess) { fprintf(STDERR, "%s\n", $check_command); } - exec($check_command, $output); + exec($check_command, &$output); $checks = json_decode(implode("\n", $output), /* as array = */ true); invariant($checks, 'Got invalid output from php-src_config_check.php'); BuildChecker::Check( diff --git a/base/PerfOptions.php b/base/PerfOptions.php index b8c2cb3..a0247cc 100644 --- a/base/PerfOptions.php +++ b/base/PerfOptions.php @@ -281,7 +281,7 @@ public function __construct(Vector $argv) { if ($fraction !== 1.0) { $this->cpuBind = true; $output = []; - exec('nproc', $output); + exec('nproc', &$output); $numProcessors = (int)($output[0]); $numDaemonProcessors = (int)($numProcessors * $fraction); $this->helperProcessors = "$numDaemonProcessors-$numProcessors"; @@ -391,7 +391,7 @@ public function validate() { $ret = 0; $output = ""; $this->siegeTmpDir = exec('ssh ' . - $this->remoteSiege . ' mktemp -d ', $output, $ret); + $this->remoteSiege . ' mktemp -d ', &$output, &$ret); if ($ret) { invariant_violation('%s', 'Invalid ssh credentials: ' . $this->remoteSiege); diff --git a/base/PerfRunner.php b/base/PerfRunner.php index 3871613..30301ae 100644 --- a/base/PerfRunner.php +++ b/base/PerfRunner.php @@ -215,7 +215,7 @@ private static function RunWithOptionsAndEngine( $combined_stats = $combined_stats->filterWithKey(($k, $v) ==> $k === 'Combined'); } else { - ksort($combined_stats); + ksort(&$combined_stats); } $combined_stats['Combined']['canonical'] = (int) !$options->notBenchmarking; diff --git a/base/PerfTarget.php b/base/PerfTarget.php index 982a080..08c84d1 100644 --- a/base/PerfTarget.php +++ b/base/PerfTarget.php @@ -52,7 +52,7 @@ final public function applyPatches(): void { } $patches = glob($dir.'/*.patch'); - sort($patches); + sort(&$patches); $dir = escapeshellarg($this->getSourceRoot()); diff --git a/base/Process.php b/base/Process.php index 24eed2d..feef9a1 100644 --- a/base/Process.php +++ b/base/Process.php @@ -79,7 +79,7 @@ public function startWorker( } } - $proc = proc_open($this->command, $spec, $pipes, null, $env); + $proc = proc_open($this->command, $spec, &$pipes, null, $env); // Give the shell some time to figure out if it could actually launch the // process @@ -163,7 +163,7 @@ public function wait(): void { return; } $status = null; - pcntl_waitpid($pid, $status); + pcntl_waitpid($pid, &$status); } public function __destruct() { diff --git a/base/SiegeStats.php b/base/SiegeStats.php index 8e27700..f5a9516 100644 --- a/base/SiegeStats.php +++ b/base/SiegeStats.php @@ -17,14 +17,14 @@ public function collectStats(): Map> { explode("\n", trim(file_get_contents($this->getLogFilePath()))); if (count($log_lines) > 1) { // Remove possible header line - array_splice($log_lines, 0, 1); + array_splice(&$log_lines, 0, 1); } invariant( count($log_lines) === 1, 'Expected 1 line in siege log file, got %d', count($log_lines), ); - $log_line = array_pop($log_lines); + $log_line = array_pop(&$log_lines); $data = (new Vector(explode(',', $log_line)))->map($x ==> trim($x)); return Map { 'Combined' => Map { diff --git a/composer.json b/composer.json index 5754465..30dfe5a 100644 --- a/composer.json +++ b/composer.json @@ -1,4 +1,7 @@ { + "require-dev": { + "hhvm": "^3.24" + }, "autoload": { "classmap": ["base/", "targets/"] } diff --git a/targets/magento1/Magento1Target.php b/targets/magento1/Magento1Target.php index aba4b3e..60db330 100644 --- a/targets/magento1/Magento1Target.php +++ b/targets/magento1/Magento1Target.php @@ -125,7 +125,7 @@ public function install(): void { exit(0); } $status = null; - pcntl_waitpid($child, $status); + pcntl_waitpid($child, &$status); } private function getInstallerArgs(): array {