From 0852a000a5555a85ac5ec7759b73dc655771a3d2 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 2 Sep 2016 01:28:23 -0700 Subject: [PATCH] Add caching to ResourceLoaderWikiModule::preloadTitleInfo() This is one of the top three DB queries showing up in xenon reverse flamegraph profiling. It works via a per-wiki check key that is bumped whenever someone changes a .js or .css page on that wiki. Change-Id: I73f419558864ba3403b4601a098f6aaf84a3e7c1 --- includes/MovePage.php | 2 +- includes/Title.php | 11 +-- includes/page/WikiPage.php | 15 ++++- .../ResourceLoaderWikiModule.php | 67 ++++++++++++++++--- .../ResourceLoaderWikiModuleTest.php | 6 ++ 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/includes/MovePage.php b/includes/MovePage.php index 8ee562a309..a8f6f8e2e6 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -539,8 +539,8 @@ class MovePage { __METHOD__ ); - // clean up the old title before reset article id - bug 45348 if ( !$redirectContent ) { + // Clean up the old title *before* reset article id - bug 45348 WikiPage::onArticleDelete( $this->oldTitle ); } diff --git a/includes/Title.php b/includes/Title.php index 5e1e8c6115..213572b95b 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -4391,16 +4391,19 @@ class Title implements LinkTarget { public function invalidateCache( $purgeTime = null ) { if ( wfReadOnly() ) { return false; - } - - if ( $this->mArticleID === 0 ) { + } elseif ( $this->mArticleID === 0 ) { return true; // avoid gap locking if we know it's not there } + $dbw = wfGetDB( DB_MASTER ); + $dbw->onTransactionPreCommitOrIdle( function () { + ResourceLoaderWikiModule::invalidateModuleCache( $this, null, null, wfWikiID() ); + } ); + $conds = $this->pageCond(); DeferredUpdates::addUpdate( new AutoCommitUpdate( - wfGetDB( DB_MASTER ), + $dbw, __METHOD__, function ( IDatabase $dbw, $fname ) use ( $conds, $purgeTime ) { $dbTimestamp = $dbw->timestamp( $purgeTime ?: time() ); diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 3dc41fba58..54bc6f6260 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -2399,6 +2399,10 @@ class WikiPage implements Page, IDBAccessObject { } elseif ( $options['changed'] ) { // bug 50785 self::onArticleEdit( $this->mTitle, $revision ); } + + ResourceLoaderWikiModule::invalidateModuleCache( + $this->mTitle, $options['oldrevision'], $revision, wfWikiID() + ); } /** @@ -2912,6 +2916,7 @@ class WikiPage implements Page, IDBAccessObject { // unless they actually try to catch exceptions (which is rare). // we need to remember the old content so we can use it to generate all deletion updates. + $revision = $this->getRevision(); try { $content = $this->getContent( Revision::RAW ); } catch ( Exception $ex ) { @@ -3011,7 +3016,7 @@ class WikiPage implements Page, IDBAccessObject { $dbw->endAtomic( __METHOD__ ); - $this->doDeleteUpdates( $id, $content ); + $this->doDeleteUpdates( $id, $content, $revision ); Hooks::run( 'ArticleDeleteComplete', [ &$wikiPageBeforeDelete, @@ -3058,11 +3063,12 @@ class WikiPage implements Page, IDBAccessObject { * Do some database updates after deletion * * @param int $id The page_id value of the page being deleted - * @param Content $content Optional page content to be used when determining + * @param Content|null $content Optional page content to be used when determining * the required updates. This may be needed because $this->getContent() * may already return null when the page proper was deleted. + * @param Revision|null $revision The latest page revision */ - public function doDeleteUpdates( $id, Content $content = null ) { + public function doDeleteUpdates( $id, Content $content = null, Revision $revision = null ) { try { $countable = $this->isCountable(); } catch ( Exception $ex ) { @@ -3090,6 +3096,9 @@ class WikiPage implements Page, IDBAccessObject { // Clear caches WikiPage::onArticleDelete( $this->mTitle ); + ResourceLoaderWikiModule::invalidateModuleCache( + $this->mTitle, $revision, null, wfWikiID() + ); // Reset this object and the Title object $this->loadFromRow( false, self::READ_LATEST ); diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php index 7cbec706e4..9c3b41cf90 100644 --- a/includes/resourceloader/ResourceLoaderWikiModule.php +++ b/includes/resourceloader/ResourceLoaderWikiModule.php @@ -26,7 +26,8 @@ * Abstraction for ResourceLoader modules which pull from wiki pages * * This can only be used for wiki pages in the MediaWiki and User namespaces, - * because of its dependence on the functionality of Title::isCssJsSubpage. + * because of its dependence on the functionality of Title::isCssJsSubpage + * and Title::isCssOrJsPage(). * * This module supports being used as a placeholder for a module on a remote wiki. * To do so, getDB() must be overloaded to return a foreign database object that @@ -143,7 +144,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule { } /** - * @param string $title + * @param string $titleText * @return null|string */ protected function getContent( $titleText ) { @@ -336,7 +337,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule { * @since 1.28 * @param ResourceLoaderContext $context * @param IDatabase $db - * @param string[] $modules + * @param string[] $moduleNames */ public static function preloadTitleInfo( ResourceLoaderContext $context, IDatabase $db, array $moduleNames @@ -345,6 +346,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule { // getDB() can be overridden to point to a foreign database. // For now, only preload local. In the future, we could preload by wikiID. $allPages = []; + /** @var ResourceLoaderWikiModule[] $wikiModules */ $wikiModules = []; foreach ( $moduleNames as $name ) { $module = $rl->getModule( $name ); @@ -357,9 +359,28 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule { } } } - $allInfo = static::fetchTitleInfo( $db, array_keys( $allPages ), __METHOD__ ); - foreach ( $wikiModules as $module ) { - $pages = $module->getPages( $context ); + + $allPageNames = array_keys( $allPages ); + sort( $allPageNames ); + $hash = sha1( implode( '|', $allPageNames ) ); + + // Avoid Zend bug where "static::" does not apply LSB in the closure + $func = [ static::class, 'fetchTitleInfo' ]; + + $cache = ObjectCache::getMainWANInstance(); + $allInfo = $cache->getWithSetCallback( + $cache->makeGlobalKey( 'resourceloader', 'titleinfo', $db->getWikiID(), $hash ), + $cache::TTL_HOUR, + function ( $curValue, &$ttl, array &$setOpts ) use ( $func, $allPageNames, $db ) { + $setOpts += Database::getCacheSetOptions( $db ); + + return call_user_func( $func, $db, $allPageNames, __METHOD__ ); + }, + [ 'checkKeys' => [ $cache->makeGlobalKey( 'resourceloader', 'titleinfo', $db->getWikiID() ) ] ] + ); + + foreach ( $wikiModules as $wikiModule ) { + $pages = $wikiModule->getPages( $context ); // Before we intersect, map the names to canonical form (T145673). $intersect = []; foreach ( $pages as $page => $unused ) { @@ -375,13 +396,41 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule { } } $info = array_intersect_key( $allInfo, $intersect ); - $pageNames = array_keys( $pages ); sort( $pageNames ); $key = implode( '|', $pageNames ); - $module->setTitleInfo( $key, $info ); + $wikiModule->setTitleInfo( $key, $info ); + } + } + + /** + * Clear the preloadTitleInfo() cache for all wiki modules on this wiki on + * page change if it was a JS or CSS page + * + * @param Title $title + * @param Revision|null $old Prior page revision + * @param Revision|null $new New page revision + * @param string $wikiId + * @since 1.28 + */ + public static function invalidateModuleCache( + Title $title, Revision $old = null, Revision $new = null, $wikiId + ) { + static $formats = [ CONTENT_FORMAT_CSS, CONTENT_FORMAT_JAVASCRIPT ]; + + if ( $old && in_array( $old->getContentFormat(), $formats ) ) { + $purge = true; + } elseif ( $new && in_array( $new->getContentFormat(), $formats ) ) { + $purge = true; + } else { + $purge = ( $title->isCssOrJsPage() || $title->isCssJsSubpage() ); + } + + if ( $purge ) { + $cache = ObjectCache::getMainWANInstance(); + $key = $cache->makeGlobalKey( 'resourceloader', 'titleinfo', $wikiId ); + $cache->touchCheckKey( $key ); } - return $allInfo; } /** diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php index b12d235f26..a332528ffb 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php @@ -199,6 +199,12 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase { $rl->register( 'testmodule', $module ); $context = new ResourceLoaderContext( $rl, new FauxRequest() ); + TestResourceLoaderWikiModule::invalidateModuleCache( + Title::newFromText( 'MediaWiki:Common.css' ), + null, + null, + wfWikiID() + ); TestResourceLoaderWikiModule::preloadTitleInfo( $context, wfGetDB( DB_REPLICA ), -- 2.20.1