From e13369d20815377cd60bc6c6bf6551b8a135f39b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 10 May 2014 17:26:03 -0700
Subject: [PATCH] Use RepositoryGraphCache to service
 diffusion.lastmodifiedquery

Summary:
Ref T2683. At least locally, browse views are now nearly instantaneous, even in Mercurial. We also fall back to what we were doing before if we miss or take too long, so this shouldn't make things very much worse even in extreme cases.

For a local `hg` repo, the time we spend pulling browse stuff has dropped from ~3,000ms to ~20ms. This is probably atypical, but not completely crazy or rigged or anything.

Test Plan: Viewed Git, Subversion and Mercurial repositories and observed dramatically better performance in Git and Mercurial as they took advantage of the cache.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, jhurwitz

Maniphest Tasks: T2683

Differential Revision: https://secure.phabricator.com/D9047
---
 ...API_diffusion_lastmodifiedquery_Method.php | 90 ++++++++++++++++---
 .../view/DiffusionBrowseTableView.php         |  1 +
 .../PhabricatorRepositoryGraphCache.php       |  2 +-
 3 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php
index d0101c4b15..d93ad78b9b 100644
--- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php
+++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php
@@ -21,23 +21,28 @@ final class ConduitAPI_diffusion_lastmodifiedquery_Method
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
-    $result = array();
-    foreach ($request->getValue('paths') as $path => $commit) {
+    $paths = $request->getValue('paths');
+    $results = $this->loadCommitsFromCache($paths);
+
+    foreach ($paths as $path => $commit) {
+      if (array_key_exists($path, $results)) {
+        continue;
+      }
       list($hash) = $repository->execxLocalCommand(
         'log -n1 --format=%%H %s -- %s',
         $commit,
         $path);
-      $result[$path] = trim($hash);
+      $results[$path] = trim($hash);
     }
 
-    return $result;
+    return $results;
   }
 
   protected function getSVNResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
-    $result = array();
+    $results = array();
     foreach ($request->getValue('paths') as $path => $commit) {
       $history_result = DiffusionQuery::callConduitWithDiffusionRequest(
         $request->getUser(),
@@ -55,30 +60,93 @@ final class ConduitAPI_diffusion_lastmodifiedquery_Method
       $history_array = DiffusionPathChange::newFromConduit(
         $history_result['pathChanges']);
       if ($history_array) {
-        $result[$path] = head($history_array)
+        $results[$path] = head($history_array)
           ->getCommit()
           ->getCommitIdentifier();
       }
     }
 
-    return $result;
+    return $results;
   }
 
   protected function getMercurialResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
-    $result = array();
-    foreach ($request->getValue('paths') as $path => $commit) {
+    $paths = $request->getValue('paths');
+    $results = $this->loadCommitsFromCache($paths);
+
+    foreach ($paths as $path => $commit) {
+      if (array_key_exists($path, $results)) {
+        continue;
+      }
+
       list($hash) = $repository->execxLocalCommand(
         'log --template %s --limit 1 --removed --rev %s -- %s',
         '{node}',
         hgsprintf('reverse(ancestors(%s))',  $commit),
         nonempty(ltrim($path, '/'), '.'));
-      $result[$path] = trim($hash);
+      $results[$path] = trim($hash);
     }
 
-    return $result;
+    return $results;
+  }
+
+  private function loadCommitsFromCache(array $map) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    $path_map = id(new DiffusionPathIDQuery(array_keys($map)))
+      ->loadPathIDs();
+
+    $commit_query = id(new DiffusionCommitQuery())
+      ->setViewer($drequest->getUser())
+      ->withRepository($repository)
+      ->withIdentifiers(array_values($map));
+    $commit_query->execute();
+
+    $commit_map = $commit_query->getIdentifierMap();
+    $commit_map = mpull($commit_map, 'getID');
+
+    $graph_cache = new PhabricatorRepositoryGraphCache();
+
+    $results = array();
+    foreach ($map as $path => $commit) {
+      $path_id = idx($path_map, $path);
+      if (!$path_id) {
+        continue;
+      }
+      $commit_id = idx($commit_map, $commit);
+      if (!$commit_id) {
+        continue;
+      }
+
+      $cache_result = $graph_cache->loadLastModifiedCommitID(
+        $commit_id,
+        $path_id);
+
+      if ($cache_result !== false) {
+        $results[$path] = $cache_result;
+      }
+    }
+
+    if ($results) {
+      $commits = id(new DiffusionCommitQuery())
+        ->setViewer($drequest->getUser())
+        ->withRepository($repository)
+        ->withIDs($results)
+        ->execute();
+      foreach ($results as $path => $id) {
+        $commit = idx($commits, $id);
+        if ($commit) {
+          $results[$path] = $commit->getCommitIdentifier();
+        } else {
+          unset($results[$path]);
+        }
+      }
+    }
+
+    return $results;
   }
 
 }
diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php
index 4ef79a1fff..9e82bbf964 100644
--- a/src/applications/diffusion/view/DiffusionBrowseTableView.php
+++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php
@@ -113,6 +113,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
           'uri'   => (string)$request->generateURI(
             array(
               'action' => 'lastmodified',
+              'stable' => true,
             )),
           'map' => $need_pull,
         ));
diff --git a/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php b/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
index 303b5b83f2..ae2f652bef 100644
--- a/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
+++ b/src/applications/repository/graphcache/PhabricatorRepositoryGraphCache.php
@@ -200,7 +200,7 @@ final class PhabricatorRepositoryGraphCache {
     if ($prefix === null) {
       $self = get_class($this);
       $size = $this->getBucketSize();
-      $prefix = "{$self}:{$size}:1:";
+      $prefix = "{$self}:{$size}:2:";
     }
 
     return $prefix.$bucket_key;