Mark reference arguments at the call site (#90)

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 #89
facebookarchive · Jan 30, 2018 · f747fc5 · f747fc5
1 parent aa38192
commit f747fc5
Show file tree

Hide file tree

Showing 11 changed files with 18 additions and 15 deletions.
diff --git 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
@@ -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
@@ -177,7 +177,7 @@ private static function GetPercentiles(
     Vector<float> $times,
   ): Map<string, float> {
     $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
@@ -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
@@ -281,7 +281,7 @@ public function __construct(Vector<string> $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
@@ -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
@@ -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
@@ -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
@@ -17,14 +17,14 @@ public function collectStats(): Map<string, Map<string, num>> {
       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
@@ -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
@@ -125,7 +125,7 @@ public function install(): void {
       exit(0);
     }
     $status = null;
-    pcntl_waitpid($child, $status);
+    pcntl_waitpid($child, &$status);
   }
 
   private function getInstallerArgs(): array {