revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00
Code Issues Releases Wiki Activity

Simplify (?) arc upload code and make it more future-oriented

Browse source 
Summary: See discussion in D4968. This makes us run `file.uploadhash` calls in parallel, to slightly improve performance.

Test Plan:
Created a binary change in a commit:

  $ head -c65535 /dev/urandom > a_random_file
  $ git add .
  $ git commit -m .
  [master 32e4c0a] .
   1 files changed, 0 insertions(+), 0 deletions(-)
   create mode 100644 a_random_file

Verified `arc diff` called `conduit.query`, then `file.uploadhash` (which we expect to fail; the file is new), then `file.upload`:

  $ arc diff HEAD^ --only --trace
  ...
  Uploading 1 files...
  >>> [15] <conduit> conduit.query() <bytes = 157>
  >>> [16] <http> http://local.aphront.com/api/conduit.query
  <<< [16] <http> 111,500 us
  <<< [15] <conduit> 112,169 us
  >>> [17] <conduit> file.uploadhash() <bytes = 254>
  >>> [18] <http> http://local.aphront.com/api/file.uploadhash
  <<< [18] <http> 85,386 us
  <<< [17] <conduit> 85,798 us
  >>> [19] <conduit> file.upload() <bytes = 97009>
  >>> [20] <http> http://local.aphront.com/api/file.upload
  <<< [20] <http> 144,098 us
  <<< [19] <conduit> 144,550 us
  Uploaded 'a_random_file' (new).
  Upload complete.
  ...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/202/

  Included changes:
    A (bin) a_random_file

Created the same diff again, verified that we skip the data transfer this time:

  $ arc diff HEAD^ --only --trace
  ...
  Uploading 1 files...
  >>> [15] <conduit> conduit.query() <bytes = 157>
  >>> [16] <http> http://local.aphront.com/api/conduit.query
  <<< [16] <http> 112,717 us
  <<< [15] <conduit> 113,374 us
  >>> [17] <conduit> file.uploadhash() <bytes = 254>
  >>> [18] <http> http://local.aphront.com/api/file.uploadhash
  <<< [18] <http> 95,545 us
  <<< [17] <conduit> 95,921 us
  Uploaded 'a_random_file' (new).
  Upload complete.
  ...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/203/

  Included changes:
    A (bin) a_random_file

Created a new commit which adds a new file and updates the exiting file:

  $ head -c65535 /dev/urandom > another_file
  $ head -c65535 /dev/urandom >> a_random_file
  $ git add .
  $ git commit -m .
  [master 28f4fbd] .
   2 files changed, 0 insertions(+), 0 deletions(-)
   create mode 100644 another_file

Verified `arc diff` transfers one file by hash (old version of the first file) and two by data (new first file, new second file):

  $ arc diff HEAD^ --only --trace
  ...
  Uploading 3 files...
  >>> [19] <conduit> conduit.query() <bytes = 157>
  >>> [20] <http> http://local.aphront.com/api/conduit.query
  <<< [20] <http> 114,825 us
  <<< [19] <conduit> 115,517 us
  >>> [21] <conduit> file.uploadhash() <bytes = 254>
  >>> [22] <http> http://local.aphront.com/api/file.uploadhash
  >>> [23] <conduit> file.uploadhash() <bytes = 254>
  >>> [24] <http> http://local.aphront.com/api/file.uploadhash
  >>> [25] <conduit> file.uploadhash() <bytes = 253>
  >>> [26] <http> http://local.aphront.com/api/file.uploadhash
  <<< [24] <http> 104,310 us
  <<< [26] <http> 105,211 us
  <<< [25] <conduit> 105,587 us
  <<< [22] <http> 112,631 us
  <<< [25] <conduit> 111,437 us
  Uploaded 'a_random_file' (old).
  <<< [25] <conduit> 111,678 us
  >>> [27] <conduit> file.upload() <bytes = 193912>
  >>> [28] <http> http://local.aphront.com/api/file.upload
  >>> [29] <conduit> file.upload() <bytes = 97379>
  >>> [30] <http> http://local.aphront.com/api/file.upload
  <<< [28] <http> 153,126 us
  <<< [29] <conduit> 161,180 us
  Uploaded 'a_random_file' (new).
  <<< [30] <http> 678,739 us
  <<< [29] <conduit> 679,121 us
  Uploaded 'another_file' (new).
  Upload complete.
  ...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/204/

  Included changes:
    M (bin) a_random_file
    A (bin) another_file

Ran the same command again, verified three uploads by hash:

  $ arc diff HEAD^ --only --trace
  ...
  Uploading 3 files...
  >>> [19] <conduit> conduit.query() <bytes = 157>
  >>> [20] <http> http://local.aphront.com/api/conduit.query
  <<< [20] <http> 117,058 us
  <<< [19] <conduit> 117,792 us
  >>> [21] <conduit> file.uploadhash() <bytes = 254>
  >>> [22] <http> http://local.aphront.com/api/file.uploadhash
  >>> [23] <conduit> file.uploadhash() <bytes = 254>
  >>> [24] <http> http://local.aphront.com/api/file.uploadhash
  >>> [25] <conduit> file.uploadhash() <bytes = 253>
  >>> [26] <http> http://local.aphront.com/api/file.uploadhash
  <<< [22] <http> 103,373 us
  <<< [24] <http> 105,418 us
  <<< [26] <http> 105,251 us
  <<< [25] <conduit> 105,604 us
  Uploaded 'a_random_file' (old).
  <<< [25] <conduit> 105,844 us
  Uploaded 'a_random_file' (new).
  <<< [25] <conduit> 106,053 us
  Uploaded 'another_file' (new).
  Upload complete.
  ...
  Created a new Differential diff:
          Diff URI: http://local.aphront.com:8080/differential/diff/205/

  Included changes:
    M (bin) a_random_file
    A (bin) another_file
  $

Reviewers: kwadwon

Reviewed By: kwadwon

CC: aran

Maniphest Tasks: T2456

Differential Revision: https://secure.phabricator.com/D4993
This commit is contained in:
epriestley 2013-02-20 12:24:32 -08:00
parent 8be64ec4c6
commit ba560159d4
1 changed files with 150 additions and 96 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

246
src/workflow/ArcanistDiffWorkflow.php
View file

@ -1092,106 +1092,11 @@ EOTEXT
}
}
$upload_futures = array();
foreach ($changes as $key => $change) {
if ($change->getFileType() != ArcanistDiffChangeType::FILE_BINARY) {
continue;
}
$path = $change->getCurrentPath();
$name = basename($path);
$old_file = $change->getOriginalFileData();
$old_dict = $this->uploadFile($old_file, $name, 'old binary');
if ($old_dict['future']) {
$upload_futures['old-'.$key] = $old_dict['future'];
}
$change->setMetadata('old:file:size', $old_dict['size']);
$change->setMetadata('old:file:mime-type', $old_dict['mime']);
$new_file = $change->getCurrentFileData();
$new_dict = $this->uploadFile($new_file, $name, 'new binary');
if ($new_dict['future']) {
$upload_futures['new-'.$key] = $new_dict['future'];
}
$change->setMetadata('new:file:size', $new_dict['size']);
$change->setMetadata('new:file:mime-type', $new_dict['mime']);
$mime_type = coalesce($new_dict['mime'], $old_dict['mime']);
if (preg_match('@^image/@', $mime_type)) {
$change->setFileType(ArcanistDiffChangeType::FILE_IMAGE);
}
}
foreach (Futures($upload_futures)->limit(4) as $key => $future) {
list($version, $key) = explode('-', $key, 2);
$change = $changes[$key];
$name = basename($change->getCurrentPath());
try {
$guid = $future->resolve();
$change->setMetadata($version.':binary-phid', $guid);
} catch (Exception $e) {
echo "Failed to upload {$version} binary '{$name}'.\n";
if (!phutil_console_confirm('Continue?', $default_no = false)) {
throw new ArcanistUsageException(
'Aborted due to file upload failure. You can use --skip-binaries '.
'to skip binary uploads.');
}
}
}
$this->uploadFilesForChanges($changes);
return $changes;
}
private function uploadFile($data, $name, $desc) {
$result = array(
'future' => null,
'mime' => null,
'size' => null
);
if ($this->getArgument('skip-binaries')) {
return $result;
}
$result['size'] = $size = strlen($data);
if (!$size) {
return $result;
}
$tmp = new TempFile();
Filesystem::writeFile($tmp, $data);
$mime_type = Filesystem::getMimeType($tmp);
$result['mime'] = $mime_type;
echo "Uploading {$desc} '{$name}' ({$mime_type}, {$size} bytes)...\n";
// Reuse storage if possible
// Try to upload file using content hash
$contentHash = sha1($data);
$call_result = $this->getConduit()->callMethodSynchronous(
'file.uploadhash',
array(
'hash' => $contentHash,
'name' => $name,
));
if ($call_result) {
$result['future'] = new ImmediateFuture($call_result);
return $result;
}
$result['future'] = $this->getConduit()->callMethod(
'file.upload',
array(
'data_base64' => base64_encode($data),
'name' => $name,
));
return $result;
}
private function getGitParentLogInfo() {
$info = array(
'parent' => null,
@ -2507,4 +2412,153 @@ EOTEXT
"interface if you want to become the owner.");
}
/* -( File Uploads )------------------------------------------------------- */
private function uploadFilesForChanges(array $changes) {
assert_instances_of($changes, 'ArcanistDiffChange');
// Collect all the files we need to upload.
$need_upload = array();
foreach ($changes as $key => $change) {
if ($change->getFileType() != ArcanistDiffChangeType::FILE_BINARY) {
continue;
}
if ($this->getArgument('skip-binaries')) {
continue;
}
$name = basename($change->getCurrentPath());
$need_upload[] = array(
'type' => 'old',
'name' => $name,
'data' => $change->getOriginalFileData(),
'change' => $change,
);
$need_upload[] = array(
'type' => 'new',
'name' => $name,
'data' => $change->getCurrentFileData(),
'change' => $change,
);
}
if (!$need_upload) {
return;
}
// Determine mime types and file sizes. Update changes from "binary" to
// "image" if the file is an image. Set image metadata.
$type_image = ArcanistDiffChangeType::FILE_IMAGE;
foreach ($need_upload as $key => $spec) {
$change = $need_upload[$key]['change'];
$type = $spec['type'];
$size = strlen($spec['data']);
$change->setMetadata("{$type}:file:size", $size);
if ($spec['data'] === null) {
// This covers the case where a file was added or removed; we don't
// need to upload it. (This is distinct from an empty file, which we
// do upload.)
unset($need_upload[$key]);
continue;
}
$mime = $this->getFileMimeType($spec['data']);
if (preg_match('@^image/@', $mime)) {
$change->setFileType($type_image);
}
$change->setMetadata("{$type}:file:mime-type", $mime);
}
echo pht("Uploading %d files...", count($need_upload))."\n";
// Now we're ready to upload the actual file data. If possible, we'll just
// transmit a hash of the file instead of the actual file data. If the data
// already exists, Phabricator can share storage. Check if we can use
// "file.uploadhash" yet (i.e., if the server is up to date enough).
// TODO: Drop this check once we bump the protocol version.
$conduit_methods = $this->getConduit()->callMethodSynchronous(
'conduit.query',
array());
$can_use_hash_upload = isset($conduit_methods['file.uploadhash']);
if ($can_use_hash_upload) {
$hash_futures = array();
foreach ($need_upload as $key => $spec) {
$hash_futures[$key] = $this->getConduit()->callMethod(
'file.uploadhash',
array(
'name' => $spec['name'],
'hash' => sha1($spec['data']),
));
}
foreach (Futures($hash_futures)->limit(8) as $key => $future) {
$type = $need_upload[$key]['type'];
$change = $need_upload[$key]['change'];
$name = $need_upload[$key]['name'];
$phid = null;
try {
$phid = $future->resolve();
} catch (Exception $e) {
// Just try uploading normally if the hash upload failed.
continue;
}
if ($phid) {
$change->setMetadata("{$type}:binary-phid", $phid);
unset($need_upload[$key]);
echo pht("Uploaded '%s' (%s).", $name, $type)."\n";
}
}
}
$upload_futures = array();
foreach ($need_upload as $key => $spec) {
$upload_futures[$key] = $this->getConduit()->callMethod(
'file.upload',
array(
'name' => $spec['name'],
'data_base64' => base64_encode($spec['data']),
));
}
foreach (Futures($upload_futures)->limit(4) as $key => $future) {
$type = $need_upload[$key]['type'];
$change = $need_upload[$key]['change'];
$name = $need_upload[$key]['name'];
try {
$phid = $future->resolve();
$change->setMetadata("{$type}:binary-phid", $phid);
echo pht("Uploaded '%s' (%s).", $name, $type)."\n";
} catch (Exception $e) {
echo "Failed to upload {$type} binary '{$name}'.\n";
if (!phutil_console_confirm('Continue?', $default_no = false)) {
throw new ArcanistUsageException(
'Aborted due to file upload failure. You can use --skip-binaries '.
'to skip binary uploads.');
}
}
}
echo pht("Upload complete.")."\n";
}
private function getFileMimeType($data) {
$tmp = new TempFile();
Filesystem::writeFile($tmp, $data);
return Filesystem::getMimeType($tmp);
}
}