From ca30efa30aaa50e8625cc6b98aa089a842ee40ad Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Thu, 1 Oct 2015 11:05:08 -0700 Subject: [PATCH] resourceloader: cache minified user and site modules * Add support for a '/* @nomin */' annotation in ResourceLoader. If present in JavaScript or CSS, the code will not be minified or cached. This allows modules like the ResourceLoaderUserTokensModule to declare themselves unfit for minification / caching without requiring a complicated refactor. * Make ResourceLoader::filter() static, at the cost of not having minifier errors in the ResourceLoader log bucket. (They will continue to be logged as exceptions, however). Change-Id: Ic1d802ee20565e61046bfbd8fd209bc56a4cbd6c --- includes/resourceloader/ResourceLoader.php | 120 +++++++----------- .../resourceloader/ResourceLoaderModule.php | 4 +- .../ResourceLoaderStartUpModule.php | 8 +- .../ResourceLoaderUserTokensModule.php | 10 +- 4 files changed, 58 insertions(+), 84 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 31b3651e70..002b4eac8c 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -87,6 +87,9 @@ class ResourceLoader implements LoggerAwareInterface { */ private $logger; + /** @var string JavaScript / CSS pragma to disable minification. **/ + const FILTER_NOMIN = ' /* @nomin */ '; + /** * Load information stored in the database about modules. * @@ -180,79 +183,57 @@ class ResourceLoader implements LoggerAwareInterface { * * @param string $filter Name of filter to run * @param string $data Text to filter, such as JavaScript or CSS text - * @param array $options For back-compat, can also be the boolean value for "cacheReport". Keys: + * @param array $options Keys: * - (bool) cache: Whether to allow caching this data. Default: true. - * - (bool) cacheReport: Whether to include the "cache key" report comment. Default: false. * @return string Filtered data, or a comment containing an error message */ - public function filter( $filter, $data, $options = array() ) { - // Back-compat - if ( is_bool( $options ) ) { - $options = array( 'cacheReport' => $options ); - } - // Defaults - $options += array( 'cache' => true, 'cacheReport' => false ); - $stats = RequestContext::getMain()->getStats(); - - // Don't filter empty content - if ( trim( $data ) === '' ) { + public static function filter( $filter, $data, Array $options = array() ) { + if ( strpos( $data, ResourceLoader::FILTER_NOMIN ) !== false ) { return $data; } - if ( !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) { - $this->logger->warning( 'Invalid filter {filter}', array( - 'filter' => $filter - ) ); - return $data; + if ( isset( $options['cache'] ) && $options['cache'] === false ) { + return self::applyFilter( $filter, $data ); } - if ( !$options['cache'] ) { - $result = self::applyFilter( $filter, $data, $this->config ); + $stats = RequestContext::getMain()->getStats(); + $cache = ObjectCache::newAccelerator( CACHE_ANYTHING ); + + $key = wfGlobalCacheKey( + 'resourceloader', + 'filter', + $filter, + self::$filterCacheVersion, md5( $data ) + ); + + $result = $cache->get( $key ); + if ( $result === false ) { + $stats->increment( "resourceloader_cache.$filter.miss" ); + $result = self::applyFilter( $filter, $data ); + $cache->set( $key, $result, 24 * 3600 ); } else { - $key = wfGlobalCacheKey( - 'resourceloader', - 'filter', - $filter, - self::$filterCacheVersion, md5( $data ) - ); - $cache = wfGetCache( wfIsHHVM() ? CACHE_ACCEL : CACHE_ANYTHING ); - $cacheEntry = $cache->get( $key ); - if ( is_string( $cacheEntry ) ) { - $stats->increment( "resourceloader_cache.$filter.hit" ); - return $cacheEntry; - } - $result = ''; - try { - $statStart = microtime( true ); - $result = self::applyFilter( $filter, $data, $this->config ); - $statTiming = microtime( true ) - $statStart; - $stats->increment( "resourceloader_cache.$filter.miss" ); - $stats->timing( "resourceloader_cache.$filter.timing", 1000 * $statTiming ); - if ( $options['cacheReport'] ) { - $result .= "\n/* cache key: $key */"; - } - // Set a TTL since HHVM's APC doesn't have any limitation or eviction logic. - $cache->set( $key, $result, 24 * 3600 ); - } catch ( Exception $e ) { - MWExceptionHandler::logException( $e ); - $this->logger->warning( 'Minification failed: {exception}', array( - 'exception' => $e - ) ); - $this->errors[] = self::formatExceptionNoComment( $e ); - } + $stats->increment( "resourceloader_cache.$filter.hit" ); + } + if ( $result === null ) { + // Cached failure + $result = $data; } return $result; } - private static function applyFilter( $filter, $data, Config $config ) { - switch ( $filter ) { - case 'minify-js': - return JavaScriptMinifier::minify( $data ); - case 'minify-css': - return CSSMin::minify( $data ); + private static function applyFilter( $filter, $data ) { + $data = trim( $data ); + if ( $data ) { + try { + $data = ( $filter === 'minify-css' ) + ? CSSMin::minify( $data ) + : JavaScriptMinifier::minify( $data ); + } catch ( Exception $e ) { + MWExceptionHandler::logException( $e ); + return null; + } } - return $data; } @@ -1040,11 +1021,7 @@ MESSAGE; } if ( !$context->getDebug() ) { - // Don't cache private modules. This is especially important in the case - // of modules which change every time they are built, like the embedded - // user.tokens module (bug T84960). - $filterOptions = array( 'cache' => ( $module->getGroup() !== 'private' ) ); - $strContent = $this->filter( $filter, $strContent, $filterOptions ); + $strContent = self::filter( $filter, $strContent ); } $out .= $strContent; @@ -1077,7 +1054,7 @@ MESSAGE; if ( count( $states ) ) { $stateScript = self::makeLoaderStateScript( $states ); if ( !$context->getDebug() ) { - $stateScript = $this->filter( 'minify-js', $stateScript ); + $stateScript = self::filter( 'minify-js', $stateScript ); } $out .= $stateScript; } @@ -1118,8 +1095,7 @@ MESSAGE; // Minify manually because the general makeModuleResponse() minification won't be // effective here due to the script being a string instead of a function. (T107377) if ( !ResourceLoader::inDebugMode() ) { - $scripts = self::applyFilter( 'minify-js', $scripts, - ConfigFactory::getDefaultInstance()->makeConfig( 'main' ) ); + $scripts = self::filter( 'minify-js', $scripts ); } } else { $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" ); @@ -1416,13 +1392,11 @@ MESSAGE; * @return string */ public static function makeConfigSetScript( array $configuration ) { - if ( ResourceLoader::inDebugMode() ) { - return Xml::encodeJsCall( 'mw.config.set', array( $configuration ), true ); - } - - $config = RequestContext::getMain()->getConfig(); - $js = Xml::encodeJsCall( 'mw.config.set', array( $configuration ), false ); - return self::applyFilter( 'minify-js', $js, $config ); + return Xml::encodeJsCall( + 'mw.config.set', + array( $configuration ), + ResourceLoader::inDebugMode() + ) . ResourceLoader::FILTER_NOMIN; } /** diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 7f7aa7668e..4ddfe554af 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -619,11 +619,11 @@ abstract class ResourceLoaderModule { foreach ( $style as $cssText ) { if ( is_string( $cssText ) ) { $stylePairs[$media][] = - $rl->filter( 'minify-css', $cssText ); + ResourceLoader::filter( 'minify-css', $cssText ); } } } elseif ( is_string( $style ) ) { - $stylePairs[$media] = $rl->filter( 'minify-css', $style ); + $stylePairs[$media] = ResourceLoader::filter( 'minify-css', $style ); } } } diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php index e5f3fb8bc2..1857d23ad0 100644 --- a/includes/resourceloader/ResourceLoaderStartUpModule.php +++ b/includes/resourceloader/ResourceLoaderStartUpModule.php @@ -221,13 +221,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { $skipFunction = $module->getSkipFunction(); if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) { - $skipFunction = $resourceLoader->filter( 'minify-js', - $skipFunction, - // There will potentially be lots of these little strings in the registrations - // manifest, we don't want to blow up the startup module with - // "/* cache key: ... */" all over it. - /* cacheReport = */ false - ); + $skipFunction = ResourceLoader::filter( 'minify-js', $skipFunction ); } $registryData[$name] = array( diff --git a/includes/resourceloader/ResourceLoaderUserTokensModule.php b/includes/resourceloader/ResourceLoaderUserTokensModule.php index ccd1dfd0b6..28f1b9a839 100644 --- a/includes/resourceloader/ResourceLoaderUserTokensModule.php +++ b/includes/resourceloader/ResourceLoaderUserTokensModule.php @@ -51,14 +51,20 @@ class ResourceLoaderUserTokensModule extends ResourceLoaderModule { } /** + * Generate the JavaScript content of this module. + * + * Add '@nomin' annotation to prevent the module's contents from getting + * cached (T84960). + * * @param ResourceLoaderContext $context * @return string */ public function getScript( ResourceLoaderContext $context ) { - return Xml::encodeJsCall( 'mw.user.tokens.set', + return Xml::encodeJsCall( + 'mw.user.tokens.set', array( $this->contextUserTokens( $context ) ), ResourceLoader::inDebugMode() - ); + ) . ResourceLoader::FILTER_NOMIN; } /** -- 2.20.1