MediaWiki r52503 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r52502‎ | r52503 (on ViewVC)‎ | r52504 >
Date:07:11, 28 June 2009
Author:tstarling
Status:resolved (Comments)
Tags:
Comment:
* Introduced a new system for localisation caching. The system is based around fast fetches of individual messages, minimising memory overhead and startup time in the typical case. It handles both core messages (formerly in Language.php) and extension messages (formerly in MessageCache.php). Profiling indicates a significant win for average throughput.
* The serialized message cache, which would have been redundant, has been removed. Similar performance characteristics can be achieved with $wgLocalisationCacheConf['manualRecache'] = true;
* Added a maintenance script rebuildLocalisationCache.php for offline rebuilding of the localisation cache.
* Extension i18n files can now contain any of the variables which can be set in Messages*.php. It is possible, and recommended, to use this feature instead of the hooks for special page aliases and magic words.
* $wgExtensionAliasesFiles, LanguageGetMagic and LanguageGetSpecialPageAliases are retained for backwards compatibility. $wgMessageCache->addMessages() and related functions have been removed. wfLoadExtensionMessages() is a no-op and can continue to be called for b/c.
* Introduced $wgCacheDirectory as a default location for the various local caches that have accumulated. Suggested $IP/cache as a good place for it in the default LocalSettings.php and created this directory with a deny-all .htaccess.
* Patched Exception.php to avoid using the message cache when an exception is thrown from within LocalisationCache, since this tends to fail horribly.
* Removed Language::getLocalisationArray(), Language::loadLocalisation(), Language::load()
* Fixed FileDependency::__sleep()
* In Cdb.php, fixed newlines in debug messages

In MessageCache::get():
* Replaced calls to $wgContLang capitalisation functions with plain PHP functions, reducing the typical case from 99us to 93us. Message cache keys are already documented as being restricted to ASCII.
* Implemented a more efficient way to filter out bogus language codes, reducing the "foo/en" case from 430us to 101us
* Optimised wfRunHooks() in the typical do-nothing case, from ~30us to ~3us. This reduced MessageCache::get() typical case time from 93us to 38us.
* Removed hook MessageNotInMwNs to save an extra 3us per cache hit. Reimplemented the only user (LocalisationUpdate) using the new hook LocalisationCacheRecache.
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r52574readded $wgLocalMessageCacheSerialized (was removed in r52503), it's still us...ialex20:43, 29 June 2009
r52618(bug 19428) PG and MySQL followups to r52503. Patch by OverlordQdemon00:55, 1 July 2009
r52727* Re-added $wgMessageCache->addMessages(), there are still some extensions (i...tstarling06:19, 3 July 2009
r53042Fixed bug reported on CR r52503, invalid date format passed to $wgLang->date(...tstarling11:45, 10 July 2009
r53043Made wfMsg('') and wfMsg(null) silently return <> as it did before r525...tstarling11:54, 10 July 2009
r54936Simplistic reorganisation for compatibility with r52503. Load messages pre-ca...tstarling14:16, 13 August 2009
r70401Remove all calls to $wgMessageCache->loadAllMessages()...platonides19:15, 3 August 2010
r70691Removed $wgEnableSerializedMessages and $wgCheckSerialized again....nikerabbit13:06, 8 August 2010
r111349Updated the message preload list for Vector and other changes since r52503tstarling06:17, 13 February 2012

Comments

#Comment by Tim Starling (talk | contribs)   07:17, 28 June 2009

The DB patch is not required on WM, but $wgCacheDirectory needs to be set to /tmp/mediawiki, and the scap scripts need to be updated to run maintenance/rebuildLocalisationCache.php instead of serialized/Makefile.

#Comment by Voice of All (talk | contribs)   12:36, 28 June 2009

I keep getting the following: Notice: Array to string conversion in B:\www\MW_changes\includes\MessageCache.php on line 533

Notice: Undefined offset: -1 in B:\www\MW_changes\languages\Language.php on line 491

Notice: Uninitialized string offset: 0 in B:\www\MW_changes\includes\MessageCache.php on line 511

Notice: Array to string conversion in B:\www\MW_changes\includes\MessageCache.php on line 512

Warning: Illegal offset type in isset or empty in B:\www\MW_changes\includes\LocalisationCache.php on line 211

Warning: Illegal offset type in isset or empty in B:\www\MW_changes\includes\LocalisationCache.php on line 268

Warning: Illegal offset type in B:\www\MW_changes\includes\LocalisationCache.php on line 276

Warning: Illegal offset type in B:\www\MW_changes\includes\LocalisationCache.php on line 277

Warning: Illegal offset type in B:\www\MW_changes\includes\LocalisationCache.php on line 224

Notice: Array to string conversion in B:\www\MW_changes\includes\MessageCache.php on line 533

#Comment by Nikerabbit (talk | contribs)   18:12, 28 June 2009

Did r52524 fix any of those?

#Comment by Aaron Schulz (talk | contribs)   20:00, 28 June 2009

No. It seems to mainly effect reviewed pages.

#Comment by Tim Starling (talk | contribs)   06:45, 2 July 2009

Can't reproduce, but I will replace some of those warnings with exceptions so that they can be debugged more easily. Obviously message keys shouldn't be arrays.

#Comment by Voice of All (talk | contribs)   12:35, 2 July 2009

Try testing the Template:United States infobox template from enwikinews. I think I've actually narrowed it down to something with that.

#Comment by Voice of All (talk | contribs)   14:52, 6 July 2009

Also:

MessageCache::get: Invalid message key of type NULL

Backtrace:

  1. 0 B:\www\MW_changes\includes\GlobalFunctions.php(646): MessageCache->get(NULL, true, Object(Language))
  2. 1 B:\www\MW_changes\includes\GlobalFunctions.php(779): wfMsgGetKey(NULL, true, Object(Language), false)
  3. 2 B:\www\MW_changes\languages\Language.php(482): wfMsgExt(NULL, Array)
  4. 3 B:\www\MW_changes\languages\Language.php(494): Language->getMessageFromDB(NULL)
  5. 4 B:\www\MW_changes\languages\Language.php(748): Language->getMonthName('-0')
  6. 5 B:\www\MW_changes\languages\Language.php(1420): Language->sprintfDate('j F Y', '2009-06-28 12:3...')
  7. 6 B:\www\MW_changes\extensions\intersection\DynamicPageList.php(449): Language->date('2009-06-28 12:3...')
#Comment by Happy-melon (talk | contribs)   15:42, 30 June 2009

I'm pretty sure this is responsible for all the extension messages suddenly failing to load on my test wiki. What needs to be changed in extensions to get messages to load correctly? Who is going to make said changes?

#Comment by Tim Starling (talk | contribs)   15:28, 1 July 2009

Sorry about that. Someone led me to believe that $wgMessageCache->addMessages() had been removed from the extensions that used it, long ago. I didn't bother to check.

#Comment by Happy-melon (talk | contribs)   17:35, 1 July 2009

These are extensions like Oversight; I don't think this is just a case of using deprecated accessors.

#Comment by OverlordQ (talk | contribs)   20:56, 30 June 2009

Few database followups to this on bug #19428

#Comment by Shinjiman (talk | contribs)   15:09, 1 July 2009

I've got the following errors when the $wgLanguageCode in LocalSettings.php is to one of the LanguageConverter enabled code, e.g. zh, ar, kk, etc. Not sure whether was that happened for this revision or not:

  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\includes\\db\\Database.php on line 916
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\includes\\db\\DatabaseMysql.php on line 12
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\extensions\\NewestPages\\NewestPages.i18n.php on line 10
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\extensions\\AbuseFilter\\AbuseFilter.i18n.php on line 8
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\includes\\db\\DatabaseMysql.php on line 12
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\includes\\db\\DatabaseMysql.php on line 69
  • PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\\Users\\Man\\Website\\mediawiki_svn\\includes\\LocalisationCache.php on line 720
#Comment by Tim Starling (talk | contribs)   06:42, 2 July 2009

Works for me, with a recache time of ~0.8 seconds. Is your database very slow?

#Comment by Jidanni (talk | contribs)   11:01, 2 July 2009

Please see Buzilla:19447. If one's $wgLanguageCode!='en' one's wiki will get stuck, unable to serve HTTP, and unable to run update.php. ~~~~

#Comment by Jidanni (talk | contribs)   11:02, 2 July 2009

Please see Bugzilla:19447. If one's $wgLanguageCode!='en' one's wiki will get stuck, unable to serve HTTP, and unable to run update.php.

#Comment by Tim Starling (talk | contribs)   08:05, 3 July 2009

More related changes in r52732.

#Comment by Jidanni (talk | contribs)   03:45, 6 July 2009

We read in config/index.php

## Set \$wgCacheDirectory to a writable directory on the web server
## to make your wiki go slightly faster. The directory should not
## be publically accessible from the web.
#\$wgCacheDirectory = \"\$IP/cache\";

I assume "slightly" and that it is commented out mean: don't bother. OK.

#Comment by Tim Starling (talk | contribs)   04:51, 6 July 2009

It's commented out because it will completely break the wiki if the cache directory is not writable. The amount of speed increase is dependent on hardware. I can assure you that Wikimedia most certainly will be bothering.

#Comment by Jidanni (talk | contribs)   05:02, 6 July 2009

OK. I assume my 'one recent change a day' wikis won't benefit, but it would be neat if some guide figures were mentioned for when it would be worth it. Glad it looks flexible enough for Manual:Wiki_family users.

#Comment by Jidanni (talk | contribs)   19:34, 6 July 2009

And: Ah, so the bigger the wiki, the more it is useful. please mention that in the comment. else the user could have just as well guessed the opposite.

#Comment by Mr.Z-man (talk | contribs)   03:40, 20 July 2009

The update to the LocalisationUpdate extension seems to have not been done yet.

#Comment by Catrope (talk | contribs)   21:06, 18 August 2009

Done now.

Status & tagging log