From 8484adcffdf61fc9088d86765524e5e5d60d9db9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 16 Jun 2015 19:32:37 -0700 Subject: [PATCH] Cache application visibility in the request cache Summary: Ref T8575. We check if users can see applications frequently, and caching on the Query isn't especially effective. Use the new Request cache instead. Test Plan: - Saw `/feed/` drop 7% (from ~830ms to ~770ms) on profiles. Reviewers: btrahan, avivey Reviewed By: avivey Subscribers: avivey, epriestley Maniphest Tasks: T8575 Differential Revision: https://secure.phabricator.com/D13321 --- .../base/PhabricatorApplication.php | 23 ++++++++++++++----- .../policy/PhabricatorPolicyAwareQuery.php | 19 ++++----------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index 03f14a241b..1f605476bc 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -449,14 +449,25 @@ abstract class PhabricatorApplication $class, PhabricatorUser $viewer) { - if (!self::isClassInstalled($class)) { - return false; + $cache = PhabricatorCaches::getRequestCache(); + $viewer_phid = $viewer->getPHID(); + $key = 'app.'.$class.'.installed.'.$viewer_phid; + + $result = $cache->getKey($key); + if ($result === null) { + if (!self::isClassInstalled($class)) { + $result = false; + } else { + $result = PhabricatorPolicyFilter::hasCapability( + $viewer, + self::getByClass($class), + PhabricatorPolicyCapability::CAN_VIEW); + } + + $cache->setKey($key, $result); } - return PhabricatorPolicyFilter::hasCapability( - $viewer, - self::getByClass($class), - PhabricatorPolicyCapability::CAN_VIEW); + return $result; } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index e7b9b35628..30b7b8dd0e 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -35,7 +35,6 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { private $workspace = array(); private $inFlightPHIDs = array(); private $policyFilteredPHIDs = array(); - private $canUseApplication; /** * Should we continue or throw an exception when a query result is filtered @@ -679,21 +678,13 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * execute the query. */ public function canViewerUseQueryApplication() { - if ($this->canUseApplication === null) { - $class = $this->getQueryApplicationClass(); - if (!$class) { - $this->canUseApplication = true; - } else { - $result = id(new PhabricatorApplicationQuery()) - ->setViewer($this->getViewer()) - ->withClasses(array($class)) - ->execute(); - - $this->canUseApplication = (bool)$result; - } + $class = $this->getQueryApplicationClass(); + if (!$class) { + return true; } - return $this->canUseApplication; + $viewer = $this->getViewer(); + return PhabricatorApplication::isClassInstalledForViewer($class, $viewer); } }