From 9180f429eb9f403918a2c4da5300331f9adfb223 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Jun 2016 12:14:39 -0700 Subject: [PATCH] Provide a general-purpose, modular user cache for settings and other similar data Summary: Ref T4103. Currently, we issue a `SELECT * FROM user_preferences ... WHERE userPHID = ...` on every page to load the viewer's settings. There are several other questionable data accesses on every page too, most of which could benefit from improved caching strategies (see T4103#178122). This query will soon get more expensive, since it may need to load several objects (e.g., the user's settings and their "role profile" settings). Although we could put that data on the User and do both in one query, it's nicer to put it on the Preferences object ("This inherits from profile X") which means we need to do several queries. Rather than paying a greater price, we can cheat this stuff into the existing query where we load the user's session by providing a user cache table and doing some JOIN magic. This lets us issue one query and try to get cache hits on a bunch of caches cheaply (well, we'll be in trouble at the MySQL JOIN limit of 61 tables, but have some headroom). For now, just get it working: - Add the table. - Try to get user settings "for free" when we load the session. - If we miss, fill user settings into the cache on-demand. - We only use this in one place (DarkConsole) for now. I'll use it more widely in the next diff. Test Plan: - Loaded page as logged-in user. - Loaded page as logged-out user. - Examined session query to see cache joins. - Changed settings, saw database cache fill. - Toggled DarkConsole on and off. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103 Differential Revision: https://secure.phabricator.com/D16001 --- .../autopatches/20160601.user.01.cache.sql | 11 ++ src/__phutil_library_map__.php | 8 +- .../engine/PhabricatorAuthSessionEngine.php | 89 +++++++++++++- .../base/controller/PhabricatorController.php | 5 +- .../people/cache/PhabricatorUserCacheType.php | 70 +++++++++++ .../PhabricatorUserPreferencesCacheType.php | 31 +++++ .../people/storage/PhabricatorUser.php | 73 ++++++++++++ .../people/storage/PhabricatorUserCache.php | 110 ++++++++++++++++++ .../PhabricatorUserPreferencesEditor.php | 24 +++- .../storage/PhabricatorUserPreferences.php | 11 ++ 10 files changed, 422 insertions(+), 10 deletions(-) create mode 100644 resources/sql/autopatches/20160601.user.01.cache.sql create mode 100644 src/applications/people/cache/PhabricatorUserCacheType.php create mode 100644 src/applications/people/cache/PhabricatorUserPreferencesCacheType.php create mode 100644 src/applications/people/storage/PhabricatorUserCache.php diff --git a/resources/sql/autopatches/20160601.user.01.cache.sql b/resources/sql/autopatches/20160601.user.01.cache.sql new file mode 100644 index 0000000000..bb3386b02e --- /dev/null +++ b/resources/sql/autopatches/20160601.user.01.cache.sql @@ -0,0 +1,11 @@ +CREATE TABLE {$NAMESPACE}_user.user_cache ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + userPHID VARBINARY(64) NOT NULL, + cacheIndex BINARY(12) NOT NULL, + cacheKey VARCHAR(255) NOT NULL COLLATE {$COLLATE_TEXT}, + cacheData LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}, + cacheType VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}, + UNIQUE KEY `key_usercache` (userPHID, cacheIndex), + KEY `key_cachekey` (cacheIndex), + KEY `key_cachetype` (cacheType) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7cac57cbc5..259c5301bf 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3596,6 +3596,8 @@ phutil_register_library_map(array( 'PhabricatorUnsubscribedFromObjectEdgeType' => 'applications/transactions/edges/PhabricatorUnsubscribedFromObjectEdgeType.php', 'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php', 'PhabricatorUserBlurbField' => 'applications/people/customfield/PhabricatorUserBlurbField.php', + 'PhabricatorUserCache' => 'applications/people/storage/PhabricatorUserCache.php', + 'PhabricatorUserCacheType' => 'applications/people/cache/PhabricatorUserCacheType.php', 'PhabricatorUserCardView' => 'applications/people/view/PhabricatorUserCardView.php', 'PhabricatorUserConfigOptions' => 'applications/people/config/PhabricatorUserConfigOptions.php', 'PhabricatorUserConfiguredCustomField' => 'applications/people/customfield/PhabricatorUserConfiguredCustomField.php', @@ -3614,6 +3616,7 @@ phutil_register_library_map(array( 'PhabricatorUserLogView' => 'applications/people/view/PhabricatorUserLogView.php', 'PhabricatorUserPHIDResolver' => 'applications/phid/resolver/PhabricatorUserPHIDResolver.php', 'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php', + 'PhabricatorUserPreferencesCacheType' => 'applications/people/cache/PhabricatorUserPreferencesCacheType.php', 'PhabricatorUserPreferencesEditor' => 'applications/settings/editor/PhabricatorUserPreferencesEditor.php', 'PhabricatorUserPreferencesPHIDType' => 'applications/settings/phid/PhabricatorUserPreferencesPHIDType.php', 'PhabricatorUserPreferencesQuery' => 'applications/settings/query/PhabricatorUserPreferencesQuery.php', @@ -8368,6 +8371,8 @@ phutil_register_library_map(array( 'PhabricatorConduitResultInterface', ), 'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField', + 'PhabricatorUserCache' => 'PhabricatorUserDAO', + 'PhabricatorUserCacheType' => 'Phobject', 'PhabricatorUserCardView' => 'AphrontTagView', 'PhabricatorUserConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorUserConfiguredCustomField' => array( @@ -8397,7 +8402,8 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', 'PhabricatorApplicationTransactionInterface', ), - 'PhabricatorUserPreferencesEditor' => 'AlmanacEditor', + 'PhabricatorUserPreferencesCacheType' => 'PhabricatorUserCacheType', + 'PhabricatorUserPreferencesEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorUserPreferencesPHIDType' => 'PhabricatorPHIDType', 'PhabricatorUserPreferencesQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorUserPreferencesTransaction' => 'PhabricatorApplicationTransaction', diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php index 1547d6bea8..a3ee910629 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -7,6 +7,7 @@ * @task hisec High Security * @task partial Partial Sessions * @task onetime One Time Login URIs + * @task cache User Cache */ final class PhabricatorAuthSessionEngine extends Phobject { @@ -111,9 +112,8 @@ final class PhabricatorAuthSessionEngine extends Phobject { $conn_r = $session_table->establishConnection('r'); $session_key = PhabricatorHash::digest($session_token); - // NOTE: We're being clever here because this happens on every page load, - // and by joining we can save a query. This might be getting too clever - // for its own good, though... + $cache_parts = $this->getUserCacheQueryParts($conn_r); + list($cache_selects, $cache_joins, $cache_map) = $cache_parts; $info = queryfx_one( $conn_r, @@ -125,12 +125,15 @@ final class PhabricatorAuthSessionEngine extends Phobject { s.isPartial AS s_isPartial, s.signedLegalpadDocuments as s_signedLegalpadDocuments, u.* + %Q FROM %T u JOIN %T s ON u.phid = s.userPHID - AND s.type = %s AND s.sessionKey = %s', + AND s.type = %s AND s.sessionKey = %s %Q', + $cache_selects, $user_table->getTableName(), $session_table->getTableName(), $session_type, - $session_key); + $session_key, + $cache_joins); if (!$info) { return null; @@ -141,14 +144,26 @@ final class PhabricatorAuthSessionEngine extends Phobject { 'sessionKey' => $session_key, 'type' => $session_type, ); + + $cache_raw = array_fill_keys($cache_map, null); foreach ($info as $key => $value) { if (strncmp($key, 's_', 2) === 0) { unset($info[$key]); $session_dict[substr($key, 2)] = $value; + continue; + } + + if (isset($cache_map[$key])) { + unset($info[$key]); + $cache_raw[$cache_map[$key]] = $value; + continue; } } $user = $user_table->loadFromArray($info); + + $user->attachRawCacheData($cache_raw); + switch ($session_type) { case PhabricatorAuthSession::TYPE_WEB: // Explicitly prevent bots and mailing lists from establishing web @@ -732,4 +747,68 @@ final class PhabricatorAuthSessionEngine extends Phobject { return PhabricatorHash::digest(implode(':', $parts)); } + +/* -( User Cache )--------------------------------------------------------- */ + + + /** + * @task cache + */ + private function getUserCacheQueryParts(AphrontDatabaseConnection $conn) { + $cache_selects = array(); + $cache_joins = array(); + $cache_map = array(); + + $keys = array(); + + $cache_types = PhabricatorUserCacheType::getAllCacheTypes(); + foreach ($cache_types as $cache_type) { + foreach ($cache_type->getAutoloadKeys() as $autoload_key) { + $keys[] = $autoload_key; + } + } + + $cache_table = id(new PhabricatorUserCache())->getTableName(); + + $cache_idx = 1; + foreach ($keys as $key) { + $join_as = 'ucache_'.$cache_idx; + $select_as = 'ucache_'.$cache_idx.'_v'; + + $cache_selects[] = qsprintf( + $conn, + '%T.cacheData %T', + $join_as, + $select_as); + + $cache_joins[] = qsprintf( + $conn, + 'LEFT JOIN %T AS %T ON u.phid = %T.userPHID + AND %T.cacheIndex = %s', + $cache_table, + $join_as, + $join_as, + $join_as, + PhabricatorHash::digestForIndex($key)); + + $cache_map[$select_as] = $key; + + $cache_idx++; + } + + if ($cache_selects) { + $cache_selects = ', '.implode(', ', $cache_selects); + } else { + $cache_selects = ''; + } + + if ($cache_joins) { + $cache_joins = implode(' ', $cache_joins); + } else { + $cache_joins = ''; + } + + return array($cache_selects, $cache_joins, $cache_map); + } + } diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 5d8ff7ba60..83e223d195 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -106,10 +106,9 @@ abstract class PhabricatorController extends AphrontController { PhabricatorEnv::setLocaleCode($user->getTranslation()); - $preferences = $user->loadPreferences(); if (PhabricatorEnv::getEnvConfig('darkconsole.enabled')) { - $dark_console = PhabricatorUserPreferences::PREFERENCE_DARK_CONSOLE; - if ($preferences->getPreference($dark_console) || + $dark_console = PhabricatorDarkConsoleSetting::SETTINGKEY; + if ($user->getUserSetting($dark_console) || PhabricatorEnv::getEnvConfig('darkconsole.always-on')) { $console = new DarkConsoleCore(); $request->getApplicationConfiguration()->setConsole($console); diff --git a/src/applications/people/cache/PhabricatorUserCacheType.php b/src/applications/people/cache/PhabricatorUserCacheType.php new file mode 100644 index 0000000000..f2304b830d --- /dev/null +++ b/src/applications/people/cache/PhabricatorUserCacheType.php @@ -0,0 +1,70 @@ +getPhobjectClassConstant('CACHETYPE'); + } + + public static function getAllCacheTypes() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getUserCacheType') + ->execute(); + } + + public static function getCacheTypeForKey($key) { + $all = self::getAllCacheTypes(); + + foreach ($all as $type) { + if ($type->canManageKey($key)) { + return $type; + } + } + + return null; + } + + public static function requireCacheTypeForKey($key) { + $type = self::getCacheTypeForKey($key); + + if (!$type) { + throw new Exception( + pht( + 'Failed to load UserCacheType to manage key "%s". This cache type '. + 'is required.', + $key)); + } + + return $type; + } + +} diff --git a/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php new file mode 100644 index 0000000000..88eeb3bbd5 --- /dev/null +++ b/src/applications/people/cache/PhabricatorUserPreferencesCacheType.php @@ -0,0 +1,31 @@ +getViewer(); + + $preferences = id(new PhabricatorUserPreferencesQuery()) + ->setViewer($viewer) + ->withUserPHIDs(mpull($users, 'getPHID')) + ->execute(); + + return mpull($preferences, 'getPreferences', 'getUserPHID'); + } + +} diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 9a65c44470..fd2f69ccb6 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -5,6 +5,7 @@ * @task image-cache Profile Image Cache * @task factors Multi-Factor Authentication * @task handles Managing Handles + * @task cache User Cache */ final class PhabricatorUser extends PhabricatorUserDAO @@ -61,6 +62,8 @@ final class PhabricatorUser private $alternateCSRFString = self::ATTACHABLE; private $session = self::ATTACHABLE; + private $rawCacheData = array(); + private $usableCacheData = array(); private $authorities = array(); private $handlePool; @@ -487,6 +490,22 @@ final class PhabricatorUser '(isPrimary = 1)'); } + public function getUserSetting($key) { + $settings_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES; + $settings = $this->requireCacheData($settings_key); + + if (array_key_exists($key, $settings)) { + return $settings[$key]; + } + + $defaults = PhabricatorSetting::getAllEnabledSettings($this); + if (isset($defaults[$key])) { + return $defaults[$key]->getSettingDefaultValue(); + } + + return null; + } + public function loadPreferences() { if ($this->preferences) { return $this->preferences; @@ -1461,4 +1480,58 @@ final class PhabricatorUser } +/* -( User Cache )--------------------------------------------------------- */ + + + /** + * @task cache + */ + public function attachRawCacheData(array $data) { + $this->rawCacheData = $data + $this->rawCacheData; + return $this; + } + + + /** + * @task cache + */ + protected function requireCacheData($key) { + if (isset($this->usableCacheData[$key])) { + return $this->usableCacheData[$key]; + } + + $type = PhabricatorUserCacheType::requireCacheTypeForKey($key); + + if (isset($this->rawCacheData[$key])) { + $raw_value = $this->rawCacheData[$key]; + + $usable_value = $type->getValueFromStorage($raw_value); + $this->usableCacheData[$key] = $usable_value; + + return $usable_value; + } + + $usable_value = $type->getDefaultValue(); + + $user_phid = $this->getPHID(); + if ($user_phid) { + $map = $type->newValueForUsers($key, array($this)); + if (array_key_exists($user_phid, $map)) { + $usable_value = $map[$user_phid]; + $raw_value = $type->getValueForStorage($usable_value); + + $this->rawCacheData[$key] = $raw_value; + PhabricatorUserCache::writeCache( + $type, + $key, + $user_phid, + $raw_value); + } + } + + $this->usableCacheData[$key] = $usable_value; + + return $usable_value; + } + } diff --git a/src/applications/people/storage/PhabricatorUserCache.php b/src/applications/people/storage/PhabricatorUserCache.php new file mode 100644 index 0000000000..7228a9dfa5 --- /dev/null +++ b/src/applications/people/storage/PhabricatorUserCache.php @@ -0,0 +1,110 @@ + false, + self::CONFIG_COLUMN_SCHEMA => array( + 'cacheIndex' => 'bytes12', + 'cacheKey' => 'text255', + 'cacheData' => 'text', + 'cacheType' => 'text32', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_usercache' => array( + 'columns' => array('userPHID', 'cacheIndex'), + 'unique' => true, + ), + 'key_cachekey' => array( + 'columns' => array('cacheIndex'), + ), + 'key_cachetype' => array( + 'columns' => array('cacheType'), + ), + ), + ) + parent::getConfiguration(); + } + + public function save() { + $this->cacheIndex = Filesystem::digestForIndex($this->getCacheKey()); + return parent::save(); + } + + public static function writeCache( + PhabricatorUserCacheType $type, + $key, + $user_phid, + $raw_value) { + + if (PhabricatorEnv::isReadOnly()) { + return; + } + + $table = new self(); + $conn_w = $table->establishConnection('w'); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + + queryfx( + $conn_w, + 'INSERT INTO %T (userPHID, cacheIndex, cacheKey, cacheData, cacheType) + VALUES (%s, %s, %s, %s, %s) + ON DUPLICATE KEY UPDATE cacheData = VALUES(cacheData)', + $table->getTableName(), + $user_phid, + PhabricatorHash::digestForIndex($key), + $key, + $raw_value, + $type->getUserCacheType()); + + unset($unguarded); + } + + public static function clearCache($key, $user_phid) { + if (PhabricatorEnv::isReadOnly()) { + return; + } + + $table = new self(); + $conn_w = $table->establishConnection('w'); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE cacheIndex = %s AND userPHID = %s', + $table->getTableName(), + PhabricatorHash::digestForIndex($key), + $user_phid); + + unset($unguarded); + } + + + public static function clearCacheForAllUsers($key) { + if (PhabricatorEnv::isReadOnly()) { + return; + } + + $table = new self(); + $conn_w = $table->establishConnection('w'); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE cacheIndex = %s', + $table->getTableName(), + PhabricatorHash::digestForIndex($key)); + + unset($unguarded); + } + +} diff --git a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php index 6be467de8f..bbb469eb01 100644 --- a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php +++ b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php @@ -1,7 +1,11 @@ getUserPHID(); + if ($user_phid) { + PhabricatorUserCache::clearCache( + PhabricatorUserPreferencesCacheType::KEY_PREFERENCES, + $user_phid); + } else { + PhabricatorUserCache::clearCacheForAllUsers( + PhabricatorUserPreferencesCacheType::KEY_PREFERENCES); + } + + + return $xactions; + } + } diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 7805daf771..68493b90d0 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -165,6 +165,17 @@ final class PhabricatorUserPreferences return false; } + // TODO: Remove this once all edits go through the Editor. For now, some + // old edits just do direct saves so make sure we nuke the cache. + public function save() { + PhabricatorUserCache::clearCache( + PhabricatorUserPreferencesCacheType::KEY_PREFERENCES, + $this->getUserPHID()); + + return parent::save(); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ -- 2.51.2