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

diff --git a/includes/MovePage.php b/includes/MovePage.php
index 8ee562a..a8f6f8e 100644 (file)
--- 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 5e1e8c6..213572b 100644 (file)
--- 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 3dc41fb..54bc6f6 100644 (file)
--- 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 7cbec70..9c3b41c 100644 (file)
--- 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 b12d235..a332528 100644 (file)
--- 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 ),