From f896dc5392a0e5d2acccaa712101bd0ff5892f21 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 22 May 2014 10:47:00 -0700 Subject: [PATCH] Put a cache in front of Celerity transforms, and update packages Summary: Fixes T5094. In some cases we do slightly expensive transformations to resources (inlining images, replacing URIs, building packages). We can throw cache in front of them easily since URIs are already permanently associated with a single resource. Also browse around and move some CSS/JS into packages. Test Plan: Added logging to verify the caches are working, saw moderately improved performance. Browsed around looking at resources tab in developer console, saw fewer total requests. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T5094 Differential Revision: https://secure.phabricator.com/D9175 --- resources/celerity/map.php | 154 ++++++++++-------- resources/celerity/packages.php | 24 ++- src/applications/cache/PhabricatorCaches.php | 49 +++++- .../celerity/CelerityResourceController.php | 98 ++++++++--- 4 files changed, 224 insertions(+), 101 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 078cd11868..be914986d5 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,16 +7,15 @@ return array( 'names' => array( - 'core.pkg.css' => '3445a3a7', - 'core.pkg.js' => 'ab0d6d3d', + 'core.pkg.css' => '48a77b2a', + 'core.pkg.js' => 'e01fd8e2', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => 'fbf57382', - 'differential.pkg.js' => '68d225fb', + 'differential.pkg.js' => 'f4c86691', 'diffusion.pkg.css' => '3783278d', 'diffusion.pkg.js' => '077e3ad0', - 'javelin.pkg.js' => 'b4831ebf', 'maniphest.pkg.css' => 'fdc718f2', - 'maniphest.pkg.js' => '2fe8af22', + 'maniphest.pkg.js' => 'd1347a35', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/context-bar.css' => '1c3b0529', 'rsrc/css/aphront/dark-console.css' => '6378ef3d', @@ -130,7 +129,7 @@ return array( 'rsrc/css/phui/phui-document.css' => '3b078dc0', 'rsrc/css/phui/phui-feed-story.css' => 'e2c9bc83', 'rsrc/css/phui/phui-fontkit.css' => 'de84aa4a', - 'rsrc/css/phui/phui-form-view.css' => '867463b4', + 'rsrc/css/phui/phui-form-view.css' => 'ed856191', 'rsrc/css/phui/phui-form.css' => 'b78ec020', 'rsrc/css/phui/phui-header-view.css' => '689dbc38', 'rsrc/css/phui/phui-icon.css' => 'cdcf2aca', @@ -762,7 +761,7 @@ return array( 'phui-font-icon-base-css' => '3b2f9260', 'phui-fontkit-css' => 'de84aa4a', 'phui-form-css' => 'b78ec020', - 'phui-form-view-css' => '867463b4', + 'phui-form-view-css' => 'ed856191', 'phui-header-view-css' => '689dbc38', 'phui-icon-view-css' => 'cdcf2aca', 'phui-info-panel-css' => '27ea50a1', @@ -2144,49 +2143,85 @@ return array( 39 => 'phui-property-list-view-css', 40 => 'phui-tag-view-css', 41 => 'phui-list-view-css', + 42 => 'font-fontawesome', + 43 => 'phui-font-icon-base-css', + 44 => 'sprite-main-header-css', + 45 => 'phui-box-css', + 46 => 'phui-object-box-css', + 47 => 'phui-timeline-view-css', + 48 => 'sprite-tokens-css', + 49 => 'tokens-css', + 50 => 'phui-status-list-view-css', ), 'core.pkg.js' => array( - 0 => 'javelin-behavior-aphront-basic-tokenizer', - 1 => 'javelin-behavior-workflow', - 2 => 'javelin-behavior-aphront-form-disable-on-submit', - 3 => 'phabricator-keyboard-shortcut-manager', - 4 => 'phabricator-keyboard-shortcut', - 5 => 'javelin-behavior-phabricator-keyboard-shortcuts', - 6 => 'javelin-behavior-refresh-csrf', - 7 => 'javelin-behavior-phabricator-watch-anchor', - 8 => 'javelin-behavior-phabricator-autofocus', - 9 => 'phuix-dropdown-menu', - 10 => 'phuix-action-list-view', - 11 => 'phuix-action-view', - 12 => 'phabricator-phtize', - 13 => 'javelin-behavior-phabricator-oncopy', - 14 => 'phabricator-tooltip', - 15 => 'javelin-behavior-phabricator-tooltips', - 16 => 'phabricator-prefab', - 17 => 'javelin-behavior-device', - 18 => 'javelin-behavior-toggle-class', - 19 => 'javelin-behavior-lightbox-attachments', - 20 => 'phabricator-busy', - 21 => 'javelin-aphlict', - 22 => 'phabricator-notification', - 23 => 'javelin-behavior-aphlict-listen', - 24 => 'javelin-behavior-phabricator-search-typeahead', - 25 => 'javelin-behavior-konami', - 26 => 'javelin-behavior-aphlict-dropdown', - 27 => 'javelin-behavior-history-install', - 28 => 'javelin-behavior-phabricator-gesture', - 29 => 'javelin-behavior-phabricator-active-nav', - 30 => 'javelin-behavior-phabricator-nav', - 31 => 'javelin-behavior-phabricator-remarkup-assist', - 32 => 'phabricator-textareautils', - 33 => 'phabricator-file-upload', - 34 => 'javelin-behavior-global-drag-and-drop', - 35 => 'javelin-behavior-phabricator-reveal-content', - 36 => 'phabricator-hovercard', - 37 => 'javelin-behavior-phabricator-hovercards', - 38 => 'javelin-color', - 39 => 'javelin-fx', + 0 => 'javelin-util', + 1 => 'javelin-install', + 2 => 'javelin-event', + 3 => 'javelin-stratcom', + 4 => 'javelin-behavior', + 5 => 'javelin-resource', + 6 => 'javelin-request', + 7 => 'javelin-vector', + 8 => 'javelin-dom', + 9 => 'javelin-json', + 10 => 'javelin-uri', + 11 => 'javelin-workflow', + 12 => 'javelin-mask', + 13 => 'javelin-typeahead', + 14 => 'javelin-typeahead-normalizer', + 15 => 'javelin-typeahead-source', + 16 => 'javelin-typeahead-preloaded-source', + 17 => 'javelin-typeahead-ondemand-source', + 18 => 'javelin-tokenizer', + 19 => 'javelin-history', + 20 => 'javelin-router', + 21 => 'javelin-routable', + 22 => 'javelin-behavior-aphront-basic-tokenizer', + 23 => 'javelin-behavior-workflow', + 24 => 'javelin-behavior-aphront-form-disable-on-submit', + 25 => 'phabricator-keyboard-shortcut-manager', + 26 => 'phabricator-keyboard-shortcut', + 27 => 'javelin-behavior-phabricator-keyboard-shortcuts', + 28 => 'javelin-behavior-refresh-csrf', + 29 => 'javelin-behavior-phabricator-watch-anchor', + 30 => 'javelin-behavior-phabricator-autofocus', + 31 => 'phuix-dropdown-menu', + 32 => 'phuix-action-list-view', + 33 => 'phuix-action-view', + 34 => 'phabricator-phtize', + 35 => 'javelin-behavior-phabricator-oncopy', + 36 => 'phabricator-tooltip', + 37 => 'javelin-behavior-phabricator-tooltips', + 38 => 'phabricator-prefab', + 39 => 'javelin-behavior-device', + 40 => 'javelin-behavior-toggle-class', + 41 => 'javelin-behavior-lightbox-attachments', + 42 => 'phabricator-busy', + 43 => 'javelin-aphlict', + 44 => 'phabricator-notification', + 45 => 'javelin-behavior-aphlict-listen', + 46 => 'javelin-behavior-phabricator-search-typeahead', + 47 => 'javelin-behavior-konami', + 48 => 'javelin-behavior-aphlict-dropdown', + 49 => 'javelin-behavior-history-install', + 50 => 'javelin-behavior-phabricator-gesture', + 51 => 'javelin-behavior-phabricator-active-nav', + 52 => 'javelin-behavior-phabricator-nav', + 53 => 'javelin-behavior-phabricator-remarkup-assist', + 54 => 'phabricator-textareautils', + 55 => 'phabricator-file-upload', + 56 => 'javelin-behavior-global-drag-and-drop', + 57 => 'javelin-behavior-phabricator-reveal-content', + 58 => 'phabricator-hovercard', + 59 => 'javelin-behavior-phabricator-hovercards', + 60 => 'javelin-color', + 61 => 'javelin-fx', + 62 => 'phabricator-draggable-list', + 63 => 'javelin-behavior-phabricator-transaction-list', + 64 => 'javelin-behavior-phabricator-show-all-transactions', + 65 => 'javelin-behavior-phui-timeline-dropdown-menu', + 66 => 'javelin-behavior-doorkeeper-tag', ), 'darkconsole.pkg.js' => array( @@ -2227,6 +2262,7 @@ return array( 15 => 'javelin-behavior-differential-dropdown-menus', 16 => 'javelin-behavior-differential-toggle-files', 17 => 'javelin-behavior-differential-user-select', + 18 => 'javelin-behavior-aphront-more', ), 'diffusion.pkg.css' => array( @@ -2239,29 +2275,6 @@ return array( 1 => 'javelin-behavior-diffusion-commit-graph', 2 => 'javelin-behavior-audit-preview', ), - 'javelin.pkg.js' => - array( - 0 => 'javelin-util', - 1 => 'javelin-install', - 2 => 'javelin-event', - 3 => 'javelin-stratcom', - 4 => 'javelin-behavior', - 5 => 'javelin-resource', - 6 => 'javelin-request', - 7 => 'javelin-vector', - 8 => 'javelin-dom', - 9 => 'javelin-json', - 10 => 'javelin-uri', - 11 => 'javelin-workflow', - 12 => 'javelin-mask', - 13 => 'javelin-typeahead', - 14 => 'javelin-typeahead-normalizer', - 15 => 'javelin-typeahead-source', - 16 => 'javelin-typeahead-preloaded-source', - 17 => 'javelin-typeahead-ondemand-source', - 18 => 'javelin-tokenizer', - 19 => 'javelin-history', - ), 'maniphest.pkg.css' => array( 0 => 'maniphest-task-summary-css', @@ -2274,6 +2287,7 @@ return array( 2 => 'javelin-behavior-maniphest-transaction-preview', 3 => 'javelin-behavior-maniphest-transaction-expand', 4 => 'javelin-behavior-maniphest-subpriority-editor', + 5 => 'javelin-behavior-maniphest-list-editor', ), ), ); diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index bead57c355..692099f2d5 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -1,7 +1,7 @@ array( + 'core.pkg.js' => array( 'javelin-util', 'javelin-install', 'javelin-event', @@ -22,8 +22,8 @@ return array( 'javelin-typeahead-ondemand-source', 'javelin-tokenizer', 'javelin-history', - ), - 'core.pkg.js' => array( + 'javelin-router', + 'javelin-routable', 'javelin-behavior-aphront-basic-tokenizer', 'javelin-behavior-workflow', 'javelin-behavior-aphront-form-disable-on-submit', @@ -53,7 +53,6 @@ return array( 'javelin-behavior-aphlict-dropdown', 'javelin-behavior-history-install', 'javelin-behavior-phabricator-gesture', - 'javelin-behavior-phabricator-active-nav', 'javelin-behavior-phabricator-nav', 'javelin-behavior-phabricator-remarkup-assist', @@ -65,6 +64,11 @@ return array( 'javelin-behavior-phabricator-hovercards', 'javelin-color', 'javelin-fx', + 'phabricator-draggable-list', + 'javelin-behavior-phabricator-transaction-list', + 'javelin-behavior-phabricator-show-all-transactions', + 'javelin-behavior-phui-timeline-dropdown-menu', + 'javelin-behavior-doorkeeper-tag', ), 'core.pkg.css' => array( 'phabricator-core-css', @@ -114,6 +118,16 @@ return array( 'phui-property-list-view-css', 'phui-tag-view-css', 'phui-list-view-css', + + 'font-fontawesome', + 'phui-font-icon-base-css', + 'sprite-main-header-css', + 'phui-box-css', + 'phui-object-box-css', + 'phui-timeline-view-css', + 'sprite-tokens-css', + 'tokens-css', + 'phui-status-list-view-css', ), 'differential.pkg.css' => array( 'differential-core-view-css', @@ -149,6 +163,7 @@ return array( 'javelin-behavior-differential-dropdown-menus', 'javelin-behavior-differential-toggle-files', 'javelin-behavior-differential-user-select', + 'javelin-behavior-aphront-more', ), 'diffusion.pkg.css' => array( 'diffusion-commit-view-css', @@ -169,6 +184,7 @@ return array( 'javelin-behavior-maniphest-transaction-preview', 'javelin-behavior-maniphest-transaction-expand', 'javelin-behavior-maniphest-subpriority-editor', + 'javelin-behavior-maniphest-list-editor', ), 'darkconsole.pkg.js' => array( 'javelin-behavior-dark-console', diff --git a/src/applications/cache/PhabricatorCaches.php b/src/applications/cache/PhabricatorCaches.php index 7a5a2d602f..860abcb130 100644 --- a/src/applications/cache/PhabricatorCaches.php +++ b/src/applications/cache/PhabricatorCaches.php @@ -1,7 +1,8 @@ List of caches. + * @task immutable + */ + private static function buildImmutableCaches() { + $caches = array(); + + $apc = new PhutilKeyValueCacheAPC(); + if ($apc->isAvailable()) { + $caches[] = $apc; + } + + $caches[] = new PhabricatorKeyValueDatabaseCache(); + + return $caches; + } + + /* -( Repository Graph Cache )--------------------------------------------- */ diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php index 0263312bec..8ab192b6f1 100644 --- a/src/infrastructure/celerity/CelerityResourceController.php +++ b/src/infrastructure/celerity/CelerityResourceController.php @@ -34,41 +34,62 @@ abstract class CelerityResourceController extends PhabricatorController { throw new Exception("Only static resources may be served."); } - if (AphrontRequest::getHTTPHeader('If-Modified-Since') && - !PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $dev_mode = PhabricatorEnv::getEnvConfig('phabricator.developer-mode'); + + if (AphrontRequest::getHTTPHeader('If-Modified-Since') && !$dev_mode) { // Return a "304 Not Modified". We don't care about the value of this // field since we never change what resource is served by a given URI. return $this->makeResponseCacheable(new Aphront304Response()); } - $map = $this->getCelerityResourceMap(); + $is_cacheable = (!$dev_mode) && + $this->isCacheableResourceType($type); - if ($map->isPackageResource($path)) { - $resource_names = $map->getResourceNamesForPackageName($path); - if (!$resource_names) { - return new Aphront404Response(); - } + $cache = null; + $data = null; + if ($is_cacheable) { + $cache = PhabricatorCaches::getImmutableCache(); - try { - $data = array(); - foreach ($resource_names as $resource_name) { - $data[] = $map->getResourceDataForName($resource_name); - } - $data = implode("\n\n", $data); - } catch (Exception $ex) { - return new Aphront404Response(); - } - } else { - try { - $data = $map->getResourceDataForName($path); - } catch (Exception $ex) { - return new Aphront404Response(); - } + $request_path = $this->getRequest()->getPath(); + $cache_key = $this->getCacheKey($request_path); + + $data = $cache->getKey($cache_key); } - $xformer = $this->buildResourceTransformer(); - if ($xformer) { - $data = $xformer->transformResource($path, $data); + if ($data === null) { + $map = $this->getCelerityResourceMap(); + + if ($map->isPackageResource($path)) { + $resource_names = $map->getResourceNamesForPackageName($path); + if (!$resource_names) { + return new Aphront404Response(); + } + + try { + $data = array(); + foreach ($resource_names as $resource_name) { + $data[] = $map->getResourceDataForName($resource_name); + } + $data = implode("\n\n", $data); + } catch (Exception $ex) { + return new Aphront404Response(); + } + } else { + try { + $data = $map->getResourceDataForName($path); + } catch (Exception $ex) { + return new Aphront404Response(); + } + } + + $xformer = $this->buildResourceTransformer(); + if ($xformer) { + $data = $xformer->transformResource($path, $data); + } + + if ($cache) { + $cache->setKey($cache_key, $data); + } } $response = new AphrontFileResponse(); @@ -109,4 +130,29 @@ abstract class CelerityResourceController extends PhabricatorController { return $response; } + + /** + * Is it appropriate to cache the data for this resource type in the fast + * immutable cache? + * + * Generally, text resources (which are small, and expensive to process) + * are cached, while other types of resources (which are large, and cheap + * to process) are not. + * + * @param string Resource type. + * @return bool True to enable caching. + */ + private function isCacheableResourceType($type) { + $types = array( + 'js' => true, + 'css' => true, + ); + + return isset($types[$type]); + } + + private function getCacheKey($path) { + return 'celerity:'.$path; + } + }