Add various flags to the HgProxy daemons

Summary:
  - Add flags to exit after an idle time or client count.
  - Add flags to control daemonization.
  - Add flags to control output.
  - Add flags to skip the "hello" frame of the protocol.
  - Make the client launch a server if one does not exist.

The one-time overhead to launch a server and run a command through it looks to be ~130% of the overhead to run the command directly with "hg", so even if we never run a second command we're not paying too much.

The incremental overhead to run subsequent command appears to be less than 3% of the overhead to run the command directly with "hg" (and maybe less than 1%, I'm not sure how long the computation part of a command like 'hg log' "actually" takes).

The overhead to launch a PHP client, connect to an existing server, run a command, and then print it and exit is roughly 50% of the overhead to run the command directly with "hg". So theoretically a user can achieve an amortized 2x performance increase for all 'hg' commands by aliasing 'hg' to the PHP client in their shell.

Test Plan:
  - Ran servers with idle and client count limits, let them idle and/or hit their connection limits, saw them exit.
  - Ran foreground and background servers.
  - Ran a daemon server with redirected stdout/stderr. Verified logs appeared.
  - Ran with --quiet.
  - Ran clients and servers with and without --skip-hello, things work if they agree and break if they disagree. The throughput gain on this is fairly small (maybe 5%?) but it seems simple enough to keep for the moment.
  - Ran serverless clients and verified that servers launched the first time, were available subsequently, and relaunched after 15 seconds idle.

Reviewers: csilvers, vrana, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2680

scripts/hgdaemon/hgdaemon_client.php

@@ -23,6 +23,13 @@ $args = new PhutilArgumentParser($argv);
 $args->parseStandardArguments();
 $args->parse(
   array(
+    array(
+      'name'      => 'skip-hello',
+      'help'      => 'Do not expect "capability" message when connecting. '.
+                     'The server must be configured not to send the message. '.
+                     'This deviates from the Mercurial protocol, but slightly '.
+                     'improves performance.',
+    ),
     array(
       'name'      => 'repository',
       'wildcard'  => true,
@@ -35,14 +42,15 @@ if (count($repo) !== 1) {
 }
 $repo = head($repo);
 
-$daemon = new ArcanistHgProxyClient($repo);
+$client = new ArcanistHgProxyClient($repo);
+$client->setSkipHello($args->getArg('skip-hello'));
 
 $t_start = microtime(true);
 
-$result = $daemon->executeCommand(
+$result = $client->executeCommand(
   array('log', '--template', '{node}', '--rev', 2));
 
 $t_end   = microtime(true);
 var_dump($result);
 
-echo "\nExecuted in ".((int)(1000 * ($t_end - $t_start)))."ms.\n";
+echo "\nExecuted in ".((int)(1000000 * ($t_end - $t_start)))."us.\n";

scripts/hgdaemon/hgdaemon_server.php

@@ -23,6 +23,31 @@ $args = new PhutilArgumentParser($argv);
 $args->parseStandardArguments();
 $args->parse(
   array(
+    array(
+      'name'      => 'quiet',
+      'help'      => 'Do not print status messages to stdout.',
+    ),
+    array(
+      'name'      => 'skip-hello',
+      'help'      => 'Do not send "capability" message when clients connect. '.
+                     'Clients must be configured not to expect the message. '.
+                     'This deviates from the Mercurial protocol, but slightly '.
+                     'improves performance.',
+    ),
+    array(
+      'name'      => 'do-not-daemonize',
+      'help'      => 'Remain in the foreground instead of daemonizing.',
+    ),
+    array(
+      'name'      => 'client-limit',
+      'param'     => 'limit',
+      'help'      => 'Exit after serving __limit__ clients.',
+    ),
+    array(
+      'name'      => 'idle-limit',
+      'param'     => 'seconds',
+      'help'      => 'Exit after __seconds__ spent idle.',
+    ),
     array(
       'name'      => 'repository',
       'wildcard'  => true,
@@ -35,5 +60,10 @@ if (count($repo) !== 1) {
 }
 $repo = head($repo);
 
-$daemon = new ArcanistHgProxyServer($repo);
+id(new ArcanistHgProxyServer($repo))
-$daemon->start();
+  ->setQuiet($args->getArg('quiet'))
+  ->setClientLimit($args->getArg('client-limit'))
+  ->setIdleLimit($args->getArg('idle-limit'))
+  ->setDoNotDaemonize($args->getArg('do-not-daemonize'))
+  ->setSkipHello($args->getArg('skip-hello'))
+  ->start();

src/hgdaemon/ArcanistHgProxyClient.php

@@ -39,6 +39,7 @@
  * which is often on the order of 100ms or more per command.
  *
  * @task construct  Construction
+ * @task config     Configuration
  * @task exec       Executing Mercurial Commands
  * @task internal   Internals
  */
@@ -47,6 +48,8 @@ final class ArcanistHgProxyClient {
   private $workingCopy;
   private $server;
 
+  private $skipHello;
+
 
 /* -(  Construction  )------------------------------------------------------- */
 
@@ -64,6 +67,23 @@ final class ArcanistHgProxyClient {
   }
 
 
+/* -(  Configuration  )------------------------------------------------------ */
+
+
+  /**
+   * When connecting, do not expect the "capabilities" message.
+   *
+   * @param bool True to skip the "capabilities" message.
+   * @return this
+   *
+   * @task config
+   */
+  public function setSkipHello($skip) {
+    $this->skipHello = $skip;
+    return $this;
+  }
+
+
 /* -(  Executing Merucurial Commands  )-------------------------------------- */
 
 
@@ -77,7 +97,15 @@ final class ArcanistHgProxyClient {
    */
   public function executeCommand(array $argv) {
     if (!$this->server) {
-      $this->server = $this->connectToDaemon();
+
+      try {
+        $server = $this->connectToDaemon();
+      } catch (Exception $ex) {
+        $this->launchDaemon();
+        $server = $this->connectToDaemon();
+      }
+
+      $this->server = $server;
     }
     $server = $this->server;
 
@@ -148,7 +176,7 @@ final class ArcanistHgProxyClient {
     $errstr = null;
 
     $socket_path = ArcanistHgProxyServer::getPathToSocket($this->workingCopy);
-    $socket = stream_socket_client('unix://'.$socket_path, $errno, $errstr);
+    $socket = @stream_socket_client('unix://'.$socket_path, $errno, $errstr);
 
     if ($errno || !$socket) {
       throw new Exception(
@@ -158,12 +186,29 @@ final class ArcanistHgProxyClient {
     $channel = new PhutilSocketChannel($socket);
     $server = new ArcanistHgServerChannel($channel);
 
-    // The protocol includes a "hello" message with capability and encoding
+    if (!$this->skipHello) {
-    // information. Read and discard it, we use only the "runcommand" capability
+      // The protocol includes a "hello" message with capability and encoding
-    // which is guaranteed to be available.
+      // information. Read and discard it, we use only the "runcommand"
-    $hello = $server->waitForMessage();
+      // capability which is guaranteed to be available.
+      $hello = $server->waitForMessage();
+    }
 
     return $server;
   }
 
+
+  /**
+   * @task internal
+   */
+  private function launchDaemon() {
+    $root = dirname(phutil_get_library_root('arcanist'));
+    $bin = $root.'/scripts/hgdaemon/hgdaemon_server.php';
+    $proxy = new ExecFuture(
+      '%s %s --idle-limit 15 --quiet %C',
+      $bin,
+      $this->workingCopy,
+      $this->skipHello ? '--skip-hello' : null);
+    $proxy->resolvex();
+  }
+
 }

src/hgdaemon/ArcanistHgProxyServer.php

@@ -36,6 +36,7 @@
  * and serve them from a single Mercurial server process.
  *
  * @task construct  Construction
+ * @task config     Configuration
  * @task server     Serving Requests
  * @task client     Managing Clients
  * @task hg         Managing Mercurial
@@ -47,6 +48,18 @@ final class ArcanistHgProxyServer {
   private $socket;
   private $hello;
 
+  private $quiet;
+
+  private $clientLimit;
+  private $lifetimeClientCount;
+
+  private $idleLimit;
+  private $idleSince;
+
+  private $skipHello;
+
+  private $doNotDaemonize;
+
 
 /* -(  Construction  )------------------------------------------------------- */
 
@@ -64,37 +77,123 @@ final class ArcanistHgProxyServer {
   }
 
 
+/* -(  Configuration  )------------------------------------------------------ */
+
+
+  /**
+   * Disable status messages to stdout. Controlled with `--quiet`.
+   *
+   * @param bool  True to disable status messages.
+   * @return this
+   *
+   * @task config
+   */
+  public function setQuiet($quiet) {
+    $this->quiet = $quiet;
+    return $this;
+  }
+
+
+  /**
+   * Configure a client limit. After serving this many clients, the server
+   * will exit. Controlled with `--client-limit`.
+   *
+   * You can use `--client-limit 1` with `--xprofile` and `--do-not-daemonize`
+   * to profile the server.
+   *
+   * @param int Client limit, or 0 to disable limit.
+   * @return this
+   *
+   * @task config
+   */
+  public function setClientLimit($limit) {
+    $this->clientLimit = $limit;
+    return $this;
+  }
+
+
+  /**
+   * Configure an idle time limit. After this many seconds idle, the server
+   * will exit. Controlled with `--idle-limit`.
+   *
+   * @param int Idle limit, or 0 to disable limit.
+   * @return this
+   *
+   * @task config
+   */
+  public function setIdleLimit($limit) {
+    $this->idleLimit = $limit;
+    return $this;
+  }
+
+
+  /**
+   * When clients connect, do not send the "capabilities" message expected by
+   * the Mercurial protocol. This deviates from the protocol and will only work
+   * if the clients are also configured not to expect the message, but slightly
+   * improves performance. Controlled with --skip-hello.
+   *
+   * @param bool True to skip the "capabilities" message.
+   * @return this
+   *
+   * @task config
+   */
+  public function setSkipHello($skip) {
+    $this->skipHello = $skip;
+    return $this;
+  }
+
+
+  /**
+   * Configure whether the server runs in the foreground or daemonizes.
+   * Controlled by --do-not-daemonize. Primarily useful for debugging.
+   *
+   * @param bool True to run in the foreground.
+   * @return this
+   *
+   * @task config
+   */
+  public function setDoNotDaemonize($do_not_daemonize) {
+    $this->doNotDaemonize = $do_not_daemonize;
+    return $this;
+  }
+
+
 /* -(  Serving Requests  )--------------------------------------------------- */
 
 
   /**
-   * Start the server. This method does not return.
+   * Start the server. This method returns after the client limit or idle
+   * limit are exceeded. If neither limit is configured, this method does not
+   * exit.
    *
-   * @return never
+   * @return null
    *
    * @task server
    */
   public function start() {
-
     // Create the unix domain socket in the working copy to listen for clients.
     $socket = $this->startWorkingCopySocket();
     $this->socket = $socket;
 
-    // TODO: Daemonize here.
+    if (!$this->doNotDaemonize) {
+      $this->daemonize();
+    }
 
     // Start the Mercurial process which we'll forward client requests to.
     $hg = $this->startMercurialProcess();
     $clients = array();
 
     $this->log(null, 'Listening');
+    $this->idleSince = time();
     while (true) {
       // Wait for activity on any active clients, the Mercurial process, or
       // the listening socket where new clients connect.
       PhutilChannel::waitForAny(
         array_merge($clients, array($hg)),
         array(
-          'read'    => array($socket),
+          'read'    => $socket ? array($socket) : array(),
-          'except'  => array($socket),
+          'except'  => $socket ? array($socket) : array()
         ));
 
       if (!$hg->update()) {
@@ -102,20 +201,61 @@ final class ArcanistHgProxyServer {
       }
 
       // Accept any new clients.
-      while ($client = $this->acceptNewClient($socket)) {
+      while ($socket && ($client = $this->acceptNewClient($socket))) {
         $clients[] = $client;
         $key = last_key($clients);
         $client->setName($key);
 
         $this->log($client, 'Connected');
+        $this->idleSince = time();
+
+        // Check if we've hit the client limit. If there's a configured
+        // client limit and we've hit it, stop accepting new connections
+        // and close the socket.
+
+        $this->lifetimeClientCount++;
+
+        if ($this->clientLimit) {
+          if ($this->lifetimeClientCount >= $this->clientLimit) {
+            $this->closeSocket();
+            $socket = null;
+          }
+        }
       }
 
       // Update all the active clients.
       foreach ($clients as $key => $client) {
-        $ok = $this->updateClient($client, $hg);
+        if ($this->updateClient($client, $hg)) {
-        if (!$ok) {
+          // In this case, the client is still connected so just move on to
-          $this->log($client, 'Disconnected');
+          // the next one. Otherwise we continue below and handle the disconect.
-          unset($clients[$key]);
+          continue;
+        }
+
+        $this->log($client, 'Disconnected');
+        unset($clients[$key]);
+
+        // If we have a client limit and we've served that many clients, exit.
+
+        if ($this->clientLimit) {
+          if ($this->lifetimeClientCount >= $this->clientLimit) {
+            if (!$clients) {
+              $this->log(null, 'Exiting (Client Limit)');
+              return;
+            }
+          }
+        }
+      }
+
+      // If we have an idle limit and haven't had any activity in at least
+      // that long, exit.
+      if ($this->idleLimit) {
+        $remaining = $this->idleLimit - (time() - $this->idleSince);
+        if ($remaining <= 0) {
+          $this->log(null, 'Exiting (Idle Limit)');
+          return;
+        }
+        if ($remaining <= 5) {
+          $this->log(null, 'Exiting in '.$remaining.' seconds');
         }
       }
     }
@@ -189,6 +329,8 @@ final class ArcanistHgProxyServer {
     $t = 1000000 * ($t_end - $t_start);
     $this->log($client, '< '.number_format($t, 0).'us');
 
+    $this->idleSince = time();
+
     return true;
   }
 
@@ -242,13 +384,15 @@ final class ArcanistHgProxyServer {
     // been set nonblocking.
     $new_client = @stream_socket_accept($socket, $timeout = 0);
     if (!$new_client) {
-      return;
+      return null;
     }
 
     $channel = new PhutilSocketChannel($new_client);
     $client = new ArcanistHgClientChannel($channel);
 
-    $client->write($this->hello);
+    if (!$this->skipHello) {
+      $client->write($this->hello);
+    }
 
     return $client;
   }
@@ -292,6 +436,10 @@ final class ArcanistHgProxyServer {
    * @task internal
    */
   public function __destruct() {
+    $this->closeSocket();
+  }
+
+  private function closeSocket() {
     if ($this->socket) {
       @stream_socket_shutdown($this->socket, STREAM_SHUT_RDWR);
       @fclose($this->socket);
@@ -301,12 +449,56 @@ final class ArcanistHgProxyServer {
   }
 
   private function log($client, $message) {
+    if ($this->quiet) {
+      return;
+    }
+
     if ($client) {
       $message = '[Client '.$client->getName().'] '.$message;
     } else {
       $message = '[Server] '.$message;
     }
+
     echo $message."\n";
   }
 
+  private function daemonize() {
+
+    // Keep stdout if it's been redirected somewhere, otherwise shut it down.
+    $keep_stdout = false;
+    $keep_stderr = false;
+    if (function_exists('posix_isatty')) {
+      if (!posix_isatty(STDOUT)) {
+        $keep_stdout = true;
+      }
+      if (!posix_isatty(STDERR)) {
+        $keep_stderr = true;
+      }
+    }
+
+    $pid = pcntl_fork();
+    if ($pid === -1) {
+      throw new Exception("Unable to fork!");
+    } else if ($pid) {
+      // We're the parent; exit. First, drop our reference to the socket so
+      // our __destruct() doesn't tear it down; the child will tear it down
+      // later.
+      $this->socket = null;
+      exit(0);
+    }
+
+    // We're the child; continue.
+
+    fclose(STDIN);
+
+    if (!$keep_stdout) {
+      fclose(STDOUT);
+      $this->quiet = true;
+    }
+
+    if (!$keep_stderr) {
+      fclose(STDERR);
+    }
+  }
+
 }