From bc3ac3158448dfe966d05787fa9272f08ddc7faa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 1 Jul 2016 11:37:01 -0700
Subject: [PATCH] Don't load the entire graph for tasks

Summary:
Ref T4788. As it turns out, our tasks are very tightly connected.

Instead of loading every parent/child task, then every parent/child of those tasks, etc., etc., only load tasks in the "same direction" that we're already heading.

For example, we load children of children, but not parents of children. And we load parents of parents, but not children of parents.

Basically we only go "up" and "down" now, but not "out" as much. This should reduce the gigantic multiple-thousand-node graphs currently shown in the UI.

I still discover the whole graph for revisiosn, because I think it's probably more useful and always much smaller. That might need adjustment too, though.

Test Plan: Seems fine locally??

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4788

Differential Revision: https://secure.phabricator.com/D16218
---
 .../DifferentialRevisionViewController.php    |  1 +
 .../graph/PhabricatorObjectGraph.php          | 35 +++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index adf6a42bfd..e7fcdba377 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -344,6 +344,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $stack_graph = id(new DifferentialRevisionGraph())
       ->setViewer($viewer)
       ->setSeedPHID($revision->getPHID())
+      ->setLoadEntireGraph(true)
       ->loadGraph();
     if (!$stack_graph->isEmpty()) {
       $stack_table = $stack_graph->newGraphTable();
diff --git a/src/infrastructure/graph/PhabricatorObjectGraph.php b/src/infrastructure/graph/PhabricatorObjectGraph.php
index 2580512eb6..0be6d6db76 100644
--- a/src/infrastructure/graph/PhabricatorObjectGraph.php
+++ b/src/infrastructure/graph/PhabricatorObjectGraph.php
@@ -5,8 +5,10 @@ abstract class PhabricatorObjectGraph
 
   private $viewer;
   private $edges = array();
+  private $edgeReach = array();
   private $seedPHID;
   private $objects;
+  private $loadEntireGraph = false;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -29,6 +31,7 @@ abstract class PhabricatorObjectGraph
 
   final public function setSeedPHID($phid) {
     $this->seedPHID = $phid;
+    $this->edgeReach[$phid] = array_fill_keys($this->getEdgeTypes(), true);
 
     return $this->addNodes(
       array(
@@ -41,7 +44,30 @@ abstract class PhabricatorObjectGraph
   }
 
   final public function getEdges($type) {
-    return idx($this->edges, $type, array());
+    $edges = idx($this->edges, $type, array());
+
+    // Remove any nodes which we never reached. We can get these when loading
+    // only part of the graph: for example, they point at other subtasks of
+    // parents or other parents of subtasks.
+    $nodes = $this->getNodes();
+    foreach ($edges as $src => $dsts) {
+      foreach ($dsts as $key => $dst) {
+        if (!isset($nodes[$dst])) {
+          unset($edges[$src][$key]);
+        }
+      }
+    }
+
+    return $edges;
+  }
+
+  final public function setLoadEntireGraph($load_entire_graph) {
+    $this->loadEntireGraph = $load_entire_graph;
+    return $this;
+  }
+
+  final public function getLoadEntireGraph() {
+    return $this->loadEntireGraph;
   }
 
   final protected function loadEdges(array $nodes) {
@@ -53,6 +79,8 @@ abstract class PhabricatorObjectGraph
 
     $query->execute();
 
+    $whole_graph = $this->getLoadEntireGraph();
+
     $map = array();
     foreach ($nodes as $node) {
       $map[$node] = array();
@@ -64,7 +92,10 @@ abstract class PhabricatorObjectGraph
 
         $this->edges[$edge_type][$node] = $dst_phids;
         foreach ($dst_phids as $dst_phid) {
-          $map[$node][] = $dst_phid;
+          if ($whole_graph || isset($this->edgeReach[$node][$edge_type])) {
+            $map[$node][] = $dst_phid;
+          }
+          $this->edgeReach[$dst_phid][$edge_type] = true;
         }
       }