From a2ab38df78a9d111459fab8d0d63e74d811edf29 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 16 Apr 2016 09:38:17 -0700
Subject: [PATCH] Improve performance of `arc branch` in Git with many branches

Summary:
This is mostly just a personal quality-of-life fix. I run this command fairly often and having it return a little faster is nice.

This replaces a `git show` for each individual branch with a big `git for-each-ref` which we were already running anyway. This is quite a bit faster.

This command also occasionally hangs or segfaults for me while executing the huge pile of subprocesses. This is unreliable to reproduce, probably some bug in some PHP extension I have, and likely hard to narrow down, and this approach is better in every way anyway.

Test Plan:
  - Ran `arc branch` in Git, observed faster output (in my `phabricator/`, about 2000ms -> 1200ms).
  - Ran `arc feature` in Mercurial.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15735
---
 src/repository/api/ArcanistGitAPI.php    | 36 +++++++++++++---
 src/workflow/ArcanistFeatureWorkflow.php | 54 +++++++++++++-----------
 2 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
index 8600674c..884d65dc 100644
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -966,19 +966,43 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
    * @return list<dict<string, string>> Dictionary of branch information.
    */
   public function getAllBranches() {
-    list($ref_list) = $this->execxLocal(
-      'for-each-ref --format=%s refs/heads',
-      '%(refname)');
-    $refs = explode("\n", rtrim($ref_list));
+    $field_list = array(
+      '%(refname)',
+      '%(objectname)',
+      '%(committerdate:raw)',
+      '%(tree)',
+      '%(subject)',
+      '%(subject)%0a%0a%(body)',
+      '%02',
+    );
+
+    list($stdout) = $this->execxLocal(
+      'for-each-ref --format=%s -- refs/heads',
+      implode('%01', $field_list));
 
     $current = $this->getBranchName();
     $result = array();
-    foreach ($refs as $ref) {
+
+    $lines = explode("\2", $stdout);
+    foreach ($lines as $line) {
+      $line = trim($line);
+      if (!strlen($line)) {
+        continue;
+      }
+
+      $fields = explode("\1", $line, 6);
+      list($ref, $hash, $epoch, $tree, $desc, $text) = $fields;
+
       $branch = $this->getBranchNameFromRef($ref);
       if ($branch) {
         $result[] = array(
           'current' => ($branch === $current),
-          'name'    => $branch,
+          'name' => $branch,
+          'hash' => $hash,
+          'tree' => $tree,
+          'epoch' => (int)$epoch,
+          'desc' => $desc,
+          'text' => $text,
         );
       }
     }
diff --git a/src/workflow/ArcanistFeatureWorkflow.php b/src/workflow/ArcanistFeatureWorkflow.php
index f9695e55..99a28200 100644
--- a/src/workflow/ArcanistFeatureWorkflow.php
+++ b/src/workflow/ArcanistFeatureWorkflow.php
@@ -174,38 +174,37 @@ EOTEXT
   private function loadCommitInfo(array $branches) {
     $repository_api = $this->getRepositoryAPI();
 
-    $futures = array();
-    foreach ($branches as $branch) {
-      if ($repository_api instanceof ArcanistMercurialAPI) {
+    $branches = ipull($branches, null, 'name');
+
+    if ($repository_api instanceof ArcanistMercurialAPI) {
+      $futures = array();
+      foreach ($branches as $branch) {
         $futures[$branch['name']] = $repository_api->execFutureLocal(
           'log -l 1 --template %s -r %s',
           "{node}\1{date|hgdate}\1{p1node}\1{desc|firstline}\1{desc}",
           hgsprintf('%s', $branch['name']));
-      } else {
-        // NOTE: "-s" is an option deep in git's diff argument parser that
-        // doesn't seem to have much documentation and has no long form. It
-        // suppresses any diff output.
-        $futures[$branch['name']] = $repository_api->execFutureLocal(
-          'show -s --format=%C %s --',
-          '%H%x01%ct%x01%T%x01%s%x01%s%n%n%b',
-          $branch['name']);
+      }
+
+      $futures = id(new FutureIterator($futures))
+        ->limit(16);
+      foreach ($futures as $name => $future) {
+        list($info) = $future->resolvex();
+
+        $fields = explode("\1", trim($info), 5);
+        list($hash, $epoch, $tree, $desc, $text) = $fields;
+
+        $branches[$name] += array(
+          'hash' => $hash,
+          'desc' => $desc,
+          'tree' => $tree,
+          'epoch' => (int)$epoch,
+          'text' => $text,
+        );
       }
     }
 
-    $branches = ipull($branches, null, 'name');
-
-    $futures = id(new FutureIterator($futures))
-      ->limit(16);
-    foreach ($futures as $name => $future) {
-      list($info) = $future->resolvex();
-      list($hash, $epoch, $tree, $desc, $text) = explode("\1", trim($info), 5);
-
-      $branch = $branches[$name] + array(
-        'hash' => $hash,
-        'desc' => $desc,
-        'tree' => $tree,
-        'epoch' => (int)$epoch,
-      );
+    foreach ($branches as $name => $branch) {
+      $text = $branch['text'];
 
       try {
         $message = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
@@ -332,6 +331,11 @@ EOTEXT
       );
     }
 
+    if (!$out) {
+      // All of the revisions are closed or abandoned.
+      return;
+    }
+
     $len_name = max(array_map('strlen', ipull($out, 'name'))) + 2;
     $len_status = max(array_map('strlen', ipull($out, 'status'))) + 2;