From 83c857e6a6ca4af4f5ce50204d5e8ffbbcc5c035 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 10 Jul 2016 07:19:47 -0700
Subject: [PATCH] (stable) In Git, only use "--find-copies-harder" on small
 diffs

Summary:
Ref T10423. This flag can cause `git diff` to take an enormously long time (the problem case was a 5M line, 20K file commit).

Instead:

  - Run without the flag first.
  - If that shows that the diff is definitely small, try again with the flag.
  - If that works, return the slower, better output.
  - If the fast diff affects too many paths or generating the slow diff takes too long, return the faster, slightly worse output.

The quality of the output differs in how well Git is able to detect "M" and "C" (moves and copies of files).

For example, if you copy `src/` to `srcpro/`, the fast output may not show that you copied files. The slow output will.

I think this is rarely useful for large copies anyway: it's interesting if a 1-2 file diff is a copy, but usually obvious/uninteresting if a 500-file diff is a copy.

Test Plan:
  - Ran `bin/repository reparse --change rXnnn` on Git changes.
  - Saw fast and slow commands execute normally.
  - Tried on a large diff, saw only the fast command execute.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10423

Differential Revision: https://secure.phabricator.com/D16266
---
 ...nternalGitRawDiffQueryConduitAPIMethod.php | 71 ++++++++++++++++---
 1 file changed, 60 insertions(+), 11 deletions(-)

diff --git a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
index 1e4c906322..ac241a282f 100644
--- a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
@@ -28,6 +28,7 @@ final class DiffusionInternalGitRawDiffQueryConduitAPIMethod
   protected function getResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
+    $commit = $request->getValue('commit');
 
     if (!$repository->isGit()) {
       throw new Exception(
@@ -44,30 +45,78 @@ final class DiffusionInternalGitRawDiffQueryConduitAPIMethod
     list($parents) = $repository->execxLocalCommand(
       'log -n1 --format=%s %s',
       '%P',
-      $request->getValue('commit'));
-
+      $commit);
     $use_log = !strlen(trim($parents));
+
+    // First, get a fast raw diff without "--find-copies-harder". This flag
+    // produces better results for moves and copies, but is explosively slow
+    // for large changes to large repositories. See T10423.
+    $raw = $this->getRawDiff($repository, $commit, $use_log, false);
+
+    // If we got a normal-sized diff (no more than 100 modified files), we'll
+    // try using "--find-copies-harder" to improve the output. This improved
+    // output is mostly useful for small changes anyway.
+    $try_harder = (substr_count($raw, "\n") <= 100);
+    if ($try_harder) {
+      try {
+        $raw = $this->getRawDiff($repository, $commit, $use_log, true);
+      } catch (Exception $ex) {
+        // Just ignore any exception we hit, we'll use the fast output
+        // instead.
+      }
+    }
+
+    return $raw;
+  }
+
+  private function getRawDiff(
+    PhabricatorRepository $repository,
+    $commit,
+    $use_log,
+    $try_harder) {
+
+    $flags = array(
+      '-n1',
+      '-M',
+      '-C',
+      '-B',
+      '--raw',
+      '-t',
+      '--abbrev=40',
+    );
+
+    if ($try_harder) {
+      $flags[] = '--find-copies-harder';
+    }
+
     if ($use_log) {
       // This is the first commit so we need to use "log". We know it's not a
       // merge commit because it couldn't be merging anything, so this is safe.
 
       // NOTE: "--pretty=format: " is to disable diff output, we only want the
       // part we get from "--raw".
-      list($raw) = $repository->execxLocalCommand(
-        'log -n1 -M -C -B --find-copies-harder --raw -t '.
-          '--pretty=format: --abbrev=40 %s',
-        $request->getValue('commit'));
+      $future = $repository->getLocalCommandFuture(
+        'log %Ls --pretty=format: %s',
+        $flags,
+        $commit);
     } else {
       // Otherwise, we can use "diff", which will give us output for merges.
       // We diff against the first parent, as this is generally the expectation
       // and results in sensible behavior.
-      list($raw) = $repository->execxLocalCommand(
-        'diff -n1 -M -C -B --find-copies-harder --raw -t '.
-          '--abbrev=40 %s^1 %s',
-        $request->getValue('commit'),
-        $request->getValue('commit'));
+      $future = $repository->getLocalCommandFuture(
+        'diff %Ls %s^1 %s',
+        $flags,
+        $commit,
+        $commit);
     }
 
+    // Don't spend more than 30 seconds generating the slower output.
+    if ($try_harder) {
+      $future->setTimeout(30);
+    }
+
+    list($raw) = $future->resolvex();
+
     return $raw;
   }