From c4de87a07ab07e3ea27153e84e94905038155542 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 5 Jun 2016 07:47:05 -0700 Subject: [PATCH] Improve some settings-related performance Summary: Ref T4103. Two small improvements: - Don't work as hard to validate translations. We just need to know if a translation exists, we don't need to count how many strings it has and build the entire menu. - Allow `getUserSetting()` to work on any setting without doing all the application/visibility checks. It's OK for code to look at, say, your "Conpherence Notifications" setting even if that application is not installed for you. Test Plan: Used XHProf and saw 404 page drop from ~60ms to ~40ms locally. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103 Differential Revision: https://secure.phabricator.com/D16046 --- .../application/PhabricatorHomeApplication.php | 16 +++++++++++++++- .../people/storage/PhabricatorUser.php | 5 ++++- .../setting/PhabricatorTranslationSetting.php | 5 +++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/applications/home/application/PhabricatorHomeApplication.php b/src/applications/home/application/PhabricatorHomeApplication.php index 0d2a8fb346..9b31ee537f 100644 --- a/src/applications/home/application/PhabricatorHomeApplication.php +++ b/src/applications/home/application/PhabricatorHomeApplication.php @@ -2,6 +2,7 @@ final class PhabricatorHomeApplication extends PhabricatorApplication { + private $quickItems; const DASHBOARD_DEFAULT = 'dashboard:default'; public function getBaseURI() { @@ -42,6 +43,11 @@ final class PhabricatorHomeApplication extends PhabricatorApplication { PhabricatorUser $user, PhabricatorController $controller = null) { + $quick_items = $this->getQuickActionItems($user); + if (!$quick_items) { + return array(); + } + $items = array(); $create_id = celerity_generate_unique_node_id(); @@ -73,7 +79,7 @@ final class PhabricatorHomeApplication extends PhabricatorApplication { PhabricatorUser $viewer, PhabricatorController $controller = null) { - $items = PhabricatorQuickActions::loadMenuItemsForUser($viewer); + $items = $this->getQuickActionItems($viewer); $view = null; if ($items) { @@ -94,4 +100,12 @@ final class PhabricatorHomeApplication extends PhabricatorApplication { return $view; } + private function getQuickActionItems(PhabricatorUser $viewer) { + if ($this->quickItems === null) { + $items = PhabricatorQuickActions::loadMenuItemsForUser($viewer); + $this->quickItems = $items; + } + return $this->quickItems; + } + } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index e78cd25d70..5e8cb77a72 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -486,7 +486,10 @@ final class PhabricatorUser $settings = array(); } - $defaults = PhabricatorSetting::getAllEnabledSettings($this); + // NOTE: To slightly improve performance, we're using all settings here, + // not just settings that are enabled for the current viewer. It's fine to + // get the value of a setting that we wouldn't let the user edit in the UI. + $defaults = PhabricatorSetting::getAllSettings(); if (array_key_exists($key, $settings)) { $value = $settings[$key]; diff --git a/src/applications/settings/setting/PhabricatorTranslationSetting.php b/src/applications/settings/setting/PhabricatorTranslationSetting.php index 3f4c5a43db..a30ded554d 100644 --- a/src/applications/settings/setting/PhabricatorTranslationSetting.php +++ b/src/applications/settings/setting/PhabricatorTranslationSetting.php @@ -26,6 +26,11 @@ final class PhabricatorTranslationSetting 'Choose which language you would like the Phabricator UI to use.'); } + public function assertValidValue($value) { + $locales = PhutilLocale::loadAllLocales(); + return isset($locales[$value]); + } + protected function getSelectOptionGroups() { $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); $locales = PhutilLocale::loadAllLocales();