Page MenuHomePhabricator
Files F14763146

D16266.diff
No OneTemporary
Actions

D16266.diff
View Options

diff --git a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
--- a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php
@@ -28,6 +28,7 @@
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 @@
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;
}

File Metadata

Mime Type
text/plain
Expires
Fri, Jan 24, 9:38 AM (18 h, 9 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7039868
Default Alt Text
D16266.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility