RCFilters: Reduce startup overhead from 'config.json' computation
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 13 Jun 2019 20:23:28 +0000 (21:23 +0100)
committerRoan Kattouw <roan.kattouw@gmail.com>
Sat, 29 Jun 2019 04:49:49 +0000 (21:49 -0700)
Follows-up 9ba1d8f526e1. Use the new 'versionCallback' to only perform
the minimum amount of work needed to detect changes, without fully
transforming the data for delivery.

Factor out message parsing into a separate step in the tag list code,
and use the messages' raw contents for the versionCallback.

Stop using WANCache for the tag list, since it's now cached by
ResourceLoader, and only regenerated when needed.

Also refactor the ChangeTags functions around tag description messages a
bit, so that we can more easily get the message keys that are going to
be used.

Bug: T201574
Bug: T223260
Change-Id: I02082aeb289ce4156170b14b8840f6d92cbadb57

includes/changetags/ChangeTags.php
includes/specialpage/ChangesListSpecialPage.php
resources/Resources.php

index 7466f82..f09bafc 100644 (file)
@@ -133,23 +133,23 @@ class ChangeTags {
        }
 
        /**
-        * Get a short description for a tag.
+        * Get the message object for the tag's short description.
         *
         * Checks if message key "mediawiki:tag-$tag" exists. If it does not,
-        * returns the HTML-escaped tag name. Uses the message if the message
-        * exists, provided it is not disabled. If the message is disabled,
-        * we consider the tag hidden, and return false.
+        * returns the tag name in a RawMessage. If the message exists, it is
+        * used, provided it is not disabled. If the message is disabled, we
+        * consider the tag hidden, and return false.
         *
+        * @since 1.34
         * @param string $tag
         * @param MessageLocalizer $context
-        * @return string|bool Tag description or false if tag is to be hidden.
-        * @since 1.25 Returns false if tag is to be hidden.
+        * @return Message|bool Tag description, or false if tag is to be hidden.
         */
-       public static function tagDescription( $tag, MessageLocalizer $context ) {
+       public static function tagShortDescriptionMessage( $tag, MessageLocalizer $context ) {
                $msg = $context->msg( "tag-$tag" );
                if ( !$msg->exists() ) {
-                       // No such message, so return the HTML-escaped tag name.
-                       return htmlspecialchars( $tag );
+                       // No such message
+                       return new RawMessage( '$1', [ Message::plaintextParam( $tag ) ] );
                }
                if ( $msg->isDisabled() ) {
                        // The message exists but is disabled, hide the tag.
@@ -157,7 +157,25 @@ class ChangeTags {
                }
 
                // Message exists and isn't disabled, use it.
-               return $msg->parse();
+               return $msg;
+       }
+
+       /**
+        * Get a short description for a tag.
+        *
+        * Checks if message key "mediawiki:tag-$tag" exists. If it does not,
+        * returns the HTML-escaped tag name. Uses the message if the message
+        * exists, provided it is not disabled. If the message is disabled,
+        * we consider the tag hidden, and return false.
+        *
+        * @param string $tag
+        * @param MessageLocalizer $context
+        * @return string|bool Tag description or false if tag is to be hidden.
+        * @since 1.25 Returns false if tag is to be hidden.
+        */
+       public static function tagDescription( $tag, MessageLocalizer $context ) {
+               $msg = self::tagShortDescriptionMessage( $tag, $context );
+               return $msg ? $msg->parse() : false;
        }
 
        /**
@@ -1468,6 +1486,7 @@ class ChangeTags {
                $cache->touchCheckKey( $cache->makeKey( 'active-tags' ) );
                $cache->touchCheckKey( $cache->makeKey( 'valid-tags-db' ) );
                $cache->touchCheckKey( $cache->makeKey( 'valid-tags-hook' ) );
+               $cache->touchCheckKey( $cache->makeKey( 'tags-usage-statistics' ) );
 
                MediaWikiServices::getInstance()->getChangeTagDefStore()->reloadMap();
        }
@@ -1479,21 +1498,35 @@ class ChangeTags {
         * @return array Array of string => int
         */
        public static function tagUsageStatistics() {
-               $dbr = wfGetDB( DB_REPLICA );
-               $res = $dbr->select(
-                       'change_tag_def',
-                       [ 'ctd_name', 'ctd_count' ],
-                       [],
-                       __METHOD__,
-                       [ 'ORDER BY' => 'ctd_count DESC' ]
-               );
+               $fname = __METHOD__;
 
-               $out = [];
-               foreach ( $res as $row ) {
-                       $out[$row->ctd_name] = $row->ctd_count;
-               }
+               $cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
+               return $cache->getWithSetCallback(
+                       $cache->makeKey( 'tags-usage-statistics' ),
+                       WANObjectCache::TTL_MINUTE * 5,
+                       function ( $oldValue, &$ttl, array &$setOpts ) use ( $fname ) {
+                               $dbr = wfGetDB( DB_REPLICA );
+                               $res = $dbr->select(
+                                       'change_tag_def',
+                                       [ 'ctd_name', 'ctd_count' ],
+                                       [],
+                                       $fname,
+                                       [ 'ORDER BY' => 'ctd_count DESC' ]
+                               );
 
-               return $out;
+                               $out = [];
+                               foreach ( $res as $row ) {
+                                       $out[$row->ctd_name] = $row->ctd_count;
+                               }
+
+                               return $out;
+                       },
+                       [
+                               'checkKeys' => [ $cache->makeKey( 'tags-usage-statistics' ) ],
+                               'lockTSE' => WANObjectCache::TTL_MINUTE * 5,
+                               'pcTTL' => WANObjectCache::TTL_PROC_LONG
+                       ]
+               );
        }
 
        /**
index 3c9abb2..f9b4542 100644 (file)
@@ -824,9 +824,27 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                }
        }
 
+       /**
+        * Get essential data about getRcFiltersConfigVars() for change detection.
+        *
+        * @internal For use by Resources.php only.
+        * @see ResourceLoaderModule::getDefinitionSummary() and ResourceLoaderModule::getVersionHash()
+        * @param ResourceLoaderContext $context
+        * @return array
+        */
+       public static function getRcFiltersConfigSummary( ResourceLoaderContext $context ) {
+               return [
+                       // Reduce version computation by avoiding Message parsing
+                       'RCFiltersChangeTags' => self::getChangeTagListSummary( $context ),
+                       'StructuredChangeFiltersEditWatchlistUrl' =>
+                               SpecialPage::getTitleFor( 'EditWatchlist' )->getLocalURL()
+               ];
+       }
+
        /**
         * Get config vars to export with the mediawiki.rcfilters.filters.ui module.
         *
+        * @internal For use by Resources.php only.
         * @param ResourceLoaderContext $context
         * @return array
         */
@@ -839,70 +857,105 @@ abstract class ChangesListSpecialPage extends SpecialPage {
        }
 
        /**
-        * Fetch the change tags list for the front end
+        * Get (cheap to compute) information about change tags.
+        *
+        * Returns an array of associative arrays with information about each tag:
+        * - name: Tag name (string)
+        * - labelMsg: Short description message (Message object)
+        * - descriptionMsg: Long description message (Message object)
+        * - cssClass: CSS class to use for RC entries with this tag
+        * - hits: Number of RC entries that have this tag
         *
         * @param ResourceLoaderContext $context
-        * @return array Tag data
+        * @return array[] Information about each tag
         */
-       protected static function getChangeTagList( ResourceLoaderContext $context ) {
-               $cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
-               return $cache->getWithSetCallback(
-                       $cache->makeKey( 'changeslistspecialpage-changetags', $context->getLanguage() ),
-                       $cache::TTL_MINUTE * 10,
-                       function () use ( $context ) {
-                               $explicitlyDefinedTags = array_fill_keys( ChangeTags::listExplicitlyDefinedTags(), 0 );
-                               $softwareActivatedTags = array_fill_keys( ChangeTags::listSoftwareActivatedTags(), 0 );
-
-                               $tagStats = ChangeTags::tagUsageStatistics();
-                               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags, $tagStats );
-
-                               // Sort by hits (disabled for now)
-                               //arsort( $tagHitCounts );
-
-                               // HACK work around ChangeTags::truncateTagDescription() requiring a RequestContext
-                               $fakeContext = RequestContext::newExtraneousContext( Title::newFromText( 'Dwimmerlaik' ) );
-                               $fakeContext->setLanguage( Language::factory( $context->getLanguage() ) );
-
-                               // Build the list and data
-                               $result = [];
-                               foreach ( $tagHitCounts as $tagName => $hits ) {
-                                       if (
-                                               (
-                                                       // Only get active tags
-                                                       isset( $explicitlyDefinedTags[ $tagName ] ) ||
-                                                       isset( $softwareActivatedTags[ $tagName ] )
-                                               ) &&
-                                               // Only get tags with more than 0 hits
-                                               $hits > 0
-                                       ) {
-                                               $result[] = [
-                                                       'name' => $tagName,
-                                                       'label' => Sanitizer::stripAllTags(
-                                                               ChangeTags::tagDescription( $tagName, $context )
-                                                       ),
-                                                       'description' =>
-                                                               ChangeTags::truncateTagDescription(
-                                                                       $tagName,
-                                                                       self::TAG_DESC_CHARACTER_LIMIT,
-                                                                       $fakeContext
-                                                               ),
-                                                       'cssClass' => Sanitizer::escapeClass( 'mw-tag-' . $tagName ),
-                                                       'hits' => $hits,
-                                               ];
-                                       }
+       protected static function getChangeTagInfo( ResourceLoaderContext $context ) {
+               $explicitlyDefinedTags = array_fill_keys( ChangeTags::listExplicitlyDefinedTags(), 0 );
+               $softwareActivatedTags = array_fill_keys( ChangeTags::listSoftwareActivatedTags(), 0 );
+
+               $tagStats = ChangeTags::tagUsageStatistics();
+               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags, $tagStats );
+
+               $result = [];
+               foreach ( $tagHitCounts as $tagName => $hits ) {
+                       if (
+                               (
+                                       // Only get active tags
+                                       isset( $explicitlyDefinedTags[ $tagName ] ) ||
+                                       isset( $softwareActivatedTags[ $tagName ] )
+                               ) &&
+                               // Only get tags with more than 0 hits
+                               $hits > 0
+                       ) {
+                               $labelMsg = ChangeTags::tagShortDescriptionMessage( $tagName, $context );
+                               if ( $labelMsg === false ) {
+                                       // Tag is hidden, skip it
+                                       continue;
                                }
+                               $result[] = [
+                                       'name' => $tagName,
+                                       // 'label' and 'description' filled in by getChangeTagList()
+                                       'labelMsg' => $labelMsg,
+                                       'descriptionMsg' => ChangeTags::tagLongDescriptionMessage( $tagName, $context ),
+                                       'cssClass' => Sanitizer::escapeClass( 'mw-tag-' . $tagName ),
+                                       'hits' => $hits,
+                               ];
+                       }
+               }
+               return $result;
+       }
 
-                               // Instead of sorting by hit count (disabled, see above), sort by display name
-                               usort( $result, function ( $a, $b ) {
-                                       return strcasecmp( $a['label'], $b['label'] );
-                               } );
+       /**
+        * Get information about change tags for use in getRcFiltersConfigSummary().
+        *
+        * This expands labelMsg and descriptionMsg to the raw values of each message, which captures
+        * changes in the messages but avoids the expensive step of parsing them.
+        *
+        * @param ResourceLoaderContext $context
+        * @return array[] Result of getChangeTagInfo(), with messages expanded to raw contents
+        */
+       protected static function getChangeTagListSummary( ResourceLoaderContext $context ) {
+               $tags = self::getChangeTagInfo( $context );
+               foreach ( $tags as &$tagInfo ) {
+                       $tagInfo['labelMsg'] = $tagInfo['labelMsg']->plain();
+                       if ( $tagInfo['descriptionMsg'] ) {
+                               $tagInfo['descriptionMsg'] = $tagInfo['descriptionMsg']->plain();
+                       }
+               }
+               return $tags;
+       }
 
-                               return $result;
-                       },
-                       [
-                               'lockTSE' => 30
-                       ]
-               );
+       /**
+        * Get information about change tags to export to JS via getRcFiltersConfigVars().
+        *
+        * This removes labelMsg and descriptionMsg, and adds label and description, which are parsed,
+        * stripped and (in the case of description) truncated versions of these messages. Message
+        * parsing is expensive, so to detect whether the tag list has changed, use
+        * getChangeTagListSummary() instead.
+        *
+        * @param ResourceLoaderContext $context
+        * @return array[] Result of getChangeTagInfo(), with messages parsed, stripped and truncated
+        */
+       protected static function getChangeTagList( ResourceLoaderContext $context ) {
+               $tags = self::getChangeTagInfo( $context );
+               $language = Language::factory( $context->getLanguage() );
+               foreach ( $tags as &$tagInfo ) {
+                       $tagInfo['label'] = Sanitizer::stripAllTags( $tagInfo['labelMsg']->parse() );
+                       $tagInfo['description'] = $tagInfo['descriptionMsg'] ?
+                               $language->truncateForVisual(
+                                       Sanitizer::stripAllTags( $tagInfo['descriptionMsg']->parse() ),
+                                       self::TAG_DESC_CHARACTER_LIMIT
+                               ) :
+                               '';
+                       unset( $tagInfo['labelMsg'] );
+                       unset( $tagInfo['descriptionMsg'] );
+               }
+
+               // Instead of sorting by hit count (disabled for now), sort by display name
+               usort( $tags, function ( $a, $b ) {
+                       return strcasecmp( $a['label'], $b['label'] );
+               } );
+               return $tags;
        }
 
        /**
index b90ead4..9b7ce26 100644 (file)
@@ -1819,7 +1819,10 @@ return [
                        'ui/RclTargetPageWidget.js',
                        'ui/RclToOrFromWidget.js',
                        'ui/WatchlistTopSectionWidget.js',
-                       [ 'name' => 'config.json', 'callback' => 'ChangesListSpecialPage::getRcFiltersConfigVars' ],
+                       [ 'name' => 'config.json',
+                               'versionCallback' => 'ChangesListSpecialPage::getRcFiltersConfigSummary',
+                               'callback' => 'ChangesListSpecialPage::getRcFiltersConfigVars',
+                       ],
                ],
                'styles' => [
                        'styles/mw.rcfilters.mixins.less',