(bug 27302) Avoid unnecessary requests for user and site modules if the relevant...

Done by adding isKnownEmpty() to ResourceLoaderModule and overriding it to check for page existence in ResourceLoaderWikiModule. Needed to rearrange some code in OutputPage::makeResourceLoaderLink() to have the emptiness check and dropping of modules work properly. Also factored the page_touched check in ResourceLoaderWikiModule::getModifiedTime() out to a separate method (getTitleMtimes()) and moved in-object caching there as well, so getModifiedTime() and isKnownEmpty() share code and caching for their timestamp/existence checks.

This does not account for the case where e.g. a user has user CSS but no user JS: I had implemented this by checking for $context->getOnly() in getTitleMtimes(), but then realized it's not safe to do this in a function called by getModifiedTime(): it causes the timestamp list in the startup module to only take scripts in account for wiki modules, because the startup module has &only=scripts set

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 1ee62c0..ad82b5a 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2462,14 +2462,29 @@ class OutputPage {
 
                $links = '';
                foreach ( $groups as $group => $modules ) {
-                       $query['modules'] = implode( '|', array_keys( $modules ) );
                        // Special handling for user-specific groups
                        if ( ( $group === 'user' || $group === 'private' ) && $wgUser->isLoggedIn() ) {
                                $query['user'] = $wgUser->getName();
                        }
+                       
+                       // Create a fake request based on the one we are about to make so modules return
+                       // correct timestamp and emptiness data
+                       $context = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
+                       // Drop modules that know they're empty
+                       foreach ( $modules as $key => $module ) {
+                               if ( $module->isKnownEmpty( $context ) ) {
+                                       unset( $modules[$key] );
+                               }
+                       }
+                       // If there are no modules left, skip this group
+                       if ( $modules === array() ) {
+                               continue;
+                       }
+                       
+                       $query['modules'] = implode( '|', array_keys( $modules ) );
+                       
                        // Support inlining of private modules if configured as such
                        if ( $group === 'private' && $wgResourceLoaderInlinePrivateModules ) {
-                               $context = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
                                if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
                                        $links .= Html::inlineStyle(
                                                $resourceLoader->makeModuleResponse( $context, $modules )
@@ -2487,9 +2502,6 @@ class OutputPage {
                        // on-wiki like site or user pages, or user preferences; we need to find the highest
                        // timestamp of these user-changable modules so we can ensure cache misses on change
                        if ( $group === 'user' || $group === 'site' ) {
-                               // Create a fake request based on the one we are about to make so modules return
-                               // correct times
-                               $context = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
                                // Get the maximum timestamp
                                $timestamp = 1;
                                foreach ( $modules as $module ) {

diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index a8bb2c0..fee5443 100644 (file)
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -279,4 +279,17 @@ abstract class ResourceLoaderModule {
                // 0 would mean now
                return 1;
        }
+       
+       /**
+        * Check whether this module is known to be empty. If a child class
+        * has an easy and cheap way to determine that this module is
+        * definitely going to be empty, it should override this method to
+        * return true in that case. Callers may optimize the request for this
+        * module away if this function returns true.
+        * @param $context ResourceLoaderContext: Context object
+        * @return Boolean
+        */
+       public function isKnownEmpty( ResourceLoaderContext $context ) {
+               return false;
+       }
 }

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 266fb1c..40b1186 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -36,8 +36,8 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
        # Origin is user-supplied code
        protected $origin = self::ORIGIN_USER_SITEWIDE;
        
-       // In-object cache for modified time
-       protected $modifiedTime = array();
+       // In-object cache for title mtimes
+       protected $titleMtimes = array();
        
        /* Abstract Protected Methods */
        
@@ -120,32 +120,18 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
        }
 
        public function getModifiedTime( ResourceLoaderContext $context ) {
-               $hash = $context->getHash();
-               if ( isset( $this->modifiedTime[$hash] ) ) {
-                       return $this->modifiedTime[$hash];
-               }
-
-               $batch = new LinkBatch;
-               foreach ( $this->getPages( $context ) as $titleText => $options ) {
-                       $batch->addObj( Title::newFromText( $titleText ) );
-               }
-
                $modifiedTime = 1; // wfTimestamp() interprets 0 as "now"
-               if ( !$batch->isEmpty() ) {
-                       $dbr = wfGetDB( DB_SLAVE );
-                       $latest = $dbr->selectField( 'page', 'MAX(page_touched)',
-                               $batch->constructSet( 'page', $dbr ),
-                               __METHOD__ );
-
-                       if ( $latest ) {
-                               $modifiedTime = wfTimestamp( TS_UNIX, $latest );
-                       }
+               $mtimes = $this->getTitleMtimes( $context );
+               if ( count( $mtimes ) ) {
+                       $modifiedTime = max( $modifiedTime, max( $mtimes ) );
                }
-
-               $this->modifiedTime[$hash] = $modifiedTime;
                return $modifiedTime;
        }
-
+       
+       public function isKnownEmpty( ResourceLoaderContext $context ) {
+               return count( $this->getTitleMtimes( $context ) ) == 0;
+       }
+       
        /**
         * @param $context ResourceLoaderContext
         * @return bool
@@ -155,4 +141,38 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
 
                return $wgContLang->getDir() !== $context->getDirection();
        }
+       
+       /**
+        * Get the modification times of all titles that would be loaded for
+        * a given context.
+        * @param $context ResourceLoaderContext: Context object
+        * @return array( prefixed DB key => UNIX timestamp ), nonexistent titles are dropped
+        */
+       protected function getTitleMtimes( ResourceLoaderContext $context ) {
+               $hash = $context->getHash();
+               if ( isset( $this->titleMtimes[$hash] ) ) {
+                       return $this->titleMtimes[$hash];
+               }
+               
+               $this->titleMtimes[$hash] = array();
+               $batch = new LinkBatch;
+               foreach ( $this->getPages( $context ) as $titleText => $options ) {
+                       $batch->addObj( Title::newFromText( $titleText ) );
+               }
+               
+               if ( !$batch->isEmpty() ) {
+                       $dbr = wfGetDB( DB_SLAVE );
+                       $res = $dbr->select( 'page',
+                               array( 'page_namespace', 'page_title', 'page_touched' ),
+                               $batch->constructSet( 'page', $dbr ),
+                               __METHOD__
+                       );
+                       foreach ( $res as $row ) {
+                               $title = Title::makeTitle( $row->page_namespace, $row->page_title );
+                               $this->titleMtimes[$hash][$title->getPrefixedDBkey()] =
+                                       wfTimestamp( TS_UNIX, $row->page_touched );
+                       }
+               }
+               return $this->titleMtimes[$hash];
+       }
 }