objectcache: Make HashBagOStuff LRU instead of least-recently-set

Refresh key in get() in addition to just set().

Change-Id: I7985b98b6a346eaed8bf0a7349b95fabea8e8614

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index c1ee065..7b85d92 100644 (file)
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -69,6 +69,11 @@ class HashBagOStuff extends BagOStuff {
                        return false;
                }
 
+               // Refresh key position for maxCacheKeys eviction
+               $temp = $this->bag[$key];
+               unset( $this->bag[$key] );
+               $this->bag[$key] = $temp;
+
                return $this->bag[$key][self::KEY_VAL];
        }
 

diff --git a/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
index 39b84e1..5194e8d 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/HashBagOStuffTest.php
@@ -27,19 +27,6 @@ class HashBagOStuffTest extends PHPUnit_Framework_TestCase {
                }
        }
 
-       public function testEvictionOrder() {
-               $cache = new HashBagOStuff( array( 'maxKeys' => 10 ) );
-               for ( $i = 0; $i < 10; $i++ ) {
-                       $cache->set( "key$i", 1 );
-                       $this->assertEquals( 1, $cache->get( "key$i" ) );
-               }
-               for ( $i = 10; $i < 20; $i++ ) {
-                       $cache->set( "key$i", 1 );
-                       $this->assertEquals( 1, $cache->get( "key$i" ) );
-                       $this->assertEquals( false, $cache->get( "key" . $i - 10 ) );
-               }
-       }
-
        public function testExpire() {
                $cache = new HashBagOStuff();
                $cacheInternal = TestingAccessWrapper::newFromObject( $cache );
@@ -56,7 +43,27 @@ class HashBagOStuffTest extends PHPUnit_Framework_TestCase {
                $this->assertEquals( false, $cache->get( 'baz' ), 'Key expired' );
        }
 
-       public function testKeyOrder() {
+       /**
+        * Ensure maxKeys eviction prefers keeping new keys.
+        */
+       public function testEvictionAdd() {
+               $cache = new HashBagOStuff( array( 'maxKeys' => 10 ) );
+               for ( $i = 0; $i < 10; $i++ ) {
+                       $cache->set( "key$i", 1 );
+                       $this->assertEquals( 1, $cache->get( "key$i" ) );
+               }
+               for ( $i = 10; $i < 20; $i++ ) {
+                       $cache->set( "key$i", 1 );
+                       $this->assertEquals( 1, $cache->get( "key$i" ) );
+                       $this->assertEquals( false, $cache->get( "key" . $i - 10 ) );
+               }
+       }
+
+       /**
+        * Ensure maxKeys eviction prefers recently set keys
+        * even if the keys pre-exist.
+        */
+       public function testEvictionSet() {
                $cache = new HashBagOStuff( array( 'maxKeys' => 3 ) );
 
                foreach ( array( 'foo', 'bar', 'baz' ) as $key ) {
@@ -75,4 +82,27 @@ class HashBagOStuffTest extends PHPUnit_Framework_TestCase {
                }
                $this->assertEquals( false, $cache->get( 'bar' ), 'Evicted bar' );
        }
+
+       /**
+        * Ensure maxKeys eviction prefers recently retrieved keys (LRU).
+        */
+       public function testEvictionGet() {
+               $cache = new HashBagOStuff( array( 'maxKeys' => 3 ) );
+
+               foreach ( array( 'foo', 'bar', 'baz' ) as $key ) {
+                       $cache->set( $key, 1 );
+               }
+
+               // Get existing key
+               $cache->get( 'foo', 1 );
+
+               // Add a 4th key (beyond the allowed maximum)
+               $cache->set( 'quux', 1 );
+
+               // Foo's life should have been extended over Bar
+               foreach ( array( 'foo', 'baz', 'quux' ) as $key ) {
+                       $this->assertEquals( 1, $cache->get( $key ), "Kept $key" );
+               }
+               $this->assertEquals( false, $cache->get( 'bar' ), 'Evicted bar' );
+       }
 }