Skip to content

Commit bc3ac31

Browse files
author
epriestley
committedJul 1, 2016
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
1 parent f263742 commit bc3ac31

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed
 

‎src/applications/differential/controller/DifferentialRevisionViewController.php

+1
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ public function handleRequest(AphrontRequest $request) {
344344
$stack_graph = id(new DifferentialRevisionGraph())
345345
->setViewer($viewer)
346346
->setSeedPHID($revision->getPHID())
347+
->setLoadEntireGraph(true)
347348
->loadGraph();
348349
if (!$stack_graph->isEmpty()) {
349350
$stack_table = $stack_graph->newGraphTable();

‎src/infrastructure/graph/PhabricatorObjectGraph.php

+33-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ abstract class PhabricatorObjectGraph
55

66
private $viewer;
77
private $edges = array();
8+
private $edgeReach = array();
89
private $seedPHID;
910
private $objects;
11+
private $loadEntireGraph = false;
1012

1113
public function setViewer(PhabricatorUser $viewer) {
1214
$this->viewer = $viewer;
@@ -29,6 +31,7 @@ abstract protected function newTable(AphrontTableView $table);
2931

3032
final public function setSeedPHID($phid) {
3133
$this->seedPHID = $phid;
34+
$this->edgeReach[$phid] = array_fill_keys($this->getEdgeTypes(), true);
3235

3336
return $this->addNodes(
3437
array(
@@ -41,7 +44,30 @@ final public function isEmpty() {
4144
}
4245

4346
final public function getEdges($type) {
44-
return idx($this->edges, $type, array());
47+
$edges = idx($this->edges, $type, array());
48+
49+
// Remove any nodes which we never reached. We can get these when loading
50+
// only part of the graph: for example, they point at other subtasks of
51+
// parents or other parents of subtasks.
52+
$nodes = $this->getNodes();
53+
foreach ($edges as $src => $dsts) {
54+
foreach ($dsts as $key => $dst) {
55+
if (!isset($nodes[$dst])) {
56+
unset($edges[$src][$key]);
57+
}
58+
}
59+
}
60+
61+
return $edges;
62+
}
63+
64+
final public function setLoadEntireGraph($load_entire_graph) {
65+
$this->loadEntireGraph = $load_entire_graph;
66+
return $this;
67+
}
68+
69+
final public function getLoadEntireGraph() {
70+
return $this->loadEntireGraph;
4571
}
4672

4773
final protected function loadEdges(array $nodes) {
@@ -53,6 +79,8 @@ final protected function loadEdges(array $nodes) {
5379

5480
$query->execute();
5581

82+
$whole_graph = $this->getLoadEntireGraph();
83+
5684
$map = array();
5785
foreach ($nodes as $node) {
5886
$map[$node] = array();
@@ -64,7 +92,10 @@ final protected function loadEdges(array $nodes) {
6492

6593
$this->edges[$edge_type][$node] = $dst_phids;
6694
foreach ($dst_phids as $dst_phid) {
67-
$map[$node][] = $dst_phid;
95+
if ($whole_graph || isset($this->edgeReach[$node][$edge_type])) {
96+
$map[$node][] = $dst_phid;
97+
}
98+
$this->edgeReach[$dst_phid][$edge_type] = true;
6899
}
69100
}
70101

0 commit comments

Comments
 (0)
Failed to load comments.