(stable) Promote 2018 Week 17

src/parser/ArcanistBundle.php

@@ -781,37 +781,46 @@ final class ArcanistBundle extends Phobject {
 
   private function emitBinaryDiffBody($data) {
     $eol = $this->getEOL('git');
+    return self::newBase85Data($data, $eol);
+  }
 
-    if (!function_exists('gzcompress')) {
-      throw new Exception(
-        pht(
-          'This patch has binary data. The PHP zlib extension is required to '.
-          'apply patches with binary data to git. Install the PHP zlib '.
-          'extension to continue.'));
+  public static function newBase85Data($data, $eol, $mode = null) {
+    // The "32bit" and "64bit" modes are used by unit tests to verify that all
+    // of the encoding pathways here work identically. In these modes, we skip
+    // compression because `gzcompress()` may not be stable and we just want
+    // to test that the output matches some expected result.
+
+    if ($mode === null) {
+      if (!function_exists('gzcompress')) {
+        throw new Exception(
+          pht(
+            'This patch has binary data. The PHP zlib extension is required '.
+            'to apply patches with binary data to git. Install the PHP zlib '.
+            'extension to continue.'));
+      }
+
+      $input = gzcompress($data);
+      $is_64bit = (PHP_INT_SIZE >= 8);
+    } else {
+      switch ($mode) {
+        case '32bit':
+          $input = $data;
+          $is_64bit = false;
+          break;
+        case '64bit':
+          $input = $data;
+          $is_64bit = true;
+          break;
+        default:
+          throw new Exception(
+            pht(
+              'Unsupported base85 encoding mode "%s".',
+              $mode));
+      }
     }
 
     // See emit_binary_diff_body() in diff.c for git's implementation.
 
-    $buf = '';
-
-    $deflated = gzcompress($data);
-    $lines = str_split($deflated, 52);
-    foreach ($lines as $line) {
-      $len = strlen($line);
-      // The first character encodes the line length.
-      if ($len <= 26) {
-        $buf .= chr($len + ord('A') - 1);
-      } else {
-        $buf .= chr($len - 26 + ord('a') - 1);
-      }
-      $buf .= self::encodeBase85($line);
-      $buf .= $eol;
-    }
-
-    return $buf;
-  }
-
-  public static function encodeBase85($data) {
     // This is implemented awkwardly in order to closely mirror git's
     // implementation in base85.c
 
@@ -839,6 +848,9 @@ final class ArcanistBundle extends Phobject {
     // (Since PHP overflows integer operations into floats, we don't need much
     // additional casting.)
 
+    // On 64 bit systems, we skip all this fanfare and just use integers. This
+    // is significantly faster.
+
     static $map = array(
       '0',
       '1',
@@ -927,27 +939,65 @@ final class ArcanistBundle extends Phobject {
       '~',
     );
 
+    $len_map = array();
+    for ($ii = 0; $ii <= 52; $ii++) {
+      if ($ii <= 26) {
+        $len_map[$ii] = chr($ii + ord('A') - 1);
+      } else {
+        $len_map[$ii] = chr($ii - 26 + ord('a') - 1);
+      }
+    }
+
     $buf = '';
 
-    $pos = 0;
-    $bytes = strlen($data);
-    while ($bytes) {
-      $accum = 0;
-      for ($count = 24; $count >= 0; $count -= 8) {
-        $val = ord($data[$pos++]);
-        $val = $val * (1 << $count);
-        $accum = $accum + $val;
-        if (--$bytes == 0) {
-          break;
+    $lines = str_split($input, 52);
+    $final = (count($lines) - 1);
+
+    foreach ($lines as $idx => $line) {
+      if ($idx === $final) {
+        $len = strlen($line);
+      } else {
+        $len = 52;
+      }
+
+      // The first character encodes the line length.
+      $buf .= $len_map[$len];
+
+      $pos = 0;
+      while ($len) {
+        $accum = 0;
+        for ($count = 24; $count >= 0; $count -= 8) {
+          $val = ord($line[$pos++]);
+          $val = $val * (1 << $count);
+          $accum = $accum + $val;
+          if (--$len == 0) {
+            break;
+          }
         }
+
+        $slice = '';
+
+        // If we're in 64bit mode, we can just use integers. Otherwise, we
+        // need to use floating point math to avoid overflows.
+
+        if ($is_64bit) {
+          for ($count = 4; $count >= 0; $count--) {
+            $val = $accum % 85;
+            $accum = $accum / 85;
+            $slice .= $map[$val];
+          }
+        } else {
+          for ($count = 4; $count >= 0; $count--) {
+            $val = (int)fmod($accum, 85.0);
+            $accum = floor($accum / 85.0);
+            $slice .= $map[$val];
+          }
+        }
+
+        $buf .= strrev($slice);
       }
-      $slice = '';
-      for ($count = 4; $count >= 0; $count--) {
-        $val = (int)fmod($accum, 85.0);
-        $accum = floor($accum / 85.0);
-        $slice .= $map[$val];
-      }
-      $buf .= strrev($slice);
+
+      $buf .= $eol;
     }
 
     return $buf;

src/parser/__tests__/ArcanistBundleTestCase.php

@@ -663,11 +663,7 @@ EODIFF;
     }
 
     $expect = Filesystem::readFile(dirname(__FILE__).'/base85/expect1.txt');
-    $expect = trim($expect);
-
-    $this->assertEqual(
-      $expect,
-      ArcanistBundle::encodeBase85($data));
+    $this->assertBase85($expect, $data, pht('Byte Sequences'));
 
     // This is just a large block of random binary data, it has no special
     // significance.
@@ -948,11 +944,27 @@ EODIFF;
       "\xe8\x1d\xa4\x18\xf3\x73\x82\xb4\x50\x59\xc2\x34\x36\x05\xeb";
 
     $expect = Filesystem::readFile(dirname(__FILE__).'/base85/expect2.txt');
-    $expect = trim($expect);
 
-    $this->assertEqual(
-      $expect,
-      ArcanistBundle::encodeBase85($data));
+    $this->assertBase85($expect, $data, pht('Random Data'));
+  }
+
+  private function assertBase85($expect, $data, $label) {
+    $modes = array(
+      '32bit',
+    );
+
+    // If this is a 64-bit machine, we can also test 64-bit mode.
+    $has_64bit = (PHP_INT_SIZE >= 8);
+    if ($has_64bit) {
+      $modes[] = '64bit';
+    }
+
+    foreach ($modes as $mode) {
+      $this->assertEqual(
+        $expect,
+        ArcanistBundle::newBase85Data($data, "\n", $mode),
+        pht('base85/%s: %s', $mode, $label));
+    }
   }
 
 }

src/parser/__tests__/base85/expect1.txt

@@ -1 +1,10 @@
-009C61O)~M2nh-c3=Iws5D^j+6crX17#SKH9337XAR!_nBqb&%C@Cr{EG;fCFflSSG&MFiI5|2yJUu=?KtV!7L`6nNNJ&adOifNtP*GA-R8>}2SXo+ITwPvYU}0ioWMyV&XlZI|Y;A6DaB*^Tbai%jczJqze0_d@fPsR8goTEOh>41ejE#<ukdcy;l$Dm3n3<ZJoSmMZprN9pq@|{(sHv)}tgWuEu(7hUw6(UkxVgH!yuH4^z`?@9#Kp$P$jQpf%+1cv(9zP<)YaD4*xB0K+}+;a;Njxq<mKk)=;`X~?CtLF@bU8V^!4`l`1$(#{Qds_|Ni~_`}+C#_xAPl^YZcV@9ypF>+0#~=jP?)<Kp4q-`?Ha+uGUK*Vfh4)6&t<&(6)v%gV{f$Hv9P!@|M9zrMY^ySll!x3;ykv$C<Uudc1EtE#D}r>3Q(qoSdppPrqZo0^%JmzI^3lai5;kB*Iui;9VehlYiOgMxv8e|~*@dwO|zcXoAjb8>NTZ*FaDYiem|XJ%z&V`5=oUtV2YTUuFIS5{S2Q&Le-PfkrtOG-&dM@B_NLqb77KR!J?J32WyH#RjiGcqwSFD@-CD=H}{CnhB%BO)OnA08bX8yXoH7Zw#16A}>+4-O3s3knGc2L=TM0|Eg6
+z009C61O)~M2nh-c3=Iws5D^j+6crX17#SKH9337XAR!_nBqb&%C@Cr{EG;fCFflSS
+zG&MFiI5|2yJUu=?KtV!7L`6nNNJ&adOifNtP*GA-R8>}2SXo+ITwPvYU}0ioWMyV&
+zXlZI|Y;A6DaB*^Tbai%jczJqze0_d@fPsR8goTEOh>41ejE#<ukdcy;l$Dm3n3<ZJ
+zoSmMZprN9pq@|{(sHv)}tgWuEu(7hUw6(UkxVgH!yuH4^z`?@9#Kp$P$jQpf%+1cv
+z(9zP<)YaD4*xB0K+}+;a;Njxq<mKk)=;`X~?CtLF@bU8V^!4`l`1$(#{Qds_|Ni~_
+z`}+C#_xAPl^YZcV@9ypF>+0#~=jP?)<Kp4q-`?Ha+uGUK*Vfh4)6&t<&(6)v%gV{f
+z$Hv9P!@|M9zrMY^ySll!x3;ykv$C<Uudc1EtE#D}r>3Q(qoSdppPrqZo0^%JmzI^3
+zlai5;kB*Iui;9VehlYiOgMxv8e|~*@dwO|zcXoAjb8>NTZ*FaDYiem|XJ%z&V`5=o
+zUtV2YTUuFIS5{S2Q&Le-PfkrtOG-&dM@B_NLqb77KR!J?J32WyH#RjiGcqwSFD@-C
+rD=H}{CnhB%BO)OnA08bX8yXoH7Zw#16A}>+4-O3s3knGc2L=TM0|Eg6