Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Optimise Skin::getLanguages()

Browse source 
Skin::getLanguages() was consuming 4% of index.php CPU time. In local
testing, it was called three times per page view. So:

* Memoize it, analogous to the nonfunctional code in SkinVector.
* Simplify ClassicInterwikiLookup by removing the option to pass a CDB
  file path. This was only ever supported by a WikimediaMaintenance
  script. In the unlikely event that someone is using this feature, they
  have the same motivation to switch to PHP as we did in T122362.
* Increase the size of ClassicInterwikiLookup's MapCacheLRU from 100 to
  1000. This helps greatly in the case when $wgInterwikiCache is false
  and more than 100 interwikis are requested and seems harmless
  otherwise.
* Optimise Title::getNsText() by assuming that the canonical name of
  NS_MAIN is the empty string.
* Rearrange Message::__construct() to avoid duplicate type checks.

Change-Id: I736cb74efc267fd2473a3267471735238217251c
This commit is contained in:
Tim Starling 2022-02-08 12:35:25 +11:00 committed by Krinkle
parent 45d6341cab
commit b90d2dd5c2
7 changed files with 169 additions and 280 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
RELEASE-NOTES-1.38
View file

@ -16,10 +16,11 @@ oldest supported upgrading version, MediaWiki 1.29.
Some specific notes for MediaWiki 1.38 upgrades are below:
* …
For notes on 1.36.x and older releases, see HISTORY.
For notes on 1.37.x and older releases, see HISTORY.
=== Configuration changes for system administrators in 1.38 ===
* Deprecate $wgAjaxUploadDestCheck, act as always-true
* $wgInterwikiCache no longer supports the string value for CDB files.
* …
==== New configuration ====

5
includes/DefaultSettings.php
View file

@ -4803,8 +4803,7 @@ $wgLocalInterwikis = [];
$wgInterwikiExpiry = 10800;
/**
* Interwiki cache, either as an associative array or a path to a constant
* database (.cdb) file.
* Interwiki cache as an associative array
*
* This data structure database is generated by the `dumpInterwiki` maintenance
* script (which lives in the WikimediaMaintenance repository) and has key
@ -4818,7 +4817,7 @@ $wgInterwikiExpiry = 10800;
* Sites mapping just specifies site name, other keys provide "local url"
* data layout.
*
* @var bool|array|string
* @var bool|array
*/
$wgInterwikiCache = false;

13
includes/Title.php
View file

@ -1161,10 +1161,15 @@ class Title implements LinkTarget, PageIdentity, IDBAccessObject {
if ( $this->isExternal() ) {
// This probably shouldn't even happen, except for interwiki transclusion.
// If possible, use the canonical name for the foreign namespace.
$nsText = MediaWikiServices::getInstance()->getNamespaceInfo()->
getCanonicalName( $this->mNamespace );
if ( $nsText !== false ) {
return $nsText;
if ( $this->mNamespace === NS_MAIN ) {
// Optimisation
return '';
} else {
$nsText = MediaWikiServices::getInstance()->getNamespaceInfo()->
getCanonicalName( $this->mNamespace );
if ( $nsText !== false ) {
return $nsText;
}
}
}

180
includes/interwiki/ClassicInterwikiLookup.php
View file

@ -22,13 +22,12 @@
namespace MediaWiki\Interwiki;
use Cdb\Exception as CdbException;
use Cdb\Reader as CdbReader;
use Interwiki;
use Language;
use MapCacheLRU;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MWException;
use WANObjectCache;
use WikiMap;
use Wikimedia\Rdbms\Database;
@ -38,7 +37,7 @@ use Wikimedia\Rdbms\ILoadBalancer;
* InterwikiLookup implementing the "classic" interwiki storage (hardcoded up to MW 1.26).
*
* This implements two levels of caching (in-process array and a WANObjectCache)
* and three storage backends (SQL, CDB, and plain PHP arrays).
* and two storage backends (SQL and plain PHP arrays).
*
* All information is loaded on creation when called by $this->fetch( $prefix ).
* All work is done on replica DB, because this should *never* change (except during
@ -69,9 +68,9 @@ class ClassicInterwikiLookup implements InterwikiLookup {
private $objectCacheExpiry;
/**
* @var bool|array|string
* @var array|null Complete pregenerated data if available
*/
private $cdbData;
private $data;
/**
* @var int
@ -83,11 +82,6 @@ class ClassicInterwikiLookup implements InterwikiLookup {
*/
private $fallbackSite;
/**
* @var CdbReader|null
*/
private $cdbReader = null;
/**
* @var string|null
*/
@ -105,9 +99,8 @@ class ClassicInterwikiLookup implements InterwikiLookup {
* @param HookContainer $hookContainer
* @param ILoadBalancer $loadBalancer
* @param int $objectCacheExpiry Expiry time for $objectCache, in seconds
* @param bool|array|string $cdbData The path of a CDB file, or
* an array resembling the contents of a CDB file,
* or false to use the database.
* @param bool|array $interwikiData The pre-generated interwiki data, or
* false to use the database.
* @param int $interwikiScopes Specify number of domains to check for messages:
* - 1: Just local wiki level
* - 2: wiki and global levels
@ -120,18 +113,23 @@ class ClassicInterwikiLookup implements InterwikiLookup {
HookContainer $hookContainer,
ILoadBalancer $loadBalancer,
$objectCacheExpiry,
$cdbData,
$interwikiData,
$interwikiScopes,
$fallbackSite
) {
$this->localCache = new MapCacheLRU( 100 );
$this->localCache = new MapCacheLRU( 1000 );
$this->contLang = $contLang;
$this->objectCache = $objectCache;
$this->hookRunner = new HookRunner( $hookContainer );
$this->loadBalancer = $loadBalancer;
$this->objectCacheExpiry = $objectCacheExpiry;
$this->cdbData = $cdbData;
if ( is_array( $interwikiData ) ) {
$this->data = $interwikiData;
} elseif ( $interwikiData ) {
throw new MWException(
'Setting $wgInterwikiCache to a CDB path is no longer supported' );
}
$this->interwikiScopes = $interwikiScopes;
$this->fallbackSite = $fallbackSite;
}
@ -160,11 +158,12 @@ class ClassicInterwikiLookup implements InterwikiLookup {
}
$prefix = $this->contLang->lc( $prefix );
return $this->localCache->getWithSetCallback(
$prefix,
function () use ( $prefix ) {
if ( $this->cdbData ) {
$iw = $this->getInterwikiCached( $prefix );
if ( $this->data !== null ) {
$iw = $this->fetchPregenerated( $prefix );
} else {
$iw = $this->load( $prefix );
if ( !$iw ) {
@ -204,8 +203,8 @@ class ClassicInterwikiLookup implements InterwikiLookup {
* @param string $prefix Interwiki prefix
* @return Interwiki|false
*/
private function getInterwikiCached( $prefix ) {
$value = $this->getInterwikiCacheEntry( $prefix );
private function fetchPregenerated( $prefix ) {
$value = $this->getPregeneratedEntry( $prefix );
if ( $value ) {
// Split values
@ -217,66 +216,36 @@ class ClassicInterwikiLookup implements InterwikiLookup {
}
/**
* Get entry from interwiki cache
* Get entry from pregenerated data
*
* @note More logic is explained in DefaultSettings.
*
* @param string $prefix Database key
* @return bool|string The interwiki entry or false if not found
*/
private function getInterwikiCacheEntry( $prefix ) {
private function getPregeneratedEntry( $prefix ) {
wfDebug( __METHOD__ . "( $prefix )" );
$wikiId = WikiMap::getCurrentWikiId();
$value = false;
try {
// Resolve site name
if ( $this->interwikiScopes >= 3 && !$this->thisSite ) {
$this->thisSite = $this->getCacheValue( '__sites:' . $wikiId );
if ( $this->thisSite == '' ) {
$this->thisSite = $this->fallbackSite;
}
}
// Resolve site name
if ( $this->interwikiScopes >= 3 && !$this->thisSite ) {
$this->thisSite = $this->data['__sites:' . $wikiId] ?? $this->fallbackSite;
}
$value = $this->getCacheValue( $wikiId . ':' . $prefix );
// Site level
if ( $value == '' && $this->interwikiScopes >= 3 ) {
$value = $this->getCacheValue( "_{$this->thisSite}:{$prefix}" );
}
// Global Level
if ( $value == '' && $this->interwikiScopes >= 2 ) {
$value = $this->getCacheValue( "__global:{$prefix}" );
}
if ( $value == 'undef' ) {
$value = '';
}
} catch ( CdbException $e ) {
wfDebug( __METHOD__ . ": CdbException caught, error message was "
. $e->getMessage() );
$value = $this->data[$wikiId . ':' . $prefix] ?? false;
// Site level
if ( $value === false && $this->interwikiScopes >= 3 ) {
$value = $this->data["_{$this->thisSite}:{$prefix}"] ?? false;
}
// Global Level
if ( $value === false && $this->interwikiScopes >= 2 ) {
$value = $this->data["__global:{$prefix}"] ?? false;
}
return $value;
}
private function getCacheValue( $key ) {
if ( $this->cdbReader === null ) {
if ( is_string( $this->cdbData ) ) {
$this->cdbReader = \Cdb\Reader::open( $this->cdbData );
} elseif ( is_array( $this->cdbData ) ) {
$this->cdbReader = new \Cdb\Reader\Hash( $this->cdbData );
} else {
$this->cdbReader = false;
}
}
if ( $this->cdbReader ) {
return $this->cdbReader->get( $key );
} else {
return false;
}
}
/**
* Load the interwiki, trying first memcached then the DB
*
@ -344,65 +313,54 @@ class ClassicInterwikiLookup implements InterwikiLookup {
}
/**
* Fetch all interwiki prefixes from interwiki cache
* Fetch all interwiki prefixes from pregenerated data
*
* @param null|string $local If not null, limits output to local/non-local interwikis
* @return array List of prefixes, where each row is an associative array
*/
private function getAllPrefixesCached( $local ) {
private function getAllPrefixesPregenerated( $local ) {
wfDebug( __METHOD__ . "()" );
$wikiId = WikiMap::getCurrentWikiId();
$data = [];
try {
/* Resolve site name */
if ( $this->interwikiScopes >= 3 && !$this->thisSite ) {
$site = $this->getCacheValue( '__sites:' . $wikiId );
/* Resolve site name */
if ( $this->interwikiScopes >= 3 && !$this->thisSite ) {
$this->thisSite = $this->data['__sites:' . $wikiId] ?? $this->fallbackSite;
}
if ( $site == '' ) {
$this->thisSite = $this->fallbackSite;
} else {
$this->thisSite = $site;
// List of interwiki sources
$sources = [];
// Global Level
if ( $this->interwikiScopes >= 2 ) {
$sources[] = '__global';
}
// Site level
if ( $this->interwikiScopes >= 3 ) {
$sources[] = '_' . $this->thisSite;
}
$sources[] = $wikiId;
foreach ( $sources as $source ) {
$list = $this->data['__list:' . $source] ?? '';
foreach ( explode( ' ', $list ) as $iw_prefix ) {
$row = $this->data["{$source}:{$iw_prefix}"] ?? null;
if ( !$row ) {
continue;
}
}
// List of interwiki sources
$sources = [];
// Global Level
if ( $this->interwikiScopes >= 2 ) {
$sources[] = '__global';
}
// Site level
if ( $this->interwikiScopes >= 3 ) {
$sources[] = '_' . $this->thisSite;
}
$sources[] = $wikiId;
list( $iw_local, $iw_url ) = explode( ' ', $row );
foreach ( $sources as $source ) {
$list = $this->getCacheValue( '__list:' . $source );
foreach ( explode( ' ', $list ) as $iw_prefix ) {
$row = $this->getCacheValue( "{$source}:{$iw_prefix}" );
if ( !$row ) {
continue;
}
list( $iw_local, $iw_url ) = explode( ' ', $row );
if ( $local !== null && $local != $iw_local ) {
continue;
}
$data[$iw_prefix] = [
'iw_prefix' => $iw_prefix,
'iw_url' => $iw_url,
'iw_local' => $iw_local,
];
if ( $local !== null && $local != $iw_local ) {
continue;
}
$data[$iw_prefix] = [
'iw_prefix' => $iw_prefix,
'iw_url' => $iw_url,
'iw_local' => $iw_local,
];
}
} catch ( CdbException $e ) {
wfDebug( __METHOD__ . ": CdbException caught, error message was "
. $e->getMessage() );
}
return array_values( $data );
@ -410,7 +368,7 @@ class ClassicInterwikiLookup implements InterwikiLookup {
/**
* Given the array returned by getAllPrefixes(), build a PHP hash which
* can be given to \Cdb\Reader\Hash() as $this->cdbData, ie as the
* can be given to self::__construct() as $interwikiData, i.e. as the
* value of $wgInterwikiCache. This is used to construct mock
* interwiki lookup services for testing (in particular, parsertests).
* @param array $allPrefixes An array of interwiki information such as
@ -483,8 +441,8 @@ class ClassicInterwikiLookup implements InterwikiLookup {
* @return array[] Interwiki rows, where each row is an associative array
*/
public function getAllPrefixes( $local = null ) {
if ( $this->cdbData ) {
return $this->getAllPrefixesCached( $local );
if ( $this->data !== null ) {
return $this->getAllPrefixesPregenerated( $local );
}
return $this->getAllPrefixesDB( $local );

19
includes/language/Message.php
View file

@ -232,18 +232,19 @@ class Message implements MessageSpecifier, Serializable {
$key = $key->getKey();
}
if ( !is_string( $key ) && !is_array( $key ) ) {
if ( is_string( $key ) ) {
$this->keysToTry = [ $key ];
$this->key = $key;
} elseif ( is_array( $key ) ) {
if ( !$key ) {
throw new InvalidArgumentException( '$key must not be an empty list' );
}
$this->keysToTry = $key;
$this->key = reset( $this->keysToTry );
} else {
throw new InvalidArgumentException( '$key must be a string or an array' );
}
$this->keysToTry = (array)$key;
if ( empty( $this->keysToTry ) ) {
throw new InvalidArgumentException( '$key must not be an empty list' );
}
$this->key = reset( $this->keysToTry );
$this->parameters = array_values( $params );
// User language is only resolved in getLanguage(). This helps preserve the
// semantic intent of "user language" across serialize() and unserialize().

158
includes/skins/Skin.php
View file

@ -77,6 +77,9 @@ abstract class Skin extends ContextSource {
/** @var string cached action for cheap lookup */
protected $action;
/** @var array|null Cache for getLanguages() */
private $languageLinks;
/**
* Get the current major version of Skin. This is used to manage changes
* to underlying data and for providing support for older and new versions of code.
@ -1338,100 +1341,93 @@ abstract class Skin extends ContextSource {
if ( $this->getConfig()->get( 'HideInterlanguageLinks' ) ) {
return [];
}
$hookContainer = MediaWikiServices::getInstance()->getHookContainer();
if ( $this->languageLinks === null ) {
$hookContainer = MediaWikiServices::getInstance()->getHookContainer();
$userLang = $this->getLanguage();
$languageLinks = [];
$langNameUtils = MediaWikiServices::getInstance()->getLanguageNameUtils();
$userLang = $this->getLanguage();
$languageLinks = [];
$langNameUtils = MediaWikiServices::getInstance()->getLanguageNameUtils();
foreach ( $this->getOutput()->getLanguageLinks() as $languageLinkText ) {
$class = 'interlanguage-link interwiki-' . explode( ':', $languageLinkText, 2 )[0];
foreach ( $this->getOutput()->getLanguageLinks() as $languageLinkText ) {
$class = 'interlanguage-link interwiki-' . explode( ':', $languageLinkText, 2 )[0];
$languageLinkTitle = Title::newFromText( $languageLinkText );
if ( !$languageLinkTitle ) {
continue;
}
$ilInterwikiCode = $this->mapInterwikiToLanguage( $languageLinkTitle->getInterwiki() );
$ilLangName = $langNameUtils->getLanguageName( $ilInterwikiCode );
if ( strval( $ilLangName ) === '' ) {
$ilDisplayTextMsg = $this->msg( "interlanguage-link-$ilInterwikiCode" );
if ( !$ilDisplayTextMsg->isDisabled() ) {
// Use custom MW message for the display text
$ilLangName = $ilDisplayTextMsg->text();
} else {
// Last resort: fallback to the language link target
$ilLangName = $languageLinkText;
$languageLinkTitle = Title::newFromText( $languageLinkText );
if ( !$languageLinkTitle ) {
continue;
}
} else {
// Use the language autonym as display text
$ilLangName = $this->getLanguage()->ucfirst( $ilLangName );
}
// CLDR extension or similar is required to localize the language name;
// otherwise we'll end up with the autonym again.
$ilLangLocalName = $langNameUtils->getLanguageName(
$ilInterwikiCode,
$userLang->getCode()
);
$ilInterwikiCode =
$this->mapInterwikiToLanguage( $languageLinkTitle->getInterwiki() );
$languageLinkTitleText = $languageLinkTitle->getText();
if ( $ilLangLocalName === '' ) {
$ilFriendlySiteName = $this->msg( "interlanguage-link-sitename-$ilInterwikiCode" );
if ( !$ilFriendlySiteName->isDisabled() ) {
if ( $languageLinkTitleText === '' ) {
$ilTitle = $this->msg(
'interlanguage-link-title-nonlangonly',
$ilFriendlySiteName->text()
)->text();
$ilLangName = $langNameUtils->getLanguageName( $ilInterwikiCode );
if ( strval( $ilLangName ) === '' ) {
$ilDisplayTextMsg = $this->msg( "interlanguage-link-$ilInterwikiCode" );
if ( !$ilDisplayTextMsg->isDisabled() ) {
// Use custom MW message for the display text
$ilLangName = $ilDisplayTextMsg->text();
} else {
$ilTitle = $this->msg(
'interlanguage-link-title-nonlang',
$languageLinkTitleText,
$ilFriendlySiteName->text()
)->text();
// Last resort: fallback to the language link target
$ilLangName = $languageLinkText;
}
} else {
// we have nothing friendly to put in the title, so fall back to
// displaying the interlanguage link itself in the title text
// (similar to what is done in page content)
$ilTitle = $languageLinkTitle->getInterwiki() .
":$languageLinkTitleText";
// Use the language autonym as display text
$ilLangName = $this->getLanguage()->ucfirst( $ilLangName );
}
} elseif ( $languageLinkTitleText === '' ) {
$ilTitle = $this->msg(
'interlanguage-link-title-langonly',
$ilLangLocalName
)->text();
} else {
$ilTitle = $this->msg(
'interlanguage-link-title',
$languageLinkTitleText,
$ilLangLocalName
)->text();
}
$ilInterwikiCodeBCP47 = LanguageCode::bcp47( $ilInterwikiCode );
$languageLink = [
'href' => $languageLinkTitle->getFullURL(),
'text' => $ilLangName,
'title' => $ilTitle,
'class' => $class,
'link-class' => 'interlanguage-link-target',
'lang' => $ilInterwikiCodeBCP47,
'hreflang' => $ilInterwikiCodeBCP47,
];
$hookContainer->run(
'SkinTemplateGetLanguageLink',
[ &$languageLink, $languageLinkTitle, $this->getTitle(), $this->getOutput() ],
[]
);
$languageLinks[] = $languageLink;
// CLDR extension or similar is required to localize the language name;
// otherwise we'll end up with the autonym again.
$ilLangLocalName =
$langNameUtils->getLanguageName( $ilInterwikiCode, $userLang->getCode() );
$languageLinkTitleText = $languageLinkTitle->getText();
if ( $ilLangLocalName === '' ) {
$ilFriendlySiteName =
$this->msg( "interlanguage-link-sitename-$ilInterwikiCode" );
if ( !$ilFriendlySiteName->isDisabled() ) {
if ( $languageLinkTitleText === '' ) {
$ilTitle =
$this->msg( 'interlanguage-link-title-nonlangonly',
$ilFriendlySiteName->text() )->text();
} else {
$ilTitle =
$this->msg( 'interlanguage-link-title-nonlang',
$languageLinkTitleText, $ilFriendlySiteName->text() )->text();
}
} else {
// we have nothing friendly to put in the title, so fall back to
// displaying the interlanguage link itself in the title text
// (similar to what is done in page content)
$ilTitle = $languageLinkTitle->getInterwiki() . ":$languageLinkTitleText";
}
} elseif ( $languageLinkTitleText === '' ) {
$ilTitle =
$this->msg( 'interlanguage-link-title-langonly', $ilLangLocalName )->text();
} else {
$ilTitle =
$this->msg( 'interlanguage-link-title', $languageLinkTitleText,
$ilLangLocalName )->text();
}
$ilInterwikiCodeBCP47 = LanguageCode::bcp47( $ilInterwikiCode );
$languageLink = [
'href' => $languageLinkTitle->getFullURL(),
'text' => $ilLangName,
'title' => $ilTitle,
'class' => $class,
'link-class' => 'interlanguage-link-target',
'lang' => $ilInterwikiCodeBCP47,
'hreflang' => $ilInterwikiCodeBCP47,
];
$hookContainer->run( 'SkinTemplateGetLanguageLink',
[ &$languageLink, $languageLinkTitle, $this->getTitle(), $this->getOutput() ],
[] );
$languageLinks[] = $languageLink;
}
$this->languageLinks = $languageLinks;
}
return $languageLinks;
return $this->languageLinks;
}
/**

71
tests/phpunit/includes/interwiki/ClassicInterwikiLookupTest.php
View file

@ -116,77 +116,6 @@ class ClassicInterwikiLookupTest extends MediaWikiIntegrationTestCase {
return $hash;
}
private function populateCDB( $thisSite, $local, $global ) {
$cdbFile = $this->getNewTempFile();
$cdb = \Cdb\Writer::open( $cdbFile );
$hash = $this->populateHash( $thisSite, $local, $global );
foreach ( $hash as $key => $value ) {
$cdb->set( $key, $value );
}
$cdb->close();
return $cdbFile;
}
public function testCDBStorage() {
// NOTE: CDB setup is expensive, so we only do
// it once and run all the tests in one go.
$zzwiki = [
'iw_prefix' => 'zz',
'iw_url' => 'http://zzwiki.org/wiki/',
'iw_local' => 0
];
$dewiki = [
'iw_prefix' => 'de',
'iw_url' => 'http://de.wikipedia.org/wiki/',
'iw_local' => 1
];
$cdbFile = $this->populateCDB(
'en',
[ $dewiki ],
[ $zzwiki ]
);
$lookup = new \MediaWiki\Interwiki\ClassicInterwikiLookup(
$this->getServiceContainer()->getLanguageFactory()->getLanguage( 'en' ),
WANObjectCache::newEmpty(),
$this->getServiceContainer()->getHookContainer(),
$this->getServiceContainer()->getDBLoadBalancer(),
60 * 60,
$cdbFile,
3,
'en'
);
$this->assertEquals(
[ $zzwiki, $dewiki ],
$lookup->getAllPrefixes(),
'getAllPrefixes()'
);
$this->assertTrue( $lookup->isValidInterwiki( 'de' ), 'known prefix is valid' );
$this->assertTrue( $lookup->isValidInterwiki( 'zz' ), 'known prefix is valid' );
$interwiki = $lookup->fetch( 'de' );
$this->assertInstanceOf( Interwiki::class, $interwiki );
$this->assertSame( 'http://de.wikipedia.org/wiki/', $interwiki->getURL(), 'getURL' );
$this->assertSame( true, $interwiki->isLocal(), 'isLocal' );
$interwiki = $lookup->fetch( 'zz' );
$this->assertInstanceOf( Interwiki::class, $interwiki );
$this->assertSame( 'http://zzwiki.org/wiki/', $interwiki->getURL(), 'getURL' );
$this->assertSame( false, $interwiki->isLocal(), 'isLocal' );
// cleanup temp file
unlink( $cdbFile );
}
public function testArrayStorage() {
$zzwiki = [
'iw_prefix' => 'zz',