Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/210845
Change subject: resourceloader: Don't cache minification of user.tokens
......................................................................
resourceloader: Don't cache minification of user.tokens
As of b1e4006b4, the tokens are different on every request.
Caching these is completely useless because the cache entry is
simply unreachable and is extra overhead on every request for
logged-in users to save content to Memcached.
Whether they should be minified at all and whether they perhaps
shouldn't change on every request is a separate matter.
Bug: T84960
Change-Id: I6016e4b01e44e0acbfd6d49f7d99555e2290c9cb
---
M includes/OutputPage.php
M includes/resourceloader/ResourceLoader.php
M includes/resourceloader/ResourceLoaderStartUpModule.php
3 files changed, 62 insertions(+), 45 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/45/210845/1
diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 8f9f9c6..9b1bd24 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2976,8 +2976,10 @@
// Load embeddable private modules before any loader links
// This needs to be TYPE_COMBINED so these modules are properly
wrapped
// in mw.loader.implement() calls and deferred until mw.user is
available
- $embedScripts = array( 'user.options', 'user.tokens' );
+ $embedScripts = array( 'user.options' );
$links[] = $this->makeResourceLoaderLink( $embedScripts,
ResourceLoaderModule::TYPE_COMBINED );
+ // Separate user.tokens as otherwise caching will be allowed
(T84960)
+ $links[] = $this->makeResourceLoaderLink( 'user.tokens',
ResourceLoaderModule::TYPE_COMBINED );
// Scripts and messages "only" requests marked for top inclusion
// Messages should go first
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 8c9c130..64d79e1 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -169,58 +169,65 @@
*
* @param string $filter Name of filter to run
* @param string $data Text to filter, such as JavaScript or CSS text
- * @param string $cacheReport Whether to include the cache key report
+ * @param array $options For back-compat, can also be the boolean value
for "cacheReport". Keys:
+ * - (bool) cache: Whether to allow caching this data. Default: true.
+ * - (bool) cacheReport: Whether to include the "cache key" report
comment. Default: true.
* @return string Filtered data, or a comment containing an error
message
*/
- public function filter( $filter, $data, $cacheReport = true ) {
+ public function filter( $filter, $data, $options = array() ) {
+ // Back-compat
+ if ( is_bool( $options ) ) {
+ $options = array( 'cacheReport' => $options );
+ }
+ // Defaults
+ $options += array( 'cache' => true, 'cacheReport' => true );
- // For empty/whitespace-only data or for unknown filters, don't
perform
- // any caching or processing
+ // For empty/whitespace-only data or for unknown filters, don't
do any caching or processing
if ( trim( $data ) === '' || !in_array( $filter, array(
'minify-js', 'minify-css' ) ) ) {
+ wfDebugLog( 'resourceloader', __METHOD__ . ": Invalid
filter: $filter" );
return $data;
}
- // Try for cache hit
- // Use CACHE_ANYTHING since filtering is very slow compared to
DB queries
- $key = wfMemcKey( 'resourceloader', 'filter', $filter,
self::$filterCacheVersion, md5( $data ) );
- $cache = wfGetCache( CACHE_ANYTHING );
- $cacheEntry = $cache->get( $key );
- if ( is_string( $cacheEntry ) ) {
- wfIncrStats( "rl-$filter-cache-hits" );
- return $cacheEntry;
- }
-
- $result = '';
- // Run the filter - we've already verified one of these will
work
- try {
- wfIncrStats( "rl-$filter-cache-misses" );
- switch ( $filter ) {
- case 'minify-js':
- $result = JavaScriptMinifier::minify(
$data,
- $this->config->get(
'ResourceLoaderMinifierStatementsOnOwnLine' ),
- $this->config->get(
'ResourceLoaderMinifierMaxLineLength' )
- );
- if ( $cacheReport ) {
- $result .= "\n/* cache key:
$key */";
- }
- break;
- case 'minify-css':
- $result = CSSMin::minify( $data );
- if ( $cacheReport ) {
- $result .= "\n/* cache key:
$key */";
- }
- break;
+ if ( !$options['cache'] ) {
+ $result = $this->applyFilter( $filter, $data );
+ } else {
+ // Use CACHE_ANYTHING since filtering is very slow
compared to DB queries
+ $key = wfMemcKey( 'resourceloader', 'filter', $filter,
self::$filterCacheVersion, md5( $data ) );
+ $cache = wfGetCache( CACHE_ANYTHING );
+ $cacheEntry = $cache->get( $key );
+ if ( is_string( $cacheEntry ) ) {
+ wfIncrStats( "rl-$filter-cache-hits" );
+ return $cacheEntry;
}
-
- // Save filtered text to Memcached
- $cache->set( $key, $result );
- } catch ( Exception $e ) {
- MWExceptionHandler::logException( $e );
- wfDebugLog( 'resourceloader', __METHOD__ . ":
minification failed: $e" );
- $this->errors[] = self::formatExceptionNoComment( $e );
+ $result = '';
+ try {
+ wfIncrStats( "rl-$filter-cache-misses" );
+ $result = $this->applyFilter( $filter, $data );
+ if ( $options['cacheReport'] ) {
+ $result .= "\n/* cache key: $key */";
+ }
+ $cache->set( $key, $result );
+ } catch ( Exception $e ) {
+ MWExceptionHandler::logException( $e );
+ wfDebugLog( 'resourceloader', __METHOD__ . ":
minification failed: $e" );
+ $this->errors[] =
self::formatExceptionNoComment( $e );
+ }
}
return $result;
+ }
+
+ private function applyFilter( $filter, $data ) {
+ switch ( $filter ) {
+ case 'minify-js':
+ return JavaScriptMinifier::minify(
$data,
+ $this->config->get(
'ResourceLoaderMinifierStatementsOnOwnLine' ),
+ $this->config->get(
'ResourceLoaderMinifierMaxLineLength' )
+ );
+ case 'minify-css':
+ return CSSMin::minify( $data );
+ }
+ return $data;
}
/* Methods */
@@ -1059,11 +1066,19 @@
}
}
+ $enableFilterCache = true;
+ if ( count( $modules ) === 1 && reset( $modules ) instanceof
ResourceLoaderUserTokensModule ) {
+ // If we're building the embedded user.tokens, don't
cache (T84960)
+ $enableFilterCache = false;
+ }
+
if ( !$context->getDebug() ) {
if ( $context->getOnly() === 'styles' ) {
$out = $this->filter( 'minify-css', $out );
} else {
- $out = $this->filter( 'minify-js', $out );
+ $out = $this->filter( 'minify-js', $out, array(
+ 'cache' => $enableFilterCache
+ ) );
}
}
diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php
b/includes/resourceloader/ResourceLoaderStartUpModule.php
index 48b3576..5886a7c 100644
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -218,9 +218,9 @@
if ( $skipFunction !== null &&
!ResourceLoader::inDebugMode() ) {
$skipFunction = $resourceLoader->filter(
'minify-js',
$skipFunction,
- // There will potentially be lots of
these little string in the registrations
+ // 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
in non-debug mode.
+ // "/* cache key: ... */" all over it.
/* cacheReport = */ false
);
}
--
To view, visit https://gerrit.wikimedia.org/r/210845
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6016e4b01e44e0acbfd6d49f7d99555e2290c9cb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
