From 005b4d6fffd839e06c33133c60e21d092a21e823 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 18 Jun 2016 22:30:21 -0700 Subject: [PATCH] Try to predict the rev_id when preparing edits 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 | 5 ++++- includes/page/WikiPage.php | 24 +++++++++++++++++++++--- includes/parser/Parser.php | 8 ++++++-- includes/parser/ParserOptions.php | 19 +++++++++++++++++-- includes/parser/ParserOutput.php | 16 ++++++++++++++++ 5 files changed, 64 insertions(+), 8 deletions(-) diff --git a/includes/api/ApiStashEdit.php b/includes/api/ApiStashEdit.php index c8a330a20e..446a98cf83 100644 --- a/includes/api/ApiStashEdit.php +++ b/includes/api/ApiStashEdit.php @@ -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; diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index a416d5643d..b06b51967a 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -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; diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php index 26b4bd9242..bd2f13181e 100644 --- a/includes/parser/Parser.php +++ b/includes/parser/Parser.php @@ -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 diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php index 91cd0f4126..891c4dde6c 100644 --- a/includes/parser/ParserOptions.php +++ b/includes/parser/ParserOptions.php @@ -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 ); } diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index 6c7ad4eb4c..3462d10c7d 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -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="(.*?)"(?:/>|>(.*?)())#'; @@ -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; } -- 2.20.1