Use only the page relevant pieces in the parser cache key. Eg. two users with differe...
authorPlatonides <platonides@users.mediawiki.org>
Mon, 9 Aug 2010 21:53:21 +0000 (21:53 +0000)
committerPlatonides <platonides@users.mediawiki.org>
Mon, 9 Aug 2010 21:53:21 +0000 (21:53 +0000)
use the same parsercache entry for articles without <math> 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.

RELEASE-NOTES
docs/memcached.txt
includes/Article.php
includes/parser/Parser.php
includes/parser/ParserCache.php
includes/parser/ParserOptions.php

index 82790c1..b524e09 100644 (file)
@@ -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.
index da6add6..d8863c9 100644 (file)
@@ -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
index d32dcc0..9ab205e 100644 (file)
@@ -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();
index 29775c6..44bc614 100644 (file)
@@ -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 );
index 1f0458b..95fe6a1 100644 (file)
@@ -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<!-- Saved in parser cache with key $key and timestamp $now -->\n";
-                       wfDebug( "Saved in parser cache with key $key and timestamp $now\n" );
+                       $parserOutput->mText .= "\n<!-- Saved in parser cache with key $parserOutputKey and timestamp $now -->\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" );
                }
        }
-
 }
index 9b1b9e5..72e4242 100644 (file)
@@ -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;
+       }
 }