From 99b4941c9ad307ccb75dcabee3b454573d0146f8 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 8 May 2015 18:14:04 -0700 Subject: [PATCH] Conpherence - use some handle pools for Durable column perf Summary: Ref T7708. This changes things to $viewer->loadHandles where applicable in the durable column render stack. I saw some big wins on my test data like 34 queries => 24 queries on a newly created room as my default thread. For my test data, the next big perf win would be to change how remarkup rendering works and try to multiload all objects of a certain type in one shot. e.g. `PhabricatorEmbedFileRemarkupRule` implements `loadObjects` as do all classes which inherit from `PhabricatorObjectRemarkupRule`. This is because `PhabricatorObjectRemarkupRule` implements its `didMarkupText` method using `loadObjects`, and `didMarkupText` gets called per transaction over in `PhabricatorMarkupEngine->process()`. Instead, the `loadObjects` in `didMarkupText` should be hitting some cache, and we should do a bulk load for all `PhabricatorEmbedFileRemarkupRule` that had matches earlier in the rendering stack. ...I think. Test Plan: carefully looked at "Services" tab in dark console and noted fewer queries with changes post changes versus pre changes Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7708 Differential Revision: https://secure.phabricator.com/D12780 --- .../conpherence/query/ConpherenceThreadQuery.php | 7 +++---- .../query/PhabricatorApplicationTransactionQuery.php | 6 ++---- .../markup/rule/PhabricatorObjectRemarkupRule.php | 7 +++---- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 03c341f090..b17044ead0 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -285,10 +285,9 @@ final class ConpherenceThreadQuery $conpherence->$method(); } $flat_phids = array_mergev($handle_phids); - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($flat_phids) - ->execute(); + $viewer = $this->getViewer(); + $handles = $viewer->loadHandles($flat_phids); + $handles = iterator_to_array($handles); foreach ($handle_phids as $conpherence_phid => $phids) { $conpherence = $conpherences[$conpherence_phid]; $conpherence->attachHandles( diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php index ac6f713eeb..4c1e3ccd17 100644 --- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php +++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php @@ -126,10 +126,8 @@ abstract class PhabricatorApplicationTransactionQuery $handles = array(); $merged = array_mergev($phids); if ($merged) { - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs($merged) - ->execute(); + $handles = $this->getViewer()->loadHandles($merged); + $handles = iterator_to_array($handles); } foreach ($xactions as $xaction) { $xaction->setHandles( diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php index 0b4471d811..28377c19d5 100644 --- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php @@ -28,10 +28,9 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { protected function loadHandles(array $objects) { $phids = mpull($objects, 'getPHID'); - $handles = id(new PhabricatorHandleQuery($phids)) - ->withPHIDs($phids) - ->setViewer($this->getEngine()->getConfig('viewer')) - ->execute(); + $viewer = $this->getEngine()->getConfig('viewer'); + $handles = $viewer->loadHandles($phids); + $handles = iterator_to_array($handles); $result = array(); foreach ($objects as $id => $object) {