From df59f4b047e5e788bc9a9ac004b137eb54c83e66 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 10 May 2014 15:32:53 -0700
Subject: [PATCH] Batch all supplementary information in Diffusion browse views

Summary: Ref T2683. Instead of sending one request for each path's history, send one request for all of it. This permits optimizations which are not currently available to us. It degrades the user experience a tiny bit in theory, but on my machine it's actually way faster already.

Test Plan: Loaded a browse page.

Reviewers: vrana, btrahan

Reviewed By: btrahan

Subscribers: epriestley, aran

Maniphest Tasks: T2683

Differential Revision: https://secure.phabricator.com/D5254
---
 .../DiffusionLastModifiedController.php       | 77 ++++++++++---------
 .../view/DiffusionBrowseTableView.php         | 18 +++--
 .../diffusion/behavior-pull-lastmodified.js   | 15 ++--
 3 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php
index 90f1c3839c..f52920d3fa 100644
--- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php
+++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php
@@ -9,47 +9,50 @@ final class DiffusionLastModifiedController extends DiffusionController {
   public function processRequest() {
     $drequest = $this->getDiffusionRequest();
     $request = $this->getRequest();
-    $commit = null;
-    $commit_data = null;
 
-    $conduit_result = $this->callConduitWithDiffusionRequest(
-      'diffusion.lastmodifiedquery',
-      array(
-        'commit' => $drequest->getCommit(),
-        'path' => $drequest->getPath()
-      ));
-    $c_dict = $conduit_result['commit'];
-    if ($c_dict) {
-      $commit = PhabricatorRepositoryCommit::newFromDictionary($c_dict);
-    }
-    $c_d_dict = $conduit_result['commitData'];
-    if ($c_d_dict) {
-      $commit_data =
-        PhabricatorRepositoryCommitData::newFromDictionary($c_d_dict);
-    }
+    $paths = $request->getStr('paths');
+    $paths = json_decode($paths, true);
 
-    $phids = array();
-    if ($commit_data) {
-      if ($commit_data->getCommitDetail('authorPHID')) {
-        $phids[$commit_data->getCommitDetail('authorPHID')] = true;
-      }
-      if ($commit_data->getCommitDetail('committerPHID')) {
-        $phids[$commit_data->getCommitDetail('committerPHID')] = true;
+    $output = array();
+    foreach ($paths as $path) {
+      $prequest = clone $drequest;
+      $prequest->setPath($path);
+
+      $conduit_result = $this->callConduitWithDiffusionRequest(
+        'diffusion.lastmodifiedquery',
+        array(
+          'commit' => $prequest->getCommit(),
+          'path' => $prequest->getPath(),
+        ));
+
+      $commit = PhabricatorRepositoryCommit::newFromDictionary(
+        $conduit_result['commit']);
+
+      $commit_data = PhabricatorRepositoryCommitData::newFromDictionary(
+        $conduit_result['commitData']);
+
+      $phids = array();
+      if ($commit_data) {
+        if ($commit_data->getCommitDetail('authorPHID')) {
+          $phids[$commit_data->getCommitDetail('authorPHID')] = true;
+        }
+        if ($commit_data->getCommitDetail('committerPHID')) {
+          $phids[$commit_data->getCommitDetail('committerPHID')] = true;
+        }
       }
+
+      $phids = array_keys($phids);
+      $handles = $this->loadViewerHandles($phids);
+
+      $view = new DiffusionBrowseTableView();
+      $view->setUser($request->getUser());
+      $output[$path] = $view->renderLastModifiedColumns(
+        $prequest,
+        $handles,
+        $commit,
+        $commit_data);
     }
 
-    $phids = array_keys($phids);
-    $handles = $this->loadViewerHandles($phids);
-
-    $view = new DiffusionBrowseTableView();
-    $view->setUser($request->getUser());
-    $output = $view->renderLastModifiedColumns(
-      $drequest,
-      $handles,
-      $commit,
-      $commit_data);
-
-    return id(new AphrontAjaxResponse())
-      ->setContent($output);
+    return id(new AphrontAjaxResponse())->setContent($output);
   }
 }
diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php
index 26ab5efcf1..7d9120fc54 100644
--- a/src/applications/diffusion/view/DiffusionBrowseTableView.php
+++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php
@@ -171,13 +171,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
           'details'   => celerity_generate_unique_node_id(),
         );
 
-        $uri = (string)$request->generateURI(
-          array(
-            'action' => 'lastmodified',
-            'path'   => $base_path.$path->getPath().$dir_slash,
-          ));
-
-        $need_pull[$uri] = $dict;
+        $need_pull[$base_path.$path->getPath().$dir_slash] = $dict;
         foreach ($dict as $k => $uniq) {
           $dict[$k] = phutil_tag('span', array('id' => $uniq), '');
         }
@@ -214,7 +208,15 @@ final class DiffusionBrowseTableView extends DiffusionView {
     }
 
     if ($need_pull) {
-      Javelin::initBehavior('diffusion-pull-lastmodified', $need_pull);
+      Javelin::initBehavior(
+        'diffusion-pull-lastmodified',
+        array(
+          'uri'   => (string)$request->generateURI(
+            array(
+              'action' => 'lastmodified',
+            )),
+          'map' => $need_pull,
+        ));
     }
 
     $branch = $this->getDiffusionRequest()->loadBranch();
diff --git a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
index fbf924df5b..e94c3f2d9d 100644
--- a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
+++ b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
@@ -3,19 +3,20 @@
  * @requires javelin-behavior
  *           javelin-dom
  *           javelin-util
- *           javelin-request
+ *           javelin-workflow
+ *           javelin-json
  */
 
 JX.behavior('diffusion-pull-lastmodified', function(config) {
 
-  for (var uri in config) {
-    new JX.Request(uri, JX.bind(config[uri], function(r) {
+  new JX.Workflow(config.uri, {paths: JX.JSON.stringify(JX.keys(config.map))})
+    .setHandler(function(r) {
       for (var k in r) {
-        if (this[k]) {
-          JX.DOM.setContent(JX.$(this[k]), JX.$H(r[k]));
+        for (var l in r[k]) {
+          JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l]));
         }
       }
-    })).send();
-  }
+    })
+    .start();
 
 });