Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

resourceloader: Don't cache minification of user.tokens

Browse source 
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
This commit is contained in:
Timo Tijhof 2015-05-14 02:31:42 +01:00 committed by Ori.livneh
parent 4e90d55fdc
commit 6fa4893928
3 changed files with 65 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
includes/OutputPage.php
View file

@ -2981,8 +2981,10 @@ class OutputPage extends ContextSource {
// 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
$links[] = $this->makeResourceLoaderLink(

101
includes/resourceloader/ResourceLoader.php
View file

@ -169,60 +169,71 @@ class ResourceLoader {
*
* @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
if ( trim( $data ) === '' || !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) {
// Don't filter empty content
if ( trim( $data ) === '' ) {
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;
if ( !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) {
wfDebugLog( 'resourceloader', __METHOD__ . ": Invalid filter: $filter" );
return $data;
}
$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;
}
$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 );
}
// 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 );
}
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 */
/**
@ -1078,11 +1089,19 @@ MESSAGE;
}
}
$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
) );
}
}

4
includes/resourceloader/ResourceLoaderStartUpModule.php
View file

@ -217,9 +217,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
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
);
}