Try to predict the rev_id when preparing edits
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 19 Jun 2016 05:30:21 +0000 (22:30 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Wed, 29 Jun 2016 12:39:33 +0000 (05:39 -0700)
During both the edit stash and first parse in on page save,
guess what the rev_id will be and use that instead of null.
Only reparse if it turns out to be wrong. This avoids extra
parsing on wikis that have low-medium traffic, and does not
cost much. The parsing that can be avoided is:
a) in doEditContent() by using the stash
b) in doEditUpdates() by using the doEditContent() result,
   whether that was able to use the stash or not itself

Also improved the parse operation logging in save paths.

Bug: T137900
Change-Id: Ic6faae70a78b4e223e4d3585cefd482c0fa00677

includes/api/ApiStashEdit.php
includes/page/WikiPage.php
includes/parser/Parser.php
includes/parser/ParserOptions.php
includes/parser/ParserOutput.php

index c8a330a..446a98c 100644 (file)
@@ -258,7 +258,10 @@ class ApiStashEdit extends ApiBase {
                } elseif ( $editInfo->output->getFlag( 'vary-revision' ) ) {
                        // This can be used for the initial parse, e.g. for filters or doEditContent(),
                        // but a second parse will be triggered in doEditUpdates(). This is not optimal.
-                       $logger->info( "Partially usable cache for key '$key' ('$title') [vary_revision]." );
+                       $logger->info( "Cache for key '$key' ('$title') has vary_revision." );
+               } elseif ( $editInfo->output->getFlag( 'vary-revision-id' ) ) {
+                       // Similar to the above if we didn't guess the ID correctly.
+                       $logger->info( "Cache for key '$key' ('$title') has vary_revision_id." );
                }
 
                return $editInfo;
index a416d56..b06b519 100644 (file)
@@ -20,6 +20,8 @@
  * @file
  */
 
+use \MediaWiki\Logger\LoggerFactory;
+
 /**
  * Class representing a MediaWiki article and history.
  *
@@ -2106,6 +2108,16 @@ class WikiPage implements Page, IDBAccessObject {
                                                }
                                        }
                                );
+                       } else {
+                               // Try to avoid a second parse if {{REVISIONID}} is used
+                               $edit->popts->setSpeculativeRevIdCallback( function () {
+                                       return 1 + (int)wfGetDB( DB_MASTER )->selectField(
+                                               'revision',
+                                               'MAX(rev_id)',
+                                               [],
+                                               __METHOD__
+                                       );
+                               } );
                        }
                        $edit->output = $edit->pstContent
                                ? $edit->pstContent->getParserOutput( $this->mTitle, $revid, $edit->popts )
@@ -2168,14 +2180,20 @@ class WikiPage implements Page, IDBAccessObject {
                ];
                $content = $revision->getContent();
 
+               $logger = LoggerFactory::getInstance( 'SaveParse' );
+
                // See if the parser output before $revision was inserted is still valid
                $editInfo = false;
                if ( !$this->mPreparedEdit ) {
-                       wfDebug( __METHOD__ . ": No prepared edit...\n" );
+                       $logger->debug( __METHOD__ . ": No prepared edit...\n" );
                } elseif ( $this->mPreparedEdit->output->getFlag( 'vary-revision' ) ) {
-                       wfDebug( __METHOD__ . ": Prepared edit has vary-revision...\n" );
+                       $logger->info( __METHOD__ . ": Prepared edit has vary-revision...\n" );
+               } elseif ( $this->mPreparedEdit->output->getFlag( 'vary-revision-id' )
+                       && $this->mPreparedEdit->output->getSpeculativeRevIdUsed() !== $revision->getId()
+               ) {
+                       $logger->info( __METHOD__ . ": Prepared edit has vary-revision-id with wrong ID...\n" );
                } elseif ( $this->mPreparedEdit->output->getFlag( 'vary-user' ) && !$options['changed'] ) {
-                       wfDebug( __METHOD__ . ": Prepared edit has vary-user and is null...\n" );
+                       $logger->info( __METHOD__ . ": Prepared edit has vary-user and is null...\n" );
                } else {
                        wfDebug( __METHOD__ . ": Using prepared edit...\n" );
                        $editInfo = $this->mPreparedEdit;
index 26b4bd9..bd2f131 100644 (file)
@@ -2600,9 +2600,13 @@ class Parser {
                        case 'revisionid':
                                # Let the edit saving system know we should parse the page
                                # *after* a revision ID has been assigned.
-                               $this->mOutput->setFlag( 'vary-revision' );
-                               wfDebug( __METHOD__ . ": {{REVISIONID}} used, setting vary-revision...\n" );
+                               $this->mOutput->setFlag( 'vary-revision-id' );
+                               wfDebug( __METHOD__ . ": {{REVISIONID}} used, setting vary-revision-id...\n" );
                                $value = $this->mRevisionId;
+                               if ( !$value && $this->mOptions->getSpeculativeRevIdCallback() ) {
+                                       $value = call_user_func( $this->mOptions->getSpeculativeRevIdCallback() );
+                                       $this->mOutput->setSpeculativeRevIdUsed( $value );
+                               }
                                break;
                        case 'revisionday':
                                # Let the edit saving system know we should parse the page
index 91cd0f4..891c4dd 100644 (file)
@@ -117,17 +117,22 @@ class ParserOptions {
        private $mRemoveComments = true;
 
        /**
-        * Callback for current revision fetching. Used as first argument to call_user_func().
+        * @var callable Callback for current revision fetching; first argument to call_user_func().
         */
        private $mCurrentRevisionCallback =
                [ 'Parser', 'statelessFetchRevision' ];
 
        /**
-        * Callback for template fetching. Used as first argument to call_user_func().
+        * @var callable Callback for template fetching; first argument to call_user_func().
         */
        private $mTemplateCallback =
                [ 'Parser', 'statelessFetchTemplate' ];
 
+       /**
+        * @var callable|null Callback to generate a guess for {{REVISIONID}}
+        */
+       private $mSpeculativeRevIdCallback;
+
        /**
         * Enable limit report in an HTML comment on output
         */
@@ -302,6 +307,11 @@ class ParserOptions {
                return $this->mTemplateCallback;
        }
 
+       /** @since 1.28 */
+       public function getSpeculativeRevIdCallback() {
+               return $this->mSpeculativeRevIdCallback;
+       }
+
        public function getEnableLimitReport() {
                return $this->mEnableLimitReport;
        }
@@ -483,6 +493,11 @@ class ParserOptions {
                return wfSetVar( $this->mCurrentRevisionCallback, $x );
        }
 
+       /** @since 1.28 */
+       public function setSpeculativeRevIdCallback( $x ) {
+               return wfSetVar( $this->mSpeculativeRevIdCallback, $x );
+       }
+
        public function setTemplateCallback( $x ) {
                return wfSetVar( $this->mTemplateCallback, $x );
        }
index 6c7ad4e..3462d10 100644 (file)
@@ -208,6 +208,9 @@ class ParserOutput extends CacheTime {
         */
        private $mFlags = [];
 
+       /** @var integer|null Assumed rev ID for {{REVISIONID}} if no revision is set */
+       private $mSpeculativeRevId;
+
        const EDITSECTION_REGEX =
                '#<(?:mw:)?editsection page="(.*?)" section="(.*?)"(?:/>|>(.*?)(</(?:mw:)?editsection>))#';
 
@@ -272,6 +275,19 @@ class ParserOutput extends CacheTime {
                return $text;
        }
 
+       /**
+        * @param integer $id
+        * @since 1.28
+        */
+       public function setSpeculativeRevIdUsed( $id ) {
+               $this->mSpeculativeRevId = $id;
+       }
+
+       /** @since 1.28 */
+       public function getSpeculativeRevIdUsed() {
+               return $this->mSpeculativeRevId;
+       }
+
        public function &getLanguageLinks() {
                return $this->mLanguageLinks;
        }