Merge "RCFilters: Log performance data"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 26 Sep 2017 17:25:47 +0000 (17:25 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 26 Sep 2017 17:25:48 +0000 (17:25 +0000)
includes/libs/MapCacheLRU.php
includes/libs/filebackend/SwiftFileBackend.php
includes/specialpage/ChangesListSpecialPage.php
includes/specials/SpecialRecentchanges.php
resources/src/mediawiki.special/mediawiki.special.css
tests/phpunit/includes/libs/MapCacheLRUTest.php [new file with mode: 0644]

index c92769f..553315f 100644 (file)
@@ -48,16 +48,45 @@ class MapCacheLRU {
                $this->maxCacheKeys = $maxKeys;
        }
 
+       /**
+        * @param array $values Key/value map in order of increasingly recent access
+        * @param int $maxKeys
+        * @return MapCacheLRU
+        * @since 1.30
+        */
+       public static function newFromArray( array $values, $maxKeys ) {
+               $mapCache = new self( $maxKeys );
+               $mapCache->cache = ( count( $values ) > $maxKeys )
+                       ? array_slice( $values, -$maxKeys, null, true )
+                       : $values;
+
+               return $mapCache;
+       }
+
+       /**
+        * @return array Key/value map in order of increasingly recent access
+        * @since 1.30
+        */
+       public function toArray() {
+               return $this->cache;
+       }
+
        /**
         * Set a key/value pair.
         * This will prune the cache if it gets too large based on LRU.
         * If the item is already set, it will be pushed to the top of the cache.
         *
+        * To reduce evictions due to one-off use of many new keys, $rank can be
+        * set to have keys start at the top of a bottom fraction of the list. A value
+        * of 3/8 means values start at the top of the bottom 3/8s of the list and are
+        * moved to the top of the list when accessed a second time.
+        *
         * @param string $key
         * @param mixed $value
+        * @param float $rank Bottom fraction of the list where keys start off [Default: 1.0]
         * @return void
         */
-       public function set( $key, $value ) {
+       public function set( $key, $value, $rank = 1.0 ) {
                if ( $this->has( $key ) ) {
                        $this->ping( $key );
                } elseif ( count( $this->cache ) >= $this->maxCacheKeys ) {
@@ -65,7 +94,15 @@ class MapCacheLRU {
                        $evictKey = key( $this->cache );
                        unset( $this->cache[$evictKey] );
                }
-               $this->cache[$key] = $value;
+
+               if ( $rank < 1.0 && $rank > 0 ) {
+                       $offset = intval( $rank * count( $this->cache ) );
+                       $this->cache = array_slice( $this->cache, 0, $offset, true )
+                               + [ $key => $value ]
+                               + array_slice( $this->cache, $offset, null, true );
+               } else {
+                       $this->cache[$key] = $value;
+               }
        }
 
        /**
@@ -116,15 +153,16 @@ class MapCacheLRU {
         * @since 1.28
         * @param string $key
         * @param callable $callback Callback that will produce the value
+        * @param float $rank Bottom fraction of the list where keys start off [Default: 1.0]
         * @return mixed The cached value if found or the result of $callback otherwise
         */
-       public function getWithSetCallback( $key, callable $callback ) {
+       public function getWithSetCallback( $key, callable $callback, $rank = 1.0 ) {
                if ( $this->has( $key ) ) {
                        $value = $this->get( $key );
                } else {
                        $value = call_user_func( $callback );
                        if ( $value !== false ) {
-                               $this->set( $key, $value );
+                               $this->set( $key, $value, $rank );
                        }
                }
 
index de5a103..4212ff5 100644 (file)
@@ -50,6 +50,10 @@ class SwiftFileBackend extends FileBackendStore {
        protected $rgwS3AccessKey;
        /** @var string S3 authentication key (RADOS Gateway) */
        protected $rgwS3SecretKey;
+       /** @var array Additional users (account:user) to open read permissions for */
+       protected $readUsers;
+       /** @var array Additional users (account:user) to open write permissions for */
+       protected $writeUsers;
 
        /** @var BagOStuff */
        protected $srvCache;
@@ -96,6 +100,8 @@ class SwiftFileBackend extends FileBackendStore {
         *                          This is used for generating expiring pre-authenticated URLs.
         *                          Only use this when using rgw and to work around
         *                          http://tracker.newdream.net/issues/3454.
+        *   - readUsers           : Swift users that should have read access (account:username)
+        *   - writeUsers          : Swift users that should have write access (account:username)
         */
        public function __construct( array $config ) {
                parent::__construct( $config );
@@ -136,6 +142,12 @@ class SwiftFileBackend extends FileBackendStore {
                } else {
                        $this->srvCache = new EmptyBagOStuff();
                }
+               $this->readUsers = isset( $config['readUsers'] )
+                       ? $config['readUsers']
+                       : [];
+               $this->writeUsers = isset( $config['writeUsers'] )
+                       ? $config['writeUsers']
+                       : [];
        }
 
        public function getFeatures() {
@@ -590,11 +602,13 @@ class SwiftFileBackend extends FileBackendStore {
 
                $stat = $this->getContainerStat( $fullCont );
                if ( is_array( $stat ) ) {
+                       $readUsers = array_merge( $this->readUsers, [ $this->swiftUser ] );
+                       $writeUsers = array_merge( $this->writeUsers, [ $this->swiftUser ] );
                        // Make container private to end-users...
                        $status->merge( $this->setContainerAccess(
                                $fullCont,
-                               [ $this->swiftUser ], // read
-                               [ $this->swiftUser ] // write
+                               $readUsers,
+                               $writeUsers
                        ) );
                } elseif ( $stat === false ) {
                        $status->fatal( 'backend-fail-usable', $params['dir'] );
@@ -611,11 +625,14 @@ class SwiftFileBackend extends FileBackendStore {
 
                $stat = $this->getContainerStat( $fullCont );
                if ( is_array( $stat ) ) {
+                       $readUsers = array_merge( $this->readUsers, [ $this->swiftUser, '.r:*' ] );
+                       $writeUsers = array_merge( $this->writeUsers, [ $this->swiftUser ] );
+
                        // Make container public to end-users...
                        $status->merge( $this->setContainerAccess(
                                $fullCont,
-                               [ $this->swiftUser, '.r:*' ], // read
-                               [ $this->swiftUser ] // write
+                               $readUsers,
+                               $writeUsers
                        ) );
                } elseif ( $stat === false ) {
                        $status->fatal( 'backend-fail-usable', $params['dir'] );
@@ -1309,7 +1326,7 @@ class SwiftFileBackend extends FileBackendStore {
         * (lists are truncated to 10000 item with no way to page), and is just a performance risk.
         *
         * @param string $container Resolved Swift container
-        * @param array $readGrps List of the possible criteria for a request to have
+        * @param array $readUsers List of the possible criteria for a request to have
         * access to read a container. Each item is one of the following formats:
         *   - account:user        : Grants access if the request is by the given user
         *   - ".r:<regex>"        : Grants access if the request is from a referrer host that
@@ -1317,12 +1334,12 @@ class SwiftFileBackend extends FileBackendStore {
         *                           Setting this to '*' effectively makes a container public.
         *   -".rlistings:<regex>" : Grants access if the request is from a referrer host that
         *                           matches the expression and the request is for a listing.
-        * @param array $writeGrps A list of the possible criteria for a request to have
+        * @param array $writeUsers A list of the possible criteria for a request to have
         * access to write to a container. Each item is of the following format:
         *   - account:user       : Grants access if the request is by the given user
         * @return StatusValue
         */
-       protected function setContainerAccess( $container, array $readGrps, array $writeGrps ) {
+       protected function setContainerAccess( $container, array $readUsers, array $writeUsers ) {
                $status = $this->newStatus();
                $auth = $this->getAuthentication();
 
@@ -1336,8 +1353,8 @@ class SwiftFileBackend extends FileBackendStore {
                        'method' => 'POST',
                        'url' => $this->storageUrl( $auth, $container ),
                        'headers' => $this->authTokenHeaders( $auth ) + [
-                               'x-container-read' => implode( ',', $readGrps ),
-                               'x-container-write' => implode( ',', $writeGrps )
+                               'x-container-read' => implode( ',', $readUsers ),
+                               'x-container-write' => implode( ',', $writeUsers )
                        ]
                ] );
 
@@ -1420,18 +1437,19 @@ class SwiftFileBackend extends FileBackendStore {
 
                // @see SwiftFileBackend::setContainerAccess()
                if ( empty( $params['noAccess'] ) ) {
-                       $readGrps = [ '.r:*', $this->swiftUser ]; // public
+                       $readUsers = array_merge( $this->readUsers, [ '.r:*', $this->swiftUser ] ); // public
                } else {
-                       $readGrps = [ $this->swiftUser ]; // private
+                       $readUsers = array_merge( $this->readUsers, [ $this->swiftUser ] ); // private
                }
-               $writeGrps = [ $this->swiftUser ]; // sanity
+
+               $writeUsers = array_merge( $this->writeUsers, [ $this->swiftUser ] ); // sanity
 
                list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $this->http->run( [
                        'method' => 'PUT',
                        'url' => $this->storageUrl( $auth, $container ),
                        'headers' => $this->authTokenHeaders( $auth ) + [
-                               'x-container-read' => implode( ',', $readGrps ),
-                               'x-container-write' => implode( ',', $writeGrps )
+                               'x-container-read' => implode( ',', $readUsers ),
+                               'x-container-write' => implode( ',', $writeUsers )
                        ]
                ] );
 
index 6fd33a7..88ec327 100644 (file)
@@ -619,7 +619,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
                        $out->addJsConfigVars(
                                'wgRCFiltersChangeTags',
-                               $this->buildChangeTagList()
+                               $this->getChangeTagList()
                        );
                        $out->addJsConfigVars(
                                'StructuredChangeFiltersDisplayConfig',
@@ -661,49 +661,60 @@ abstract class ChangesListSpecialPage extends SpecialPage {
         *
         * @return Array Tag data
         */
-       protected function buildChangeTagList() {
-               $explicitlyDefinedTags = array_fill_keys( ChangeTags::listExplicitlyDefinedTags(), 0 );
-               $softwareActivatedTags = array_fill_keys( ChangeTags::listSoftwareActivatedTags(), 0 );
-
-               // Hit counts disabled for perf reasons, see T169997
-               /*
-               $tagStats = ChangeTags::tagUsageStatistics();
-               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags, $tagStats );
-
-               // Sort by hits
-               arsort( $tagHitCounts );
-               */
-               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags );
-
-               // Build the list and data
-               $result = [];
-               foreach ( $tagHitCounts as $tagName => $hits ) {
-                       if (
-                               // Only get active tags
-                               isset( $explicitlyDefinedTags[ $tagName ] ) ||
-                               isset( $softwareActivatedTags[ $tagName ] )
-                       ) {
-                               // Parse description
-                               $desc = ChangeTags::tagLongDescriptionMessage( $tagName, $this->getContext() );
-
-                               $result[] = [
-                                       'name' => $tagName,
-                                       'label' => Sanitizer::stripAllTags(
-                                               ChangeTags::tagDescription( $tagName, $this->getContext() )
-                                       ),
-                                       'description' => $desc ? Sanitizer::stripAllTags( $desc->parse() ) : '',
-                                       'cssClass' => Sanitizer::escapeClass( 'mw-tag-' . $tagName ),
-                                       'hits' => $hits,
-                               ];
-                       }
-               }
+       protected function getChangeTagList() {
+               $cache = ObjectCache::getMainWANInstance();
+               $context = $this->getContext();
+               return $cache->getWithSetCallback(
+                       $cache->makeKey( 'changeslistspecialpage-changetags', $context->getLanguage()->getCode() ),
+                       $cache::TTL_MINUTE * 10,
+                       function () use ( $context ) {
+                               $explicitlyDefinedTags = array_fill_keys( ChangeTags::listExplicitlyDefinedTags(), 0 );
+                               $softwareActivatedTags = array_fill_keys( ChangeTags::listSoftwareActivatedTags(), 0 );
+
+                               // Hit counts disabled for perf reasons, see T169997
+                               /*
+                               $tagStats = ChangeTags::tagUsageStatistics();
+                               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags, $tagStats );
+
+                               // Sort by hits
+                               arsort( $tagHitCounts );
+                               */
+                               $tagHitCounts = array_merge( $explicitlyDefinedTags, $softwareActivatedTags );
+
+                               // Build the list and data
+                               $result = [];
+                               foreach ( $tagHitCounts as $tagName => $hits ) {
+                                       if (
+                                               // Only get active tags
+                                               isset( $explicitlyDefinedTags[ $tagName ] ) ||
+                                               isset( $softwareActivatedTags[ $tagName ] )
+                                       ) {
+                                               // Parse description
+                                               $desc = ChangeTags::tagLongDescriptionMessage( $tagName, $context );
+
+                                               $result[] = [
+                                                       'name' => $tagName,
+                                                       'label' => Sanitizer::stripAllTags(
+                                                               ChangeTags::tagDescription( $tagName, $context )
+                                                       ),
+                                                       'description' => $desc ? Sanitizer::stripAllTags( $desc->parse() ) : '',
+                                                       'cssClass' => Sanitizer::escapeClass( 'mw-tag-' . $tagName ),
+                                                       'hits' => $hits,
+                                               ];
+                                       }
+                               }
 
-               // Instead of sorting by hit count (disabled, see above), sort by display name
-               usort( $result, function ( $a, $b ) {
-                       return strcasecmp( $a['label'], $b['label'] );
-               } );
+                               // Instead of sorting by hit count (disabled, see above), sort by display name
+                               usort( $result, function ( $a, $b ) {
+                                       return strcasecmp( $a['label'], $b['label'] );
+                               } );
 
-               return $result;
+                               return $result;
+                       },
+                       [
+                               'lockTSE' => 30
+                       ]
+               );
        }
 
        /**
index 522a0a6..6ce1968 100644 (file)
@@ -620,7 +620,8 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
                        if ( $this->isStructuredFilterUiEnabled() ) {
                                // Check whether the widget is already collapsed or expanded
                                $collapsedState = $this->getRequest()->getCookie( 'rcfilters-toplinks-collapsed-state' );
-                               $collapsedClass = $collapsedState === 'collapsed' ? 'mw-rcfilters-toplinks-collapsed' : '';
+                               // Note that an empty/unset cookie means collapsed, so check for !== 'expanded'
+                               $collapsedClass = $collapsedState !== 'expanded' ? 'mw-rcfilters-toplinks-collapsed' : '';
 
                                $contentTitle = Html::rawElement( 'div',
                                        [ 'class' => 'mw-recentchanges-toplinks-title ' . $collapsedClass ],
index ed7dc22..9ea8019 100644 (file)
@@ -1,17 +1,17 @@
 /* Special:AllMessages */
-#mw-allmessagestable .allmessages-customised td.am_default {
+#mw-allmessagestable .allmessages-customised .am_default {
        background-color: #fcffc4;
 }
 
-#mw-allmessagestable tr.allmessages-customised:hover td.am_default {
+#mw-allmessagestable .allmessages-customised:hover .am_default {
        background-color: #faff90;
 }
 
-#mw-allmessagestable td.am_actual {
+#mw-allmessagestable .am_actual {
        background-color: #e2ffe2;
 }
 
-#mw-allmessagestable tr.allmessages-customised:hover + tr.allmessages-customised td.am_actual {
+#mw-allmessagestable .allmessages-customised:hover + .allmessages-customised .am_actual {
        background-color: #b1ffb1;
 }
 
@@ -30,7 +30,7 @@
 }
 
 /* Special:Block */
-p.mw-ipb-conveniencelinks {
+.mw-ipb-conveniencelinks {
        font-size: 90%;
        text-align: right;
 }
@@ -39,13 +39,13 @@ label[for='mw-input-wpConfirm'] {
        font-weight: bold;
 }
 
-tr.mw-block-hideuser {
+.mw-block-hideuser {
        font-weight: bold;
 }
 
 /* Special:BlockList */
-table.mw-blocklist span.mw-usertoollinks,
-span.mw-blocklist-actions {
+.mw-blocklist .mw-usertoollinks,
+.mw-blocklist-actions {
        white-space: nowrap;
        font-size: 90%;
 }
@@ -64,8 +64,8 @@ span.mw-blocklist-actions {
 }
 
 /* Special:EmailUser */
-td#mw-emailuser-sender,
-td#mw-emailuser-recipient {
+#mw-emailuser-sender,
+#mw-emailuser-recipient {
        font-weight: bold;
 }
 
@@ -75,7 +75,7 @@ td#mw-emailuser-recipient {
 }
 
 /* Special:ListGroupRights */
-table.mw-listgrouprights-table tr {
+.mw-listgrouprights-table tr {
        vertical-align: top;
 }
 .listgrouprights-revoked {
@@ -83,7 +83,7 @@ table.mw-listgrouprights-table tr {
 }
 
 /* Special:RevisionDelete */
-p.mw-revdel-editreasons {
+.mw-revdel-editreasons {
        font-size: 90%;
        text-align: right;
 }
@@ -100,18 +100,18 @@ p.mw-revdel-editreasons {
 }
 
 /* Special:Statistics */
-td.mw-statistics-numbers {
+.mw-statistics-numbers {
        text-align: right;
 }
 
 /* Special:ProtectedPages */
-table.mw-protectedpages span.mw-usertoollinks,
-span.mw-protectedpages-length,
-span.mw-protectedpages-actions {
+.mw-protectedpages .mw-usertoollinks,
+.mw-protectedpages-length,
+.mw-protectedpages-actions {
        white-space: nowrap;
        font-size: 90%;
 }
-span.mw-protectedpages-unknown {
+.mw-protectedpages-unknown {
        color: #72777d;
        font-size: 90%;
 }
@@ -120,11 +120,11 @@ span.mw-protectedpages-unknown {
 .mw-userrights-disabled {
        color: #72777d;
 }
-table.mw-userrights-groups * td,
-table.mw-userrights-groups * th {
+.mw-userrights-groups * td,
+.mw-userrights-groups * th {
        padding-right: 1.5em;
 }
 
-table.mw-userrights-groups * th {
+.mw-userrights-groups * th {
        text-align: left;
 }
diff --git a/tests/phpunit/includes/libs/MapCacheLRUTest.php b/tests/phpunit/includes/libs/MapCacheLRUTest.php
new file mode 100644 (file)
index 0000000..60a5057
--- /dev/null
@@ -0,0 +1,114 @@
+<?php
+/**
+ * @group Cache
+ */
+class MapCacheLRUTest extends PHPUnit_Framework_TestCase {
+       /**
+        * @covers MapCacheLRU::newFromArray()
+        * @covers MapCacheLRU::toArray()
+        * @covers MapCacheLRU::getAllKeys()
+        * @covers MapCacheLRU::clear()
+        */
+       function testArrayConversion() {
+               $raw = [ 'd' => 4, 'c' => 3, 'b' => 2, 'a' => 1 ];
+               $cache = MapCacheLRU::newFromArray( $raw, 3 );
+
+               $this->assertSame( true, $cache->has( 'a' ) );
+               $this->assertSame( true, $cache->has( 'b' ) );
+               $this->assertSame( true, $cache->has( 'c' ) );
+               $this->assertSame( 1, $cache->get( 'a' ) );
+               $this->assertSame( 2, $cache->get( 'b' ) );
+               $this->assertSame( 3, $cache->get( 'c' ) );
+
+               $this->assertSame(
+                       [ 'a' => 1, 'b' => 2, 'c' => 3 ],
+                       $cache->toArray()
+               );
+               $this->assertSame(
+                       [ 'a', 'b', 'c' ],
+                       $cache->getAllKeys()
+               );
+
+               $cache->clear( 'a' );
+               $this->assertSame(
+                       [ 'b' => 2, 'c' => 3 ],
+                       $cache->toArray()
+               );
+
+               $cache->clear();
+               $this->assertSame(
+                       [],
+                       $cache->toArray()
+               );
+       }
+
+       /**
+        * @covers MapCacheLRU::has()
+        * @covers MapCacheLRU::get()
+        * @covers MapCacheLRU::set()
+        */
+       function testLRU() {
+               $raw = [ 'a' => 1, 'b' => 2, 'c' => 3 ];
+               $cache = MapCacheLRU::newFromArray( $raw, 3 );
+
+               $this->assertSame( true, $cache->has( 'c' ) );
+               $this->assertSame(
+                       [ 'a' => 1, 'b' => 2, 'c' => 3 ],
+                       $cache->toArray()
+               );
+
+               $this->assertSame( 3, $cache->get( 'c' ) );
+               $this->assertSame(
+                       [ 'a' => 1, 'b' => 2, 'c' => 3 ],
+                       $cache->toArray()
+               );
+
+               $this->assertSame( 1, $cache->get( 'a' ) );
+               $this->assertSame(
+                       [ 'b' => 2, 'c' => 3, 'a' => 1 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'a', 1 );
+               $this->assertSame(
+                       [ 'b' => 2, 'c' => 3, 'a' => 1 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'b', 22 );
+               $this->assertSame(
+                       [ 'c' => 3, 'a' => 1, 'b' => 22 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'd', 4 );
+               $this->assertSame(
+                       [ 'a' => 1, 'b' => 22, 'd' => 4 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'e', 5, 0.33 );
+               $this->assertSame(
+                       [ 'e' => 5, 'b' => 22, 'd' => 4 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'f', 6, 0.66 );
+               $this->assertSame(
+                       [ 'b' => 22, 'f' => 6, 'd' => 4 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'g', 7, 0.90 );
+               $this->assertSame(
+                       [ 'f' => 6, 'g' => 7, 'd' => 4 ],
+                       $cache->toArray()
+               );
+
+               $cache->set( 'g', 7, 1.0 );
+               $this->assertSame(
+                       [ 'f' => 6, 'd' => 4, 'g' => 7 ],
+                       $cache->toArray()
+               );
+       }
+}