r75334 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r75333‎ | r75334 | r75335 >
Date:19:16, 24 October 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
* If a action=parse request provides an oldid that is actually the current revision id, try the parser cache, and save it to it if necessary
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/api/ApiParse.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiParse.php
@@ -89,15 +89,36 @@
9090 $this->dieUsage( "You don't have permission to view deleted revisions", 'permissiondenied' );
9191 }
9292
93 - $text = $rev->getText( Revision::FOR_THIS_USER );
9493 $titleObj = $rev->getTitle();
 94+
9595 $wgTitle = $titleObj;
9696
97 - if ( $this->section !== false ) {
98 - $text = $this->getSectionText( $text, 'r' . $rev );
 97+ //If for some reason the "oldid" is actually the current revision, it may be cached
 98+ if ( $titleObj->getLatestRevID() === $oldid ) {
 99+ $p_result = false;
 100+ $pcache = ParserCache::singleton();
 101+ if ( $wgEnableParserCache ) {
 102+ $p_result = $pcache->get( $titleObj, $popts );
 103+ }
 104+ if ( !$p_result ) {
 105+ $text = $rev->getText( Revision::FOR_THIS_USER );
 106+ $p_result = $wgParser->parse( $text, $titleObj, $popts );
 107+
 108+ if ( $wgEnableParserCache ) {
 109+ $pcache->save( $p_result, $titleObj, $popts );
 110+ }
 111+ }
 112+ } else {
 113+ $text = $rev->getText( Revision::FOR_THIS_USER );
 114+
 115+ $wgTitle = $titleObj;
 116+
 117+ if ( $this->section !== false ) {
 118+ $text = $this->getSectionText( $text, 'r' . $rev );
 119+ }
 120+
 121+ $p_result = $wgParser->parse( $text, $titleObj, $popts );
99122 }
100 -
101 - $p_result = $wgParser->parse( $text, $titleObj, $popts );
102123 } else {
103124 if ( !is_null ( $pageid ) ) {
104125 $titleObj = Title::newFromID( $pageid );
Index: trunk/phase3/RELEASE-NOTES
@@ -444,6 +444,8 @@
445445 * Added iiprop=parsedcomment to prop=imageinfo, similar to prop=revisions
446446 * Added rvparse to parse revisions. For performance reasons if this option is
447447 used, rvlimit is enforced to 1.
 448+* If a action=parse request provides an oldid that is actually the current revision
 449+ id, try the parser cache, and save it to it if necessary
448450
449451 === Languages updated in 1.17 ===
450452

Follow-up revisions

RevisionCommit summaryAuthorDate
r75853Add bug 25748 for the work in r75334 to RELEASE-NOTESreedy13:55, 2 November 2010
r79451Followup r75334. Change to int == string, rather than int === string (not goi...reedy22:35, 1 January 2011
r79454Per Platonides on CR r75334, use $article->getParserOutput() to get latest re...reedy22:49, 1 January 2011

Comments

#Comment by Bryan (talk | contribs)   20:05, 16 November 2010

I don't really like how this module has multiple copies of the whole get-from-cache -> parse -> save-to-cache routine. Can't that be factorized out to a functoin?

#Comment by Reedy (talk | contribs)   22:10, 1 January 2011
#Comment by Duplicatebug (talk | contribs)   21:05, 1 January 2011
  • Looks like $oldid is (always?) a string, but $titleObj->getLatestRevID() gives (always?) a integer, so there is never a lookup in the ParserCache.
  • The ParserCache need a Article, not a Title.
  • oldid with section gives errors.
  • when oldid = latestRevid the section param is not supported.
#Comment by Reedy (talk | contribs)   23:02, 1 January 2011

r79455 fixes the last issue

#Comment by Platonides (talk | contribs)   22:14, 1 January 2011

What about using Article::getParserOutput() ?

I would prefer that ApiQueryRevisions.php / ApiParse.php didn't have to mess directly with the ParserCache,

#Comment by Reedy (talk | contribs)   22:19, 1 January 2011

That would look reasonably sane...

#Comment by Reedy (talk | contribs)   22:40, 1 January 2011

Only thing would be.. The method, as is, doesn't put stuff back into the parser cache... Probably worth doing, no?

#Comment by Platonides (talk | contribs)   22:43, 1 January 2011

Article::getParserOutput() does put things into the parsercache when rendering, although it's done a couple functions down: in Article::getOutputFromWikitext()

Status & tagging log