From: Roan Kattouw Date: Sat, 19 Feb 2011 17:07:05 +0000 (+0000) Subject: (bug 27302) Avoid unnecessary requests for user and site modules if the relevant... X-Git-Tag: 1.31.0-rc.0~31889 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/ajouter.php?a=commitdiff_plain;h=880f09b10c0fe7016117aadae6926af2e3062f60;p=lhc%2Fweb%2Fwiklou.git (bug 27302) Avoid unnecessary requests for user and site modules if the relevant wiki pages don't exist. 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 1ee62c09c7..ad82b5a712 100644 --- 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 a8bb2c08d0..fee54432fd 100644 --- 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 266fb1cac4..40b1186816 100644 --- 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]; + } }