Restructure base85 unit tests to support inlining and multiple encoding pathways

Summary:
Ref T13130. I want to take a crack at improving performance here, but two possible approaches (inlining the actual encoding; using integers if they're big enough) aren't easy to test right now.

Restructure the tests so they can support these kinds of refactoring.

The "32bit" and "64bit" modes currently do the same thing, but I expect to introduce introduce separate encoding pathways in a future change, if the profiler says it actually helps.

(I'll hold this and everything that comes after it until I make meaningful performance improvements.)

Test Plan: Ran `arc unit`, got passes on tests.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13130

Differential Revision: https://secure.phabricator.com/D19407

src/parser/ArcanistBundle.php

@@ -781,21 +781,46 @@ final class ArcanistBundle extends Phobject {
 
   private function emitBinaryDiffBody($data) {
     $eol = $this->getEOL('git');
+    return self::newBase85Data($data, $eol);
+  }
 
-    if (!function_exists('gzcompress')) {
+  public static function newBase85Data($data, $eol, $mode = null) {
-      throw new Exception(
+    // The "32bit" and "64bit" modes are used by unit tests to verify that all
-        pht(
+    // of the encoding pathways here work identically. In these modes, we skip
-          'This patch has binary data. The PHP zlib extension is required to '.
+    // compression because `gzcompress()` may not be stable and we just want
-          'apply patches with binary data to git. Install the PHP zlib '.
+    // to test that the output matches some expected result.
-          'extension to continue.'));
+
+    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);
+    } else {
+      switch ($mode) {
+        case '32bit':
+          $input = $data;
+          break;
+        case '64bit':
+          $input = $data;
+          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($input, 52);
-    $lines = str_split($deflated, 52);
     foreach ($lines as $line) {
       $len = strlen($line);
       // The first character encodes the line length.
@@ -811,7 +836,7 @@ final class ArcanistBundle extends Phobject {
     return $buf;
   }
 
-  public static function encodeBase85($data) {
+  private static function encodeBase85($data) {
     // This is implemented awkwardly in order to closely mirror git's
     // implementation in base85.c
 

src/parser/__tests__/ArcanistBundleTestCase.php

@@ -663,11 +663,7 @@ EODIFF;
     }
 
     $expect = Filesystem::readFile(dirname(__FILE__).'/base85/expect1.txt');
-    $expect = trim($expect);
+    $this->assertBase85($expect, $data, pht('Byte Sequences'));
-
-    $this->assertEqual(
-      $expect,
-      ArcanistBundle::encodeBase85($data));
 
     // 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(
+    $this->assertBase85($expect, $data, pht('Random Data'));
-      $expect,
+  }
-      ArcanistBundle::encodeBase85($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