From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 3 Nov 2015 10:06:40 +0000 (-0800)
Subject: objectcache: Make HashBagOStuff LRU instead of least-recently-set
X-Git-Tag: 1.31.0-rc.0~9109
X-Git-Url: http://git.cyclocoop.org/%22.%24image2.%22?a=commitdiff_plain;h=209217da8a372756ed1c2fdafed5f6912bc642da;p=lhc%2Fweb%2Fwiklou.git

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 c1ee065a20..7b85d92580 100644
--- 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 39b84e104f..5194e8d972 100644
--- 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' );
+	}
 }