Improve Diffusion behavior for directories with impressive numbers of files

Summary:
Fixes T4366. Two years ago, Facebook put 16,000 files in a directory. Today, the page has nearly loaded.

Paginate large directories.

Test Plan:
  - Viewed home and browse views in Git, Mercurial and Subversion.

I put an artificially small page size (5) on home:

{F1055653}

I pushed 16,000 files to a directory and paged through them. Here's the last page, which rendered in about 200ms:

{F1055655}

Our behavior is a bit better than GitHub here, which shows only the first 1,000 files, disables pagination, and can't retrieve history for the files:

{F1055656}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4366

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

src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php

@@ -22,6 +22,8 @@ final class DiffusionBrowseQueryConduitAPIMethod
       'path' => 'optional string',
       'commit' => 'optional string',
       'needValidityOnly' => 'optional bool',
+      'limit' => 'optional int',
+      'offset' => 'optional int',
     );
   }
 
@@ -35,6 +37,8 @@ final class DiffusionBrowseQueryConduitAPIMethod
     $repository = $drequest->getRepository();
     $path = $request->getValue('path');
     $commit = $request->getValue('commit');
+    $offset = (int)$request->getValue('offset');
+    $limit = (int)$request->getValue('limit');
     $result = $this->getEmptyResultSet();
 
     if ($path == '') {
@@ -99,6 +103,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
       $prefix = '';
     }
 
+    $count = 0;
     $results = array();
     foreach (explode("\0", rtrim($stdout)) as $line) {
       // NOTE: Limit to 5 components so we parse filenames with spaces in them
@@ -140,9 +145,17 @@ final class DiffusionBrowseQueryConduitAPIMethod
       $path_result->setFileType($file_type);
       $path_result->setFileSize($size);
 
+      if ($count >= $offset) {
         $results[] = $path_result;
       }
 
+      $count++;
+
+      if ($limit && $count >= ($offset + $limit)) {
+        break;
+      }
+    }
+
     // If we identified submodules, lookup the module info at this commit to
     // find their source URIs.
 
@@ -196,6 +209,8 @@ final class DiffusionBrowseQueryConduitAPIMethod
     $repository = $drequest->getRepository();
     $path = $request->getValue('path');
     $commit = $request->getValue('commit');
+    $offset = (int)$request->getValue('offset');
+    $limit = (int)$request->getValue('limit');
     $result = $this->getEmptyResultSet();
 
 
@@ -215,6 +230,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
     // but ours do.
     $trim_len = $match_len ? $match_len + 1 : 0;
 
+    $count = 0;
     foreach ($entire_manifest as $path) {
       if (strncmp($path, $match_against, $match_len)) {
         continue;
@@ -236,9 +252,18 @@ final class DiffusionBrowseQueryConduitAPIMethod
       } else {
         $type = DifferentialChangeType::FILE_DIRECTORY;
       }
+
+      if ($count >= $offset) {
         $results[reset($parts)] = $type;
       }
 
+      $count++;
+
+      if ($limit && ($count >= ($offset + $limit))) {
+        break;
+      }
+    }
+
     foreach ($results as $key => $type) {
       $path_result = new DiffusionRepositoryPath();
       $path_result->setPath($key);
@@ -266,6 +291,8 @@ final class DiffusionBrowseQueryConduitAPIMethod
     $repository = $drequest->getRepository();
     $path = $request->getValue('path');
     $commit = $request->getValue('commit');
+    $offset = (int)$request->getValue('offset');
+    $limit = (int)$request->getValue('limit');
     $result = $this->getEmptyResultSet();
 
     $subpath = $repository->getDetail('svn-subpath');
@@ -422,6 +449,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
     $path_normal = DiffusionPathIDQuery::normalizePath($path);
 
     $results = array();
+    $count = 0;
     foreach ($browse as $file) {
 
       $full_path = $file['pathName'];
@@ -431,9 +459,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
       $result_path = new DiffusionRepositoryPath();
       $result_path->setPath($file_path);
       $result_path->setFullPath($full_path);
-//      $result_path->setHash($hash);
       $result_path->setFileType($file['fileType']);
-//      $result_path->setFileSize($size);
 
       if (!empty($file['hasCommit'])) {
         $commit = idx($commits, $file['svnCommit']);
@@ -444,9 +470,17 @@ final class DiffusionBrowseQueryConduitAPIMethod
         }
       }
 
+      if ($count >= $offset) {
         $results[] = $result_path;
       }
 
+      $count++;
+
+      if ($limit && ($count >= ($offset + $limit))) {
+        break;
+      }
+    }
+
     if (empty($results)) {
       $result->setReasonForEmptyResultSet(
         DiffusionBrowseResultSet::REASON_IS_EMPTY);

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -27,20 +27,30 @@ final class DiffusionBrowseController extends DiffusionController {
       return $this->browseSearch();
     }
 
+    $pager = id(new PHUIPagerView())
+      ->readFromRequest($request);
+
     $results = DiffusionBrowseResultSet::newFromConduit(
       $this->callConduitWithDiffusionRequest(
         'diffusion.browsequery',
         array(
           'path' => $drequest->getPath(),
           'commit' => $drequest->getStableCommit(),
+          'offset' => $pager->getOffset(),
+          'limit' => $pager->getPageSize() + 1,
         )));
+
     $reason = $results->getReasonForEmptyResultSet();
     $is_file = ($reason == DiffusionBrowseResultSet::REASON_IS_FILE);
 
     if ($is_file) {
       return $this->browseFile($results);
     } else {
-      return $this->browseDirectory($results);
+      $paths = $results->getPaths();
+      $paths = $pager->sliceResults($paths);
+      $results->setPaths($paths);
+
+      return $this->browseDirectory($results, $pager);
     }
   }
 
@@ -262,7 +272,10 @@ final class DiffusionBrowseController extends DiffusionController {
       ->appendChild($content);
   }
 
-  public function browseDirectory(DiffusionBrowseResultSet $results) {
+  public function browseDirectory(
+    DiffusionBrowseResultSet $results,
+    PHUIPagerView $pager) {
+
     $request = $this->getRequest();
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
@@ -339,6 +352,8 @@ final class DiffusionBrowseController extends DiffusionController {
         'view'   => 'browse',
       ));
 
+    $pager_box = $this->renderTablePagerBox($pager);
+
     return $this->newPage()
       ->setTitle(
         array(
@@ -346,7 +361,11 @@ final class DiffusionBrowseController extends DiffusionController {
           $repository->getDisplayName(),
         ))
       ->setCrumbs($crumbs)
-      ->appendChild($content);
+      ->appendChild(
+        array(
+          $content,
+          $pager_box,
+        ));
   }
 
   private function renderSearchResults() {

src/applications/diffusion/controller/DiffusionController.php

@@ -110,7 +110,7 @@ abstract class DiffusionController extends PhabricatorController {
     $crumb_list = array();
 
     // On the home page, we don't have a DiffusionRequest.
-    if ($this->diffusionRequest) {
+    if ($this->hasDiffusionRequest()) {
       $drequest = $this->getDiffusionRequest();
       $repository = $drequest->getRepository();
     } else {

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -88,6 +88,7 @@ final class DiffusionRepositoryController extends DiffusionController {
 
 
   private function buildNormalContent(DiffusionRequest $drequest) {
+    $request = $this->getRequest();
     $repository = $drequest->getRepository();
 
     $phids = array();
@@ -123,6 +124,9 @@ final class DiffusionRepositoryController extends DiffusionController {
       $history_exception = $ex;
     }
 
+    $browse_pager = id(new PHUIPagerView())
+      ->readFromRequest($request);
+
     try {
       $browse_results = DiffusionBrowseResultSet::newFromConduit(
         $this->callConduitWithDiffusionRequest(
@@ -130,8 +134,10 @@ final class DiffusionRepositoryController extends DiffusionController {
           array(
             'path' => $drequest->getPath(),
             'commit' => $drequest->getCommit(),
+            'limit' => $browse_pager->getPageSize() + 1,
           )));
       $browse_paths = $browse_results->getPaths();
+      $browse_paths = $browse_pager->sliceResults($browse_paths);
 
       foreach ($browse_paths as $item) {
         $data = $item->getLastCommitData();
@@ -178,7 +184,8 @@ final class DiffusionRepositoryController extends DiffusionController {
       $browse_results,
       $browse_paths,
       $browse_exception,
-      $handles);
+      $handles,
+      $browse_pager);
 
     $content[] = $this->buildHistoryTable(
       $history_results,
@@ -588,7 +595,8 @@ final class DiffusionRepositoryController extends DiffusionController {
     $browse_results,
     $browse_paths,
     $browse_exception,
-    array $handles) {
+    array $handles,
+    PHUIPagerView $pager) {
 
     require_celerity_resource('diffusion-icons-css');
 
@@ -669,7 +677,19 @@ final class DiffusionRepositoryController extends DiffusionController {
 
     $browse_panel->setTable($browse_table);
 
-    return array($locate_panel, $browse_panel);
+    $pager->setURI($browse_uri, 'offset');
+
+    if ($pager->willShowPagingControls()) {
+      $pager_box = $this->renderTablePagerBox($pager);
+    } else {
+      $pager_box = null;
+    }
+
+    return array(
+      $locate_panel,
+      $browse_panel,
+      $pager_box,
+    );
   }
 
   private function renderCloneCommand(

src/applications/repository/storage/PhabricatorRepository.php

@@ -627,7 +627,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       case 'refs':
         break;
       case 'branch':
-        $req_branch = true;
+        // NOTE: This does not actually require a branch, and won't have one
+        // in Subversion. Possibly this should be more clear.
         break;
       case 'commit':
       case 'rendering-ref':