From: Platonides Date: Mon, 9 Aug 2010 21:53:21 +0000 (+0000) Subject: Use only the page relevant pieces in the parser cache key. Eg. two users with differe... X-Git-Tag: 1.31.0-rc.0~35571 X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=34d35fb6f93755a31f4f5a818a5e96564084da3c;p=lhc%2Fweb%2Fwiklou.git Use only the page relevant pieces in the parser cache key. Eg. two users with different math options will now use the same parsercache entry for articles without tags. The cache key format is kept as a fallback so the old cached entries can be reused. Should boost parsercache hits, but it also makes easier to pollute the parsercache by tag hooks that behave badly, directly using $wgUser or $wgLang. Extensions hooking PageRenderingHash now see !edit=0 and the !printable=1 bits. Fixes bug 24714 - Usage of {{#dateformat: }} in wikis without $wgUseDynamicDates can lead to unexpected results Builds upon r70498, r70498, r70501, r70517, r70651, r70653, r70765, r70780. --- diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 82790c150d..b524e0996f 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -144,6 +144,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN result in a URL ending in "#Hello?" rather than "#Hello.3F". * (bug 8140) Add dedicated CSS classes to Special:Newpages elements * (bug 11005) Add CSS class to empty pages in Special:Newpages +* The parser cache is now shared amongst users whose different settings aren't + used in the page. === Bug fixes in 1.17 === * (bug 17560) Half-broken deletion moved image files to deletion archive @@ -287,6 +289,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * (bug 15470) First letters of filenames are always capitalized by upload JS. * (bug 21215) NoLocalSettings.php doesn't tolerate rewrite rules * (bug 21052) Fix link color for stubs in NewPages +* (bug 24714) Usage of {{#dateformat: }} in wikis without $wgUseDynamicDates no + longer pollutes the parser cache. === API changes in 1.17 === * (bug 22738) Allow filtering by action type on query=logevent. diff --git a/docs/memcached.txt b/docs/memcached.txt index da6add66f2..d8863c914c 100644 --- a/docs/memcached.txt +++ b/docs/memcached.txt @@ -153,16 +153,20 @@ Newtalk: Parser Cache: stored in: $parserMemc controlled by: $wgEnableParserCache - key: $wgDBname:pcache:idhash:$pageid-$renderkey!$hash$edit + key: $wgDBname:pcache:idhash:$pageid-$renderkey!$hash $pageid: id of the page $renderkey: 1 if action=render, 0 otherwise - $hash: hash of user options, see User::getPageRenderingHash() - $edit: '!edit=0' if the user can't edit the page, '' otherwise + $hash: hash of user options applied to the page, see ParserOptions::optionsHash() ex: wikidb:pcache:idhash:1-0!1!0!!en!2 stores: ParserOutput object - modified by: Article::editUpdates() - expriy: $wgParserCacheExpireTime or one hour if it contains specific magic - words + modified by: Article::editUpdates() or Article::getOutputFromWikitext() + expiry: $wgParserCacheExpireTime or less if it contains short lived functions + + key: $wgDBname:pcache:idoptions:$pageid + stores: CacheTime object with an additional list of used options for the hash, + serves as ParserCache pointer. + modified by: ParserCache::save() + expiry: The same as the ParserCache entry it points to. Ping limiter: controlled by: $wgRateLimits diff --git a/includes/Article.php b/includes/Article.php index d32dcc0ed8..9ab205eee1 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -996,6 +996,8 @@ class Article { case 4: # Run the parse, protected by a pool counter wfDebug( __METHOD__ . ": doing uncached parse\n" ); + + $this->checkTouched(); $key = $parserCache->getKey( $this, $parserOptions ); $poolCounter = PoolCounter::factory( 'Article::view', $key ); $dirtyCallback = $useParserCache ? array( $this, 'tryDirtyCache' ) : false; @@ -1493,10 +1495,9 @@ class Article { * @return boolean */ public function tryDirtyCache() { - global $wgOut; $parserCache = ParserCache::singleton(); - $options = $this->getParserOptions(); + $options = clone $this->getParserOptions(); if ( $wgOut->isPrintable() ) { $options->setIsPrintable( true ); @@ -3609,8 +3610,8 @@ class Article { $edit->revid = $revid; $edit->newText = $text; $edit->pst = $this->preSaveTransform( $text ); - $options = $this->getParserOptions(); - $edit->output = $wgParser->parse( $edit->pst, $this->mTitle, $options, true, true, $revid ); + $edit->popts = clone $this->getParserOptions(); + $edit->output = $wgParser->parse( $edit->pst, $this->mTitle, $edit->popts, true, true, $revid ); $edit->oldText = $this->getContent(); $this->mPreparedEdit = $edit; @@ -3649,9 +3650,8 @@ class Article { # Save it to the parser cache if ( $wgEnableParserCache ) { - $popts = $this->getParserOptions(); $parserCache = ParserCache::singleton(); - $parserCache->save( $editInfo->output, $this, $popts ); + $parserCache->save( $editInfo->output, $this, $editInfo->popts ); } # Update the links tables @@ -4418,7 +4418,7 @@ class Article { global $wgParser, $wgEnableParserCache, $wgUseFileCache; if ( !$parserOptions ) { - $parserOptions = $this->getParserOptions(); + $parserOptions = clone $this->getParserOptions(); } $time = - wfTime(); diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index 29775c6134..44bc614e22 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -266,6 +266,7 @@ class Parser { $this->clearState(); } + $options->resetUsage(); $this->mOptions = $options; $this->setTitle( $title ); # Page title has to be set for the pre-processor @@ -454,6 +455,7 @@ class Parser { wfProfileIn( __METHOD__ ); $this->clearState(); $this->setOutputType( self::OT_PREPROCESS ); + $options->resetUsage(); $this->mOptions = $options; $this->setTitle( $title ); if ( $revid !== null ) { @@ -477,6 +479,7 @@ class Parser { # Parser (re)initialisation $this->clearState(); $this->setOutputType( self::OT_PLAIN ); + $options->resetUsage(); $this->mOptions = $options; $this->setTitle( $title ); @@ -4041,6 +4044,7 @@ class Parser { * @return String: the altered wiki markup */ public function preSaveTransform( $text, Title $title, $user, $options, $clearState = true ) { + $options->resetUsage(); $this->mOptions = $options; $this->setTitle( $title ); $this->setOutputType( self::OT_WIKI ); @@ -4265,6 +4269,7 @@ class Parser { */ public function startExternalParse( &$title, $options, $outputType, $clearState = true ) { $this->setTitle( $title ); + $options->resetUsage(); $this->mOptions = $options; $this->setOutputType( $outputType ); if ( $clearState ) { @@ -5140,6 +5145,7 @@ class Parser { $title = Title::newFromText( $title ); } $this->mTitle = $title; + $options->resetUsage(); $this->mOptions = $options; $this->setOutputType( $outputType ); $text = $this->replaceVariables( $text ); diff --git a/includes/parser/ParserCache.php b/includes/parser/ParserCache.php index 1f0458b0a7..95fe6a1faf 100644 --- a/includes/parser/ParserCache.php +++ b/includes/parser/ParserCache.php @@ -31,45 +31,93 @@ class ParserCache { } $this->mMemc = $memCached; } - - function getKey( $article, $popts ) { + + protected function getParserOutputKey( $article, $hash ) { global $wgRequest; - - if( $popts instanceof User ) // It used to be getKey( &$article, &$user ) - $popts = ParserOptions::newFromUser( $popts ); - - $user = $popts->mUser; - $printable = ( $popts->getIsPrintable() ) ? '!printable=1' : ''; - $hash = $user->getPageRenderingHash(); - if( ! $popts->getEditSection() ) { - // section edit links have been suppressed - $edit = '!edit=0'; - } else { - $edit = ''; - } + // idhash seem to mean 'page id' + 'rendering hash' (r3710) $pageid = $article->getID(); $renderkey = (int)($wgRequest->getVal('action') == 'render'); - $key = wfMemcKey( 'pcache', 'idhash', "{$pageid}-{$renderkey}!{$hash}{$edit}{$printable}" ); + + $key = wfMemcKey( 'pcache', 'idhash', "{$pageid}-{$renderkey}!{$hash}" ); return $key; } + protected function getOptionsKey( $article ) { + $pageid = $article->getID(); + return wfMemcKey( 'pcache', 'idoptions', "{$pageid}" ); + } + function getETag( $article, $popts ) { - return 'W/"' . $this->getKey($article, $popts) . "--" . $article->mTouched. '"'; + return 'W/"' . $this->getParserOutputKey( $article, + $popts->optionsHash( ParserOptions::legacyOptions() ) ) . + "--" . $article->mTouched . '"'; } - function getDirty( $article, $popts ) { - $key = $this->getKey( $article, $popts ); - wfDebug( "Trying parser cache $key\n" ); - $value = $this->mMemc->get( $key ); + /** + * Retrieve the ParserOutput from ParserCache, even if it's outdated. + */ + public function getDirty( $article, $popts ) { + $value = $this->mMemc->get( $article, $popts, true ); return is_object( $value ) ? $value : false; } - function get( $article, $popts ) { + /** + * Used to provide a unique id for the PoolCounter. + * It would be preferable to have this code in get() + * instead of having Article looking in our internals. + * + * Precondition: $article->checkTouched() has been called. + */ + public function getKey( $article, $popts, $useOutdated = true ) { + global $wgCacheEpoch; + + // Determine the options which affect this article + $optionsKey = $this->mMemc->get( $this->getOptionsKey( $article ) ); + if ( $optionsKey !== false ) { + if ( !$useOutdated && $optionsKey->expired( $article->mTouched ) ) { + wfIncrStats( "pcache_miss_expired" ); + $cacheTime = $optionsKey->getCacheTime(); + wfDebug( "Parser options key expired, touched {$article->mTouched}, epoch $wgCacheEpoch, cached $cacheTime\n" ); + return false; + } + + $usedOptions = $optionsKey->mUsedOptions; + wfDebug( "Parser cache options found.\n" ); + } else { + # TODO: Fail here $wgParserCacheExpireTime after deployment unless $useOutdated + + $usedOptions = ParserOptions::legacyOptions(); + } + + return $this->getParserOutputKey( $article, $popts->optionsHash( $usedOptions ) ); + } + + /** + * Retrieve the ParserOutput from ParserCache. + * false if not found or outdated. + */ + public function get( $article, $popts, $useOutdated = false ) { global $wgCacheEpoch; wfProfileIn( __METHOD__ ); - $value = $this->getDirty( $article, $popts ); + $canCache = $article->checkTouched(); + if ( !$canCache ) { + // It's a redirect now + wfProfileOut( __METHOD__ ); + return false; + } + + // Having called checkTouched() ensures this will be loaded + $touched = $article->mTouched; + + $parserOutputKey = $this->getKey( $article, $popts, $useOutdated ); + if ( $parserOutputKey === false ) { + wfProfileOut( __METHOD__ ); + return false; + } + + $value = $this->mMemc->get( $parserOutputKey ); if ( !$value ) { wfDebug( "Parser cache miss.\n" ); wfIncrStats( "pcache_miss_absent" ); @@ -78,18 +126,10 @@ class ParserCache { } wfDebug( "Found.\n" ); - # Invalid if article has changed since the cache was made - $canCache = $article->checkTouched(); - $cacheTime = $value->getCacheTime(); - $touched = $article->mTouched; - if ( !$canCache || $value->expired( $touched ) ) { - if ( !$canCache ) { - wfIncrStats( "pcache_miss_invalid" ); - wfDebug( "Invalid cached redirect, touched $touched, epoch $wgCacheEpoch, cached $cacheTime\n" ); - } else { - wfIncrStats( "pcache_miss_expired" ); - wfDebug( "Key expired, touched $touched, epoch $wgCacheEpoch, cached $cacheTime\n" ); - } + + if ( !$useOutdated && $value->expired( $touched ) ) { + wfIncrStats( "pcache_miss_expired" ); + wfDebug( "ParserOutput key expired, touched $touched, epoch $wgCacheEpoch, cached $cacheTime\n" ); $value = false; } else { if ( isset( $value->mTimestamp ) ) { @@ -102,25 +142,37 @@ class ParserCache { return $value; } - function save( $parserOutput, $article, $popts ){ - $key = $this->getKey( $article, $popts ); + + public function save( $parserOutput, $article, $popts ) { $expire = $parserOutput->getCacheExpiry(); - if( $expire > 0 ) { + if( $expire > 0 ) { $now = wfTimestampNow(); + + $optionsKey = new CacheTime; + $optionsKey->mUsedOptions = $popts->usedOptions(); + $optionsKey->updateCacheExpiry( $expire ); + + $optionsKey->setCacheTime( $now ); $parserOutput->setCacheTime( $now ); + $optionsKey->setContainsOldMagic( $parserOutput->containsOldMagic() ); + + $parserOutputKey = $this->getParserOutputKey( $article, $popts->optionsHash( $optionsKey->mUsedOptions ) ); + // Save the timestamp so that we don't have to load the revision row on view $parserOutput->mTimestamp = $article->getTimestamp(); - $parserOutput->mText .= "\n\n"; - wfDebug( "Saved in parser cache with key $key and timestamp $now\n" ); + $parserOutput->mText .= "\n\n"; + wfDebug( "Saved in parser cache with key $parserOutputKey and timestamp $now\n" ); - $this->mMemc->set( $key, $parserOutput, $expire ); + // Save the parser output + $this->mMemc->set( $parserOutputKey, $parserOutput, $expire ); + // ...and its pointer + $this->mMemc->set( $this->getOptionsKey( $article ), $optionsKey, $expire ); } else { wfDebug( "Parser output was marked as uncacheable and has not been saved.\n" ); } } - } diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php index 9b1b9e5836..72e4242d7a 100644 --- a/includes/parser/ParserOptions.php +++ b/includes/parser/ParserOptions.php @@ -38,13 +38,18 @@ class ParserOptions { var $mIsSectionPreview; # Parsing the page for a "preview" operation on a single section var $mIsPrintable; # Parsing the printable version of the page + + protected $accessedOptions; + function getUseDynamicDates() { return $this->mUseDynamicDates; } function getInterwikiMagic() { return $this->mInterwikiMagic; } function getAllowExternalImages() { return $this->mAllowExternalImages; } function getAllowExternalImagesFrom() { return $this->mAllowExternalImagesFrom; } function getEnableImageWhitelist() { return $this->mEnableImageWhitelist; } - function getEditSection() { return $this->mEditSection; } - function getNumberHeadings() { return $this->mNumberHeadings; } + function getEditSection() { $this->accessedOptions['editsection'] = true; + return $this->mEditSection; } + function getNumberHeadings() { $this->accessedOptions['numberheadings'] = true; + return $this->mNumberHeadings; } function getAllowSpecialInclusion() { return $this->mAllowSpecialInclusion; } function getTidy() { return $this->mTidy; } function getInterfaceMessage() { return $this->mInterfaceMessage; } @@ -58,12 +63,15 @@ class ParserOptions { function getEnableLimitReport() { return $this->mEnableLimitReport; } function getCleanSignatures() { return $this->mCleanSignatures; } function getExternalLinkTarget() { return $this->mExternalLinkTarget; } - function getMath() { return $this->mMath; } - function getThumbSize() { return $this->mThumbSize; } + function getMath() { $this->accessedOptions['math'] = true; + return $this->mMath; } + function getThumbSize() { $this->accessedOptions['thumbsize'] = true; + return $this->mThumbSize; } function getIsPreview() { return $this->mIsPreview; } function getIsSectionPreview() { return $this->mIsSectionPreview; } - function getIsPrintable() { return $this->mIsPrintable; } + function getIsPrintable() { $this->accessedOptions['printable'] = true; + return $this->mIsPrintable; } function getSkin() { if ( !isset( $this->mSkin ) ) { @@ -73,6 +81,7 @@ class ParserOptions { } function getDateFormat() { + $this->accessedOptions['dateformat'] = true; if ( !isset( $this->mDateFormat ) ) { $this->mDateFormat = $this->mUser->getDatePreference(); } @@ -90,6 +99,7 @@ class ParserOptions { # Using this fragments the cache and is discouraged. Yes, {{int: }} uses this, # producing inconsistent tables (Bug 14404). function getUserLang() { + $this->accessedOptions['userlang'] = true; return $this->mUserLang; } @@ -191,4 +201,111 @@ class ParserOptions { wfProfileOut( __METHOD__ ); } + + /** + * Returns the options from this ParserOptions which have been used. + */ + public function usedOptions() { + return array_keys( $this->accessedOptions ); + } + + /** + * Resets the memory of options usage. + */ + public function resetUsage() { + $this->accessedOptions = array(); + } + + /** + * Returns the full array of options that would have been used by + * in 1.16. + * Used to get the old parser cache entries when available. + */ + public static function legacyOptions() { + global $wgUseDynamicDates; + $legacyOpts = array( 'math', 'stubthreshold', 'numberheadings', 'userlang', 'thumbsize', 'editsection', 'printable' ); + if ( $wgUseDynamicDates ) { + $legacyOpts[] = 'dateformat'; + } + return $legacyOpts; + } + + /** + * Generate a hash string with the values set on these ParserOptions + * for the keys given in the array. + * This will be used as part of the hash key for the parser cache, + * so users sharign the options with vary for the same page share + * the same cached data safely. + * + * Replaces User::getPageRenderingHash() + * + * Extensions which require it should install 'PageRenderingHash' hook, + * which will give them a chance to modify this key based on their own + * settings. + * + * @since 1.17 + * @return \string Page rendering hash + */ + public function optionsHash( $forOptions ) { + global $wgContLang, $wgRenderHashAppend; + + $confstr = ''; + + if ( in_array( 'math', $forOptions ) ) + $confstr .= $this->mMath; + else + $confstr .= '*'; + + + // Space assigned for the stubthreshold but unused + // since it disables the parser cache, its value will always + // be 0 when this function is called by parsercache. + // The conditional is here to avoid a confusing 0 + if ( in_array( 'stubthreshold', $forOptions ) ) + $confstr .= '!0' ; + else + $confstr .= '!*' ; + + if ( in_array( 'dateformat', $forOptions ) ) + $confstr .= '!' . $this->mDateFormat; + + if ( in_array( 'numberheadings', $forOptions ) ) + $confstr .= '!' . ( $this->mNumberHeadings ? '1' : '' ); + else + $confstr .= '!*'; + + if ( in_array( 'userlang', $forOptions ) ) + $confstr .= '!' . $this->mUserLang; + else + $confstr .= '!*'; + + if ( in_array( 'thumbsize', $forOptions ) ) + $confstr .= '!' . $this->mThumbSize; + else + $confstr .= '!*'; + + // add in language specific options, if any + // FIXME: This is just a way of retrieving the url/user preferred variant + $confstr .= $wgContLang->getExtraHashOptions(); + + // Since the skin could be overloading link(), it should be + // included here but in practice, none of our skins do that. + // $confstr .= "!" . $this->mSkin->getSkinName(); + + $confstr .= $wgRenderHashAppend; + + if ( !$this->mEditSection && in_array( 'editsection', $forOptions ) ) + $confstr .= '!edit=0'; + if ( $this->mIsPrintable && in_array( 'printable', $forOptions ) ) + $confstr .= '!printable=1'; + + // Give a chance for extensions to modify the hash, if they have + // extra options or other effects on the parser cache. + wfRunHooks( 'PageRenderingHash', array( &$confstr ) ); + + // Make it a valid memcached key fragment + $confstr = str_replace( ' ', '_', $confstr ); + + return $confstr; + } }