From 0dfcb35377007e78a82ca4ff8323c7b39069373a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 2 Apr 2013 06:44:55 -0700 Subject: [PATCH] Provide `needAllTransactions()` for ConpherenceThreadQuery Summary: Conphrenece is pretty slow right now; one reason is that threads can not be loaded without also loading all of their transactions. I want to get rid of this requirement, so make it explicit and then make all existing queries require it. In particular, loading a page like `/conpherence/1/` means we load all the transactions four times: to check that the thread exists, to build the thread list (which also loads all transactions for all other visible threads), to build the thread itself, and load all the transactions to build the widget panels. Ref T2421. Test Plan: Viewed threads, lists, widgets; replied to threads. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2421 Differential Revision: https://secure.phabricator.com/D5509 --- .../controller/ConpherenceController.php | 1 + .../controller/ConpherenceListController.php | 1 + .../controller/ConpherenceUpdateController.php | 2 ++ .../controller/ConpherenceViewController.php | 1 + .../controller/ConpherenceWidgetController.php | 1 + .../conpherence/query/ConpherenceThreadQuery.php | 13 ++++++++++++- 6 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index b4dc1f267b..ed314f2cd4 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -46,6 +46,7 @@ abstract class ConpherenceController extends PhabricatorController { $all_conpherences = id(new ConpherenceThreadQuery()) ->setViewer($user) ->withPHIDs($all_conpherence_phids) + ->needAllTransactions(true) ->execute(); $unread_conpherences = array_select_keys( $all_conpherences, diff --git a/src/applications/conpherence/controller/ConpherenceListController.php b/src/applications/conpherence/controller/ConpherenceListController.php index 288913b89b..070d175d8a 100644 --- a/src/applications/conpherence/controller/ConpherenceListController.php +++ b/src/applications/conpherence/controller/ConpherenceListController.php @@ -31,6 +31,7 @@ final class ConpherenceListController if ($conpherence_id) { $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) + ->needAllTransactions(true) ->withIDs(array($conpherence_id)) ->executeOne(); if (!$conpherence) { diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 1e464353c6..e6078cccbc 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -32,6 +32,7 @@ final class ConpherenceUpdateController ->withIDs(array($conpherence_id)) ->needOrigPics(true) ->needHeaderPics(true) + ->needAllTransactions(true) ->executeOne(); $supported_formats = PhabricatorFile::getTransformableImageFormats(); @@ -258,6 +259,7 @@ final class ConpherenceUpdateController ->setAfterID($latest_transaction_id) ->needHeaderPics(true) ->needWidgetData(true) + ->needAllTransactions(true) ->withIDs(array($conpherence_id)) ->executeOne(); diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index 2b7a8c0e24..8c71c22c15 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -41,6 +41,7 @@ final class ConpherenceViewController extends ->setViewer($user) ->withIDs(array($conpherence_id)) ->needHeaderPics(true) + ->needAllTransactions(true) ->executeOne(); $this->setConpherence($conpherence); diff --git a/src/applications/conpherence/controller/ConpherenceWidgetController.php b/src/applications/conpherence/controller/ConpherenceWidgetController.php index e7502678ba..7c1cd51d20 100644 --- a/src/applications/conpherence/controller/ConpherenceWidgetController.php +++ b/src/applications/conpherence/controller/ConpherenceWidgetController.php @@ -50,6 +50,7 @@ final class ConpherenceWidgetController extends ->setViewer($user) ->withIDs(array($conpherence_id)) ->needWidgetData(true) + ->needAllTransactions(true) ->executeOne(); $this->setConpherence($conpherence); diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 45895d2ded..bf2904ac8e 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -11,6 +11,7 @@ final class ConpherenceThreadQuery private $needWidgetData; private $needHeaderPics; private $needOrigPics; + private $needAllTransactions; public function needOrigPics($need_orig_pics) { $this->needOrigPics = $need_orig_pics; @@ -27,6 +28,11 @@ final class ConpherenceThreadQuery return $this; } + public function needAllTransactions($need_all_transactions) { + $this->needAllTransactions = $need_all_transactions; + return $this; + } + public function withIDs(array $ids) { $this->ids = $ids; return $this; @@ -54,8 +60,13 @@ final class ConpherenceThreadQuery if ($conpherences) { $conpherences = mpull($conpherences, null, 'getPHID'); $this->loadParticipants($conpherences); - $this->loadTransactionsAndHandles($conpherences); + + if ($this->needAllTransactions) { + $this->loadTransactionsAndHandles($conpherences); + } + $this->loadFilePHIDs($conpherences); + if ($this->needWidgetData) { $this->loadWidgetData($conpherences); }