Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ParserOutputAccess: Fork primary and secondary caches for parsoid

Browse source 
Uses flag to detect which cache instance to use based on ParserOptions
and sets the primary and secondary caches accordingly.  This ensures
that the ParserCacheMetadata cache used by the ParserCache is also
appropriately forked for Parsoid, as Parsoid may consult different
options in the ParserCache than core does.

A follow up patch will attempt to refactor this to be less
parsoid-specific.

Bug: T327769
Bug: T330677
Co-authored-by: Alangi Derick <alangiderick@gmail.com>
Change-Id: Id580b97ad9a0b90bbe56d4de3c2f999274fe329b
This commit is contained in:
C. Scott Ananian 2023-03-23 10:48:49 -04:00
parent cfd9c516e1
commit 2caf69797c
4 changed files with 173 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
includes/ServiceWiring.php
View file

@ -1355,8 +1355,7 @@ return [
'ParserOutputAccess' => static function ( MediaWikiServices $services ): ParserOutputAccess {
return new ParserOutputAccess(
$services->getParserCache(),
$services->getParserCacheFactory()->getRevisionOutputCache( 'rcache' ),
$services->getParserCacheFactory(),
$services->getRevisionLookup(),
$services->getRevisionRenderer(),
$services->getStatsdDataFactory(),

72
includes/page/ParserOutputAccess.php
View file

@ -22,6 +22,7 @@ namespace MediaWiki\Page;
use IBufferingStatsdDataFactory;
use InvalidArgumentException;
use MediaWiki\Logger\Spi as LoggerSpi;
use MediaWiki\Parser\ParserCacheFactory;
use MediaWiki\Parser\RevisionOutputCache;
use MediaWiki\Revision\RevisionLookup;
use MediaWiki\Revision\RevisionRecord;
@ -84,13 +85,7 @@ class ParserOutputAccess {
/** @var string Use secondary cache */
private const CACHE_SECONDARY = 'secondary';
/** @var ParserCache */
private $primaryCache;
/**
* @var RevisionOutputCache
*/
private $secondaryCache;
private ParserCacheFactory $parserCacheFactory;
/**
* In cases that an extension tries to get the same ParserOutput of
@ -121,8 +116,7 @@ class ParserOutputAccess {
private $titleFormatter;
/**
* @param ParserCache $primaryCache
* @param RevisionOutputCache $secondaryCache
* @param ParserCacheFactory $parserCacheFactory
* @param RevisionLookup $revisionLookup
* @param RevisionRenderer $revisionRenderer
* @param IBufferingStatsdDataFactory $statsDataFactory
@ -132,8 +126,7 @@ class ParserOutputAccess {
* @param TitleFormatter $titleFormatter
*/
public function __construct(
ParserCache $primaryCache,
RevisionOutputCache $secondaryCache,
ParserCacheFactory $parserCacheFactory,
RevisionLookup $revisionLookup,
RevisionRenderer $revisionRenderer,
IBufferingStatsdDataFactory $statsDataFactory,
@ -142,8 +135,7 @@ class ParserOutputAccess {
WikiPageFactory $wikiPageFactory,
TitleFormatter $titleFormatter
) {
$this->primaryCache = $primaryCache;
$this->secondaryCache = $secondaryCache;
$this->parserCacheFactory = $parserCacheFactory;
$this->revisionLookup = $revisionLookup;
$this->revisionRenderer = $revisionRenderer;
$this->statsDataFactory = $statsDataFactory;
@ -210,15 +202,17 @@ class ParserOutputAccess {
): ?ParserOutput {
$isOld = $revision && $revision->getId() !== $page->getLatest();
$useCache = $this->shouldUseCache( $page, $revision );
$classCacheKey = $this->primaryCache->makeParserOutputKey( $page, $parserOptions );
$primaryCache = $this->getPrimaryCache( $parserOptions );
$classCacheKey = $primaryCache->makeParserOutputKey( $page, $parserOptions );
if ( $useCache === self::CACHE_PRIMARY ) {
if ( isset( $this->localCache[$classCacheKey] ) && !$isOld ) {
return $this->localCache[$classCacheKey];
}
$output = $this->primaryCache->get( $page, $parserOptions );
$output = $primaryCache->get( $page, $parserOptions );
} elseif ( $useCache === self::CACHE_SECONDARY && $revision ) {
$output = $this->secondaryCache->get( $revision, $parserOptions );
$secondaryCache = $this->getSecondaryCache( $parserOptions );
$output = $secondaryCache->get( $revision, $parserOptions );
} else {
$output = null;
}
@ -301,7 +295,8 @@ class ParserOutputAccess {
Assert::postcondition( $output || !$status->isOK(), 'Worker returned invalid status' );
if ( $output && !$isOld ) {
$classCacheKey = $this->primaryCache->makeParserOutputKey( $page, $parserOptions );
$primaryCache = $this->getPrimaryCache( $parserOptions );
$classCacheKey = $primaryCache->makeParserOutputKey( $page, $parserOptions );
$this->localCache[$classCacheKey] = $output;
}
@ -375,12 +370,13 @@ class ParserOutputAccess {
int $options
): PoolCounterWork {
$useCache = $this->shouldUseCache( $page, $revision );
$primaryCache = $this->getPrimaryCache( $parserOptions );
switch ( $useCache ) {
case self::CACHE_PRIMARY:
$this->statsDataFactory->increment( 'ParserOutputAccess.PoolWork.Current' );
$parserCacheMetadata = $this->primaryCache->getMetadata( $page );
$cacheKey = $this->primaryCache->makeParserOutputKey( $page, $parserOptions,
$parserCacheMetadata = $primaryCache->getMetadata( $page );
$cacheKey = $primaryCache->makeParserOutputKey( $page, $parserOptions,
$parserCacheMetadata ? $parserCacheMetadata->getUsedOptions() : null
);
@ -392,7 +388,7 @@ class ParserOutputAccess {
$revision,
$parserOptions,
$this->revisionRenderer,
$this->primaryCache,
$primaryCache,
$this->lbFactory,
$this->loggerSpi,
$this->wikiPageFactory,
@ -401,10 +397,11 @@ class ParserOutputAccess {
case self::CACHE_SECONDARY:
$this->statsDataFactory->increment( 'ParserOutputAccess.PoolWork.Old' );
$workKey = $this->secondaryCache->makeParserOutputKey( $revision, $parserOptions );
$secondaryCache = $this->getSecondaryCache( $parserOptions );
$workKey = $secondaryCache->makeParserOutputKey( $revision, $parserOptions );
return new PoolWorkArticleViewOld(
$workKey,
$this->secondaryCache,
$secondaryCache,
$revision,
$parserOptions,
$this->revisionRenderer,
@ -413,7 +410,8 @@ class ParserOutputAccess {
default:
$this->statsDataFactory->increment( 'ParserOutputAccess.PoolWork.Uncached' );
$workKey = $this->secondaryCache->makeParserOutputKeyOptionalRevId( $revision, $parserOptions );
$secondaryCache = $this->getSecondaryCache( $parserOptions );
$workKey = $secondaryCache->makeParserOutputKeyOptionalRevId( $revision, $parserOptions );
return new PoolWorkArticleView(
$workKey,
$revision,
@ -426,4 +424,32 @@ class ParserOutputAccess {
// unreachable
}
private function getPrimaryCache( ParserOptions $pOpts ): ParserCache {
if ( $pOpts->getUseParsoid() ) {
// T331148: This is different from
// ParsoidOutputAccess::PARSOID_PARSER_CACHE_NAME; will be
// renamed once the contents cached on the read-views and
// the REST path are identical.
return $this->parserCacheFactory->getParserCache(
'parsoid-' . ParserCacheFactory::DEFAULT_NAME
);
}
return $this->parserCacheFactory->getParserCache(
ParserCacheFactory::DEFAULT_NAME
);
}
private function getSecondaryCache( ParserOptions $pOpts ): RevisionOutputCache {
if ( $pOpts->getUseParsoid() ) {
return $this->parserCacheFactory->getRevisionOutputCache(
'parsoid-' . ParserCacheFactory::DEFAULT_RCACHE_NAME
);
}
return $this->parserCacheFactory->getRevisionOutputCache(
ParserCacheFactory::DEFAULT_RCACHE_NAME
);
}
}

3
includes/parser/ParserCacheFactory.php
View file

@ -43,6 +43,9 @@ class ParserCacheFactory {
/** @var string name of ParserCache for the default parser */
public const DEFAULT_NAME = 'pcache';
/** @var string name of RevisionOutputCache for the default parser */
public const DEFAULT_RCACHE_NAME = 'rcache';
/** @var BagOStuff */
private $parserCacheBackend;

122
tests/phpunit/includes/page/ParserOutputAccessTest.php
View file

@ -3,6 +3,7 @@ use MediaWiki\Json\JsonCodec;
use MediaWiki\Logger\Spi as LoggerSpi;
use MediaWiki\MainConfigNames;
use MediaWiki\Page\ParserOutputAccess;
use MediaWiki\Parser\ParserCacheFactory;
use MediaWiki\Parser\RevisionOutputCache;
use MediaWiki\PoolCounter\PoolCounterFactory;
use MediaWiki\Revision\MutableRevisionRecord;
@ -130,6 +131,26 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
$revisionOutputCache = $this->getRevisionOutputCache( new HashBagOStuff() );
}
$parserCacheFactory = $this->createMock( ParserCacheFactory::class );
$parserCacheFactory->method( 'getParserCache' )->willReturn( $parserCache );
$parserCacheFactory->method( 'getRevisionOutputCache' )->willReturn( $revisionOutputCache );
return $this->getParserOutputAccessWithCacheFactory(
$parserCacheFactory,
$maxRenderCalls
);
}
/**
* @param ParserCacheFactory $parserCacheFactory
* @param int|bool $maxRenderCalls
*
* @return ParserOutputAccess
* @throws Exception
*/
private function getParserOutputAccessWithCacheFactory(
$parserCacheFactory,
$maxRenderCalls = false
) {
$revRenderer = $this->getServiceContainer()->getRevisionRenderer();
if ( $maxRenderCalls ) {
@ -143,8 +164,7 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
}
return new ParserOutputAccess(
$parserCache,
$revisionOutputCache,
$parserCacheFactory,
$this->getServiceContainer()->getRevisionLookup(),
$revRenderer,
new NullStatsdDataFactory(),
@ -735,4 +755,102 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
$this->assertContainsHtml( 'World', $result );
}
public function testParsoidCacheSplit() {
$parserCacheFactory = $this->createMock( ParserCacheFactory::class );
$revisionOutputCache = $this->getRevisionOutputCache( new HashBagOStuff() );
$caches = [
$this->getParserCache( new HashBagOStuff() ),
$this->getParserCache( new HashBagOStuff() ),
];
$calls = [];
$parserCacheFactory
->method( 'getParserCache' )
->willReturnCallback( static function ( $cacheName ) use ( &$calls, $caches ) {
static $cacheList = [];
$calls[] = $cacheName;
$which = array_search( $cacheName, $cacheList );
if ( $which === false ) {
$which = count( $cacheList );
$cacheList[] = $cacheName;
}
return $caches[$which];
} );
$parserCacheFactory
->method( 'getRevisionOutputCache' )
->willReturn( $revisionOutputCache );
$access = $this->getParserOutputAccessWithCacheFactory( $parserCacheFactory );
$parserOptions0 = $this->getParserOptions();
$page = $this->getNonexistingTestPage( __METHOD__ );
$output = $access->getCachedParserOutput( $page, $parserOptions0 );
$this->assertNull( $output );
// $calls[0] will remember what cache name we used.
$this->assertCount( 1, $calls );
$parserOptions1 = $this->getParserOptions();
$parserOptions1->setUseParsoid();
$output = $access->getCachedParserOutput( $page, $parserOptions1 );
$this->assertNull( $output );
$this->assertCount( 2, $calls );
// Check that we used a different cache name this time.
$this->assertNotEquals( $calls[1], $calls[0], "Should use different caches" );
// Try this again, with actual content.
$calls = [];
$this->editPage( $page, "__NOTOC__" );
$status0 = $access->getParserOutput( $page, $parserOptions0 );
$this->assertContainsHtml( '<div class="mw-parser-output"></div>', $status0 );
$status1 = $access->getParserOutput( $page, $parserOptions1 );
$this->assertContainsHtml( '<meta property="mw:PageProp/notoc"', $status1 );
$this->assertNotSameHtml( $status0, $status1 );
}
public function testParsoidRevisionCacheSplit() {
$parserCacheFactory = $this->createMock( ParserCacheFactory::class );
$parserCache = $this->getParserCache( new HashBagOStuff() );
$caches = [
$this->getRevisionOutputCache( new HashBagOStuff() ),
$this->getRevisionOutputCache( new HashBagOStuff() ),
];
$calls = [];
$parserCacheFactory
->method( 'getParserCache' )
->willReturn( $parserCache );
$parserCacheFactory
->method( 'getRevisionOutputCache' )
->willReturnCallback( static function ( $cacheName ) use ( &$calls, $caches ) {
static $cacheList = [];
$calls[] = $cacheName;
$which = array_search( $cacheName, $cacheList );
if ( $which === false ) {
$which = count( $cacheList );
$cacheList[] = $cacheName;
}
return $caches[$which];
} );
$access = $this->getParserOutputAccessWithCacheFactory( $parserCacheFactory );
$page = $this->getNonexistingTestPage( __METHOD__ );
$firstRev = $this->editPage( $page, 'First __NOTOC__' )->getNewRevision();
$secondRev = $this->editPage( $page, 'Second __NOTOC__' )->getNewRevision();
$parserOptions0 = $this->getParserOptions();
$status = $access->getParserOutput( $page, $parserOptions0, $firstRev );
$this->assertContainsHtml( 'First', $status );
// Check that we used the "not parsoid" revision cache
$this->assertTrue( count( $calls ) > 0 );
$notParsoid = $calls[0];
$this->assertEquals( array_fill( 0, count( $calls ), $notParsoid ), $calls );
$calls = [];
$parserOptions1 = $this->getParserOptions();
$parserOptions1->setUseParsoid();
$status = $access->getParserOutput( $page, $parserOptions1, $firstRev );
$this->assertContainsHtml( 'First', $status );
$this->assertContainsHtml( '<meta property="mw:PageProp/notoc"', $status );
$this->assertTrue( count( $calls ) > 0 );
$parsoid = $calls[0];
$this->assertNotEquals( $notParsoid, $parsoid, "Should use different caches" );
$this->assertEquals( array_fill( 0, count( $calls ), $parsoid ), $calls );
}
}