r92549 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92548‎ | r92549 | r92550 >
Date:18:35, 19 July 2011
Author:catrope
Status:reverted (Comments)
Tags:
Comment:
1.18: Back out adaptive message cache (r71412, r71418, r74177, r74179) by merging r81444 from REL1_17
Modified paths:
  • /branches/REL1_18/phase3/includes/DefaultSettings.php (modified) (history)
  • /branches/REL1_18/phase3/includes/Wiki.php (modified) (history)
  • /branches/REL1_18/phase3/includes/cache/MessageCache.php (modified) (history)

Diff [purge]

Index: branches/REL1_18/phase3/includes/Wiki.php
@@ -419,7 +419,6 @@
420420 * Ends this task peacefully
421421 */
422422 public function restInPeace() {
423 - MessageCache::logMessages();
424423 wfLogProfilingData();
425424 // Commit and close up!
426425 $factory = wfGetLBFactory();
Index: branches/REL1_18/phase3/includes/cache/MessageCache.php
@@ -43,26 +43,6 @@
4444 protected $mLoadedLanguages = array();
4545
4646 /**
47 - * Used for automatic detection of most used messages.
48 - */
49 - protected $mRequestedMessages = array();
50 -
51 - /**
52 - * How long the message request counts are stored. Longer period gives
53 - * better sample, but also takes longer to adapt changes. The counts
54 - * are aggregrated per day, regardless of the value of this variable.
55 - */
56 - protected static $mAdaptiveDataAge = 604800; // Is 7*24*3600
57 -
58 - /**
59 - * Filter the tail of less used messages that are requested more seldom
60 - * than this factor times the number of request of most requested message.
61 - * These messages are not loaded in the default set, but are still cached
62 - * individually on demand with the normal cache expiry time.
63 - */
64 - protected static $mAdaptiveInclusionThreshold = 0.05;
65 -
66 - /**
6747 * Singleton instance
6848 *
6949 * @var MessageCache
@@ -369,12 +349,12 @@
370350 * $wgMaxMsgCacheEntrySize are assigned a special value, and are loaded
371351 * on-demand from the database later.
372352 *
373 - * @param $code String: language code.
374 - * @return Array: loaded messages for storing in caches.
 353+ * @param $code Optional language code, see documenation of load().
 354+ * @return Array: Loaded messages for storing in caches.
375355 */
376 - function loadFromDB( $code ) {
 356+ function loadFromDB( $code = false ) {
377357 wfProfileIn( __METHOD__ );
378 - global $wgMaxMsgCacheEntrySize, $wgLanguageCode, $wgAdaptiveMessageCache;
 358+ global $wgMaxMsgCacheEntrySize, $wgLanguageCode;
379359 $dbr = wfGetDB( DB_SLAVE );
380360 $cache = array();
381361
@@ -384,26 +364,19 @@
385365 'page_namespace' => NS_MEDIAWIKI,
386366 );
387367
388 - $mostused = array();
389 - if ( $wgAdaptiveMessageCache ) {
390 - $mostused = $this->getMostUsedMessages();
 368+ if ( $code ) {
 369+ # Is this fast enough. Should not matter if the filtering is done in the
 370+ # database or in code.
391371 if ( $code !== $wgLanguageCode ) {
392 - foreach ( $mostused as $key => $value ) {
393 - $mostused[$key] = "$value/$code";
394 - }
 372+ # Messages for particular language
 373+ $conds[] = 'page_title' . $dbr->buildLike( $dbr->anyString(), "/$code" );
 374+ } else {
 375+ # Effectively disallows use of '/' character in NS_MEDIAWIKI for uses
 376+ # other than language code.
 377+ $conds[] = 'page_title NOT' . $dbr->buildLike( $dbr->anyString(), '/', $dbr->anyString() );
395378 }
396379 }
397380
398 - if ( count( $mostused ) ) {
399 - $conds['page_title'] = $mostused;
400 - } elseif ( $code !== $wgLanguageCode ) {
401 - $conds[] = 'page_title' . $dbr->buildLike( $dbr->anyString(), "/$code" );
402 - } else {
403 - # Effectively disallows use of '/' character in NS_MEDIAWIKI for uses
404 - # other than language code.
405 - $conds[] = 'page_title NOT' . $dbr->buildLike( $dbr->anyString(), '/', $dbr->anyString() );
406 - }
407 -
408381 # Conditions to fetch oversized pages to ignore them
409382 $bigConds = $conds;
410383 $bigConds[] = 'page_len > ' . intval( $wgMaxMsgCacheEntrySize );
@@ -431,12 +404,6 @@
432405 $cache[$row->page_title] = ' ' . Revision::getRevisionText( $row );
433406 }
434407
435 - foreach ( $mostused as $key ) {
436 - if ( !isset( $cache[$key] ) ) {
437 - $cache[$key] = '!NONEXISTENT';
438 - }
439 - }
440 -
441408 $cache['VERSION'] = MSG_CACHE_VERSION;
442409 wfProfileOut( __METHOD__ );
443410 return $cache;
@@ -608,13 +575,6 @@
609576 $uckey = $wgContLang->ucfirst( $lckey );
610577 }
611578
612 - /**
613 - * Record each message request, but only once per request.
614 - * This information is not used unless $wgAdaptiveMessageCache
615 - * is enabled.
616 - */
617 - $this->mRequestedMessages[$uckey] = true;
618 -
619579 # Try the MediaWiki namespace
620580 if( !$this->mDisable && $useDB ) {
621581 $title = $uckey;
@@ -679,7 +639,8 @@
680640 * @param $code String: code denoting the language to try.
681641 */
682642 function getMsgFromNamespace( $title, $code ) {
683 - global $wgAdaptiveMessageCache;
 643+ $type = false;
 644+ $message = false;
684645
685646 $this->load( $code );
686647 if ( isset( $this->mCache[$code][$title] ) ) {
@@ -688,26 +649,13 @@
689650 return substr( $entry, 1 );
690651 } elseif ( $entry === '!NONEXISTENT' ) {
691652 return false;
692 - } elseif( $entry === '!TOO BIG' ) {
693 - // Fall through and try invididual message cache below
694653 }
695 - } else {
696 - // XXX: This is not cached in process cache, should it?
697 - $message = false;
698 - wfRunHooks( 'MessagesPreLoad', array( $title, &$message ) );
699 - if ( $message !== false ) {
700 - return $message;
701 - }
 654+ }
702655
703 - /**
704 - * If message cache is in normal mode, it is guaranteed
705 - * (except bugs) that there is always entry (or placeholder)
706 - * in the cache if message exists. Thus we can do minor
707 - * performance improvement and return false early.
708 - */
709 - if ( !$wgAdaptiveMessageCache ) {
710 - return false;
711 - }
 656+ # Call message hooks, in case they are defined
 657+ wfRunHooks('MessagesPreLoad', array( $title, &$message ) );
 658+ if ( $message !== false ) {
 659+ return $message;
712660 }
713661
714662 # Try the individual message cache
@@ -733,7 +681,6 @@
734682 $this->mCache[$code][$title] = ' ' . $message;
735683 $this->mMemc->set( $titleKey, ' ' . $message, $this->mExpiry );
736684 } else {
737 - $message = false;
738685 $this->mCache[$code][$title] = '!NONEXISTENT';
739686 $this->mMemc->set( $titleKey, '!NONEXISTENT', $this->mExpiry );
740687 }
@@ -868,77 +815,4 @@
869816 return array( $message, $lang );
870817 }
871818
872 - public static function logMessages() {
873 - wfProfileIn( __METHOD__ );
874 - global $wgAdaptiveMessageCache;
875 - if ( !$wgAdaptiveMessageCache || !self::$instance instanceof MessageCache ) {
876 - wfProfileOut( __METHOD__ );
877 - return;
878 - }
879 -
880 - $cachekey = wfMemckey( 'message-profiling' );
881 - $cache = wfGetCache( CACHE_DB );
882 - $data = $cache->get( $cachekey );
883 -
884 - if ( !$data ) {
885 - $data = array();
886 - }
887 -
888 - $age = self::$mAdaptiveDataAge;
889 - $filterDate = substr( wfTimestamp( TS_MW, time() - $age ), 0, 8 );
890 - foreach ( array_keys( $data ) as $key ) {
891 - if ( $key < $filterDate ) {
892 - unset( $data[$key] );
893 - }
894 - }
895 -
896 - $index = substr( wfTimestampNow(), 0, 8 );
897 - if ( !isset( $data[$index] ) ) {
898 - $data[$index] = array();
899 - }
900 -
901 - foreach ( self::$instance->mRequestedMessages as $message => $_ ) {
902 - if ( !isset( $data[$index][$message] ) ) {
903 - $data[$index][$message] = 0;
904 - }
905 - $data[$index][$message]++;
906 - }
907 -
908 - $cache->set( $cachekey, $data );
909 - wfProfileOut( __METHOD__ );
910 - }
911 -
912 - public function getMostUsedMessages() {
913 - wfProfileIn( __METHOD__ );
914 - $cachekey = wfMemcKey( 'message-profiling' );
915 - $cache = wfGetCache( CACHE_DB );
916 - $data = $cache->get( $cachekey );
917 - if ( !$data ) {
918 - wfProfileOut( __METHOD__ );
919 - return array();
920 - }
921 -
922 - $list = array();
923 -
924 - foreach( $data as $messages ) {
925 - foreach( $messages as $message => $count ) {
926 - $key = $message;
927 - if ( !isset( $list[$key] ) ) {
928 - $list[$key] = 0;
929 - }
930 - $list[$key] += $count;
931 - }
932 - }
933 -
934 - $max = max( $list );
935 - foreach ( $list as $message => $count ) {
936 - if ( $count < intval( $max * self::$mAdaptiveInclusionThreshold ) ) {
937 - unset( $list[$message] );
938 - }
939 - }
940 -
941 - wfProfileOut( __METHOD__ );
942 - return array_keys( $list );
943 - }
944 -
945819 }
Property changes on: branches/REL1_18/phase3/includes/cache/MessageCache.php
___________________________________________________________________
Added: svn:mergeinfo
946820 Merged /branches/new-installer/phase3/includes/cache/MessageCache.php:r43664-66004
947821 Merged /branches/wmf-deployment/includes/cache/MessageCache.php:r53381
948822 Merged /branches/REL1_17/phase3/includes/MessageCache.php:r81444
949823 Merged /branches/REL1_15/phase3/includes/cache/MessageCache.php:r51646
950824 Merged /branches/REL1_17/phase3/includes/cache/MessageCache.php:r81444
951825 Merged /branches/sqlite/includes/cache/MessageCache.php:r58211-58321
Index: branches/REL1_18/phase3/includes/DefaultSettings.php
@@ -1566,13 +1566,6 @@
15671567 $wgLocalMessageCacheSerialized = true;
15681568
15691569 /**
1570 - * Instead of caching everything, keep track which messages are requested and
1571 - * load only most used messages. This only makes sense if there is lots of
1572 - * interface messages customised in the wiki (like hundreds in many languages).
1573 - */
1574 -$wgAdaptiveMessageCache = false;
1575 -
1576 -/**
15771570 * Localisation cache configuration. Associative array with keys:
15781571 * class: The class to use. May be overridden by extensions.
15791572 *

Follow-up revisions

RevisionCommit summaryAuthorDate
r99065Revert r92549, botched feature removal in REL1_18 causing a severe performanc...tstarling00:54, 6 October 2011
r99066Revert r92549, botched feature removal in REL1_18 causing a severe performanc...tstarling01:00, 6 October 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r71412Added $wgAdaptiveMessageCache to avoid caching huge pile of never used messag...nikerabbit16:41, 21 August 2010
r71418Follow up r71412. $wgContLang is not used.platonides20:47, 21 August 2010
r74177Use 7*24*3600 instead of 604800....siebrand23:23, 2 October 2010
r74179Fix broken r74177 because it needs a constant and address CR comment on r7141...siebrand23:34, 2 October 2010
r814441.17: Back out r71412 (adaptive message cache) and its followups r71418, r741...catrope11:58, 3 February 2011

Comments

#Comment by Nikerabbit (talk | contribs)   05:48, 20 July 2011
-			if ( !$wgAdaptiveMessageCache ) {
-				return false;
-			}

That return false should be there in else clause for the if above. Same bug is in 1.17 which increases calls to underlying cache system many times.

#Comment by Catrope (talk | contribs)   18:10, 20 July 2011

I don't quite understand what you mean. Could you change this in REL1_17 and REL1_18 yourself, or otherwise like send me a patch or something?

#Comment by Tim Starling (talk | contribs)   00:37, 6 October 2011

Please set a "fixme" status if you see bugs like this. This went live to Wikimedia and caused a serious performance regression.

#Comment by Tim Starling (talk | contribs)   01:06, 6 October 2011

Reverted. If this feature needs to be removed, then it should be done in trunk, not in every release branch, and the removal should go through normal code review processes.

Status & tagging log