jenkins-bot has submitted this change and it was merged.
Change subject: Updates after performance review
......................................................................
Updates after performance review
* Use MapCacheLRU in displayGlobalPage()
* Optimize brokenLink to avoid DB queries if already known
* Don't check prefs if GlobalPreferences is disabled
* Use MapCacheLRU in getCentralTouched() and call
reuseConnection()
* getRemoteParsedText() has a fixed length memcached key
* Optionally use $wgConf in getRemoteURL()
* Use a custom timeout for internal API requests
* Gracefully fail if an internal API request fails
Bug: 70585
Change-Id: I7440151f29bf38b6c135d8508ac1a0cf24a5677e
---
M GlobalUserPage.body.php
M GlobalUserPage.hooks.php
M GlobalUserPage.php
3 files changed, 117 insertions(+), 55 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/GlobalUserPage.body.php b/GlobalUserPage.body.php
index 421aeb6..df8f89c 100644
--- a/GlobalUserPage.body.php
+++ b/GlobalUserPage.body.php
@@ -12,6 +12,16 @@
*/
private $cache;
+ /**
+ * @var MapCacheLRU
+ */
+ private static $displayCache;
+
+ /**
+ * @var MapCacheLRU
+ */
+ private static $touchedCache;
+
public function __construct( Title $title, Config $config ) {
global $wgMemc;
parent::__construct( $title );
@@ -32,8 +42,9 @@
$out = $this->getContext()->getOutput();
$parsedOutput = $this->getRemoteParsedText(
self::getCentralTouched( $user ) );
- // If the user page is empty, don't use it
- if ( !trim( $parsedOutput['text']['*'] ) ) {
+ // If the user page is empty or the API request failed, show
the normal
+ // missing article page
+ if ( !$parsedOutput || !trim( $parsedOutput['text']['*'] ) ) {
parent::showMissingArticle();
return;
}
@@ -79,15 +90,6 @@
}
/**
- * @param int $type DB_SLAVE or DB_MASTER
- * @return DatabaseBase
- */
- protected static function getRemoteDB( $type ) {
- global $wgGlobalUserPageDBname;
- return wfGetDB( $type, array(), $wgGlobalUserPageDBname );
- }
-
- /**
* Given a Title, assuming it doesn't exist, should
* we display a global user page on it
*
@@ -96,28 +98,33 @@
*/
public static function displayGlobalPage( Title $title ) {
global $wgGlobalUserPageDBname;
- static $cache = array();
+ if ( !self::$displayCache ) {
+ self::$displayCache = new MapCacheLRU( 100 );
+ }
$text = $title->getPrefixedText();
// Do some instance caching since this can be
// called frequently due do the Linker hook
- if ( isset( $cache[$text] ) ) {
- return $cache[$text];
+ if ( self::$displayCache->has( $text ) ) {
+ return self::$displayCache->get( $text );
}
if ( !self::canBeGlobal( $title ) ) {
- $cache[$text] = false;
+ self::$displayCache->set( $text, false );
return false;
}
$user = User::newFromName( $title->getText() );
if ( !$user || $user->getId() === 0 ) {
- $cache[$text] = false;
+ self::$displayCache->set( $text, false );
return false;
}
- if ( !$user->getOption( 'globaluserpage' ) ) {
- $cache[$text] = false;
- return false;
+ // Only check preferences if E:GlobalPreferences is installed
+ if ( class_exists( 'GlobalPreferences' ) ) {
+ if ( !$user->getOption( 'globaluserpage' ) ) {
+ self::$displayCache->set( $text, false );
+ return false;
+ }
}
// Allow for authorization extensions to determine
@@ -125,12 +132,13 @@
// This hook intentionally functions the same
// as the one in Extension:GlobalCssJs.
if ( !wfRunHooks( 'LoadGlobalUserPage', array( $user,
$wgGlobalUserPageDBname, wfWikiID() ) ) ) {
- $cache[$text] = false;
+ self::$displayCache->set( $text, false );
return false;
}
- $cache[$text] = (bool)self::getCentralTouched( $user );
- return $cache[$text];
+ $touched = (bool)self::getCentralTouched( $user );
+ self::$displayCache->set( $text, $touched );
+ return $touched;
}
/**
@@ -141,20 +149,27 @@
* @return string|bool
*/
protected static function getCentralTouched( User $user ) {
- static $cache = array();
- if ( !isset( $cache[$user->getName()] ) ) {
- $cache[$user->getName()] = self::getRemoteDB( DB_SLAVE
)->selectField(
- 'page',
- 'page_touched',
- array(
- 'page_namespace' => NS_USER,
- 'page_title' =>
$user->getUserPage()->getDBkey()
- ),
- __METHOD__
- );
+ if ( self::$touchedCache->has( $user->getName() ) ) {
+ return self::$touchedCache->get( $user->getName() );
}
- return $cache[$user->getName()];
+ global $wgGlobalUserPageDBname;
+ $lb = wfGetLB( $wgGlobalUserPageDBname );
+ $dbr = $lb->getConnection( DB_SLAVE, array(),
$wgGlobalUserPageDBname );
+ $touched = $dbr->selectField(
+ 'page',
+ 'page_touched',
+ array(
+ 'page_namespace' => NS_USER,
+ 'page_title' => $user->getUserPage()->getDBkey()
+ ),
+ __METHOD__
+ );
+ $lb->reuseConnection( $dbr );
+
+ self::$touchedCache->set( $user->getName(), $touched );
+
+ return $touched;
}
/**
@@ -194,11 +209,16 @@
$langCode = $this->getContext()->getLanguage()->getCode();
// Need language code in the key since we pass &uselang= to the
API.
- $key =
"globaluserpage:parsed:$touched:$langCode:{$this->getUsername()}";
+ $key = "globaluserpage:parsed:$touched:$langCode:" . md5(
$this->getUsername() );
$data = $this->cache->get( $key );
if ( $data === false ){
$data = $this->parseWikiText( $this->getTitle(),
$langCode );
- $this->cache->set( $key, $data, $this->config->get(
'GlobalUserPageCacheExpiry' ) );
+ if ( $data ) {
+ $this->cache->set( $key, $data,
$this->config->get( 'GlobalUserPageCacheExpiry' ) );
+ } else {
+ // Cache failure for 10 seconds
+ $this->cache->set( $key, null, 10 );
+ }
}
return $data;
@@ -239,16 +259,43 @@
* Makes an API request to the central wiki
*
* @param $params array
- * @return array
+ * @return array|bool false if the request failed
*/
protected function makeAPIRequest( $params ) {
$params['format'] = 'json';
$url = wfAppendQuery( $this->config->get(
'GlobalUserPageAPIUrl' ), $params );
- $req = MWHttpRequest::factory( $url );
- $req->execute();
+ $req = MWHttpRequest::factory(
+ $url,
+ array( 'timeout' => $this->config->get(
'GlobalUserPageTimeout' ) )
+ );
+ $status = $req->execute();
+ if ( !$status->isOK() ) {
+ return false;
+ }
$json = $req->getContent();
$decoded = FormatJson::decode( $json, true );
return $decoded;
+ }
+
+ /**
+ * Returns a URL to the user page on the central wiki,
+ * attempts to use SiteConfiguration if possible, else
+ * falls back to using an API request
+ *
+ * @return string
+ */
+ protected function getRemoteURL() {
+ $url = WikiMap::getForeignURL(
+ $this->config->get( 'GlobalUserPageDBname' ),
+ 'User:' . $this->getUsername()
+ );
+
+ if ( $url !== false ) {
+ return $url;
+ } else {
+ // Fallback to the API
+ return $this->getRemoteURLFromAPI();
+ }
}
/**
@@ -259,7 +306,7 @@
*
* @return string
*/
- protected function getRemoteURL() {
+ protected function getRemoteURLFromAPI() {
$key = 'globaluserpage:url:' . md5( $this->getUsername() );
$data = $this->cache->get( $key );
if ( $data === false ) {
@@ -271,6 +318,10 @@
'indexpageids' => '1',
);
$resp = $this->makeAPIRequest( $params );
+ if ( $resp === false ) {
+ // Don't cache upon failure
+ return '';
+ }
$pageInfo =
$resp['query']['pages'][$resp['query']['pageids'][0]];
if ( isset( $pageInfo['canonicalurl'] ) ) {
// New in 1.24
@@ -308,6 +359,6 @@
'prop' => 'text|modules'
);
$data = $this->makeAPIRequest( $params );
- return $data['parse'];
+ return $data !== false ? $data['parse'] : false;
}
}
diff --git a/GlobalUserPage.hooks.php b/GlobalUserPage.hooks.php
index 5ea610d..6b09ea5 100644
--- a/GlobalUserPage.hooks.php
+++ b/GlobalUserPage.hooks.php
@@ -102,22 +102,25 @@
/**
* Turn red User: links into blue ones
*
- * @param $linker Linker
- * @param $target Title
- * @param $text String
- * @param $customAtrribs Array: array of custom attributes [unused]
- * @param $query [unused]
- * @param $ret String: return value (link HTML)
- * @return Boolean
+ * @param DummyLinker $linker for b/c
+ * @param Title $target
+ * @param string $text
+ * @param array $customAttribs custom attributes
+ * @param array $query
+ * @param array $options
+ * @param string $ret return value (link HTML)
+ * @return bool
*/
public static function brokenLink( $linker, $target, &$text,
&$customAttribs, &$query, &$options, &$ret ) {
+ if ( in_array( 'known', $options ) || $target->isKnown() ) {
+ return true;
+ }
+
if ( GlobalUserPage::displayGlobalPage( $target ) ) {
- if ( in_array( 'known', $options ) ||
$target->isKnown() ) {
- return true;
- } else {
- $ret = Linker::linkKnown( $target, $text );
- return false;
- }
+ $options = array_merge(
+ $options,
+ array( 'known', 'noclasses' )
+ );
}
return true;
diff --git a/GlobalUserPage.php b/GlobalUserPage.php
index 5e4f534..bafa0a5 100644
--- a/GlobalUserPage.php
+++ b/GlobalUserPage.php
@@ -68,11 +68,19 @@
*/
$wgGlobalUserPageCSSRLSourceName = false;
+/**
+ * Timeout for internal API requests. To use $wgHTTPTimeout,
+ * set this to 'default'
+ *
+ * @var string|int
+ */
+$wgGlobalUserPageTimeout = 10;
+
// Extension credits that will show up on Special:Version
$wgExtensionCredits['other'][] = array(
'path' => __FILE__,
'name' => 'GlobalUserPage',
- 'version' => '0.9',
+ 'version' => '0.9.1',
'author' => array( 'Kunal Mehta', 'Jack Phoenix' ),
'url' => 'https://www.mediawiki.org/wiki/Extension:GlobalUserPage',
'descriptionmsg' => 'globaluserpage-desc',
--
To view, visit https://gerrit.wikimedia.org/r/169653
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7440151f29bf38b6c135d8508ac1a0cf24a5677e
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/GlobalUserPage
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Jack Phoenix <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: MZMcBride <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
