From: Aaron Schulz Date: Sat, 30 Jun 2012 03:08:06 +0000 (-0700) Subject: [FileBackend] Factored out code into ProcessCacheLRU class. X-Git-Tag: 1.31.0-rc.0~23066 X-Git-Url: http://git.cyclocoop.org/%24action?a=commitdiff_plain;h=878f5aae5c25b29df80d0ccb125961a77fcec82f;p=lhc%2Fweb%2Fwiklou.git [FileBackend] Factored out code into ProcessCacheLRU class. Factor out duplicated code into the new ProcessCacheLRU. It is a fixed size an per process cache which invalidate the least recently used cache key. Change-Id: Ib4475f21879ef6286ce2a28f248acd296fdbd45d --- diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 11cf616fe9..6c37d1abcb 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -419,6 +419,7 @@ $wgAutoloadLocalClasses = array( 'LinkCache' => 'includes/cache/LinkCache.php', 'MessageCache' => 'includes/cache/MessageCache.php', 'ObjectFileCache' => 'includes/cache/ObjectFileCache.php', + 'ProcessCacheLRU' => 'includes/cache/ProcessCacheLRU.php', 'ResourceFileCache' => 'includes/cache/ResourceFileCache.php', 'SquidUpdate' => 'includes/cache/SquidUpdate.php', 'TitleDependency' => 'includes/cache/CacheDependency.php', diff --git a/includes/cache/ProcessCacheLRU.php b/includes/cache/ProcessCacheLRU.php new file mode 100644 index 0000000000..a9bcd9beac --- /dev/null +++ b/includes/cache/ProcessCacheLRU.php @@ -0,0 +1,120 @@ + prop => value) + + protected $maxCacheKeys; // integer; max entries + + /** + * @param $maxEntries integer Maximum number of entries allowed (min 1). + * @throws MWException When $maxCacheKeys is not an int or =< 0. + */ + public function __construct( $maxKeys ) { + if ( !is_int( $maxKeys ) || $maxKeys < 1 ) { + throw new MWException( __METHOD__ . " must be given an integer and >= 1" ); + } + $this->maxCacheKeys = $maxKeys; + } + + /** + * Set a property field for a cache entry. + * This will prune the cache if it gets too large. + * If the item is already set, it will be pushed to the top of the cache. + * + * @param $key string + * @param $prop string + * @param $value mixed + * @return void + */ + public function set( $key, $prop, $value ) { + if ( isset( $this->cache[$key] ) ) { + $this->ping( $key ); // push to top + } elseif ( count( $this->cache ) >= $this->maxCacheKeys ) { + reset( $this->cache ); + unset( $this->cache[key( $this->cache )] ); + } + $this->cache[$key][$prop] = $value; + } + + /** + * Check if a property field exists for a cache entry. + * + * @param $key string + * @param $prop string + * @return bool + */ + public function has( $key, $prop ) { + return isset( $this->cache[$key][$prop] ); + } + + /** + * Get a property field for a cache entry. + * This returns null if the property is not set. + * If the item is already set, it will be pushed to the top of the cache. + * + * @param $key string + * @param $prop string + * @return mixed + */ + public function get( $key, $prop ) { + if ( isset( $this->cache[$key][$prop] ) ) { + $this->ping( $key ); // push to top + return $this->cache[$key][$prop]; + } else { + return null; + } + } + + /** + * Clear one or several cache entries, or all cache entries + * + * @param $keys string|Array + * @return void + */ + public function clear( $keys = null ) { + if ( $keys === null ) { + $this->cache = array(); + } else { + foreach ( (array)$keys as $key ) { + unset( $this->cache[$key] ); + } + } + } + + /** + * Push an entry to the top of the cache + * + * @param $key string + */ + protected function ping( $key ) { + $item = $this->cache[$key]; + unset( $this->cache[$key] ); + $this->cache[$key] = $item; + } +} diff --git a/includes/filerepo/backend/FileBackendStore.php b/includes/filerepo/backend/FileBackendStore.php index 9c713d381c..49bb039497 100644 --- a/includes/filerepo/backend/FileBackendStore.php +++ b/includes/filerepo/backend/FileBackendStore.php @@ -38,13 +38,10 @@ abstract class FileBackendStore extends FileBackend { /** @var BagOStuff */ protected $memCache; - - /** @var Array Map of paths to small (RAM/disk) cache items */ - protected $cache = array(); // (storage path => key => value) - protected $maxCacheSize = 300; // integer; max paths with entries - /** @var Array Map of paths to large (RAM/disk) cache items */ - protected $expensiveCache = array(); // (storage path => key => value) - protected $maxExpensiveCacheSize = 5; // integer; max paths with entries + /** @var ProcessCacheLRU */ + protected $cheapCache; // Map of paths to small (RAM/disk) cache items + /** @var ProcessCacheLRU */ + protected $expensiveCache; // Map of paths to large (RAM/disk) cache items /** @var Array Map of container names to sharding settings */ protected $shardViaHashLevels = array(); // (container name => config array) @@ -58,7 +55,9 @@ abstract class FileBackendStore extends FileBackend { */ public function __construct( array $config ) { parent::__construct( $config ); - $this->memCache = new EmptyBagOStuff(); // disabled by default + $this->memCache = new EmptyBagOStuff(); // disabled by default + $this->cheapCache = new ProcessCacheLRU( 300 ); + $this->expensiveCache = new ProcessCacheLRU( 5 ); } /** @@ -579,17 +578,17 @@ abstract class FileBackendStore extends FileBackend { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); $latest = !empty( $params['latest'] ); // use latest data? - if ( !isset( $this->cache[$path]['stat'] ) ) { + if ( !$this->cheapCache->has( $path, 'stat' ) ) { $this->primeFileCache( array( $path ) ); // check persistent cache } - if ( isset( $this->cache[$path]['stat'] ) ) { + if ( $this->cheapCache->has( $path, 'stat' ) ) { + $stat = $this->cheapCache->get( $path, 'stat' ); // If we want the latest data, check that this cached // value was in fact fetched with the latest available data. - if ( !$latest || $this->cache[$path]['stat']['latest'] ) { - $this->pingCache( $path ); // LRU + if ( !$latest || $stat['latest'] ) { wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); - return $this->cache[$path]['stat']; + return $stat; } } wfProfileIn( __METHOD__ . '-miss' ); @@ -599,13 +598,11 @@ abstract class FileBackendStore extends FileBackend { wfProfileOut( __METHOD__ . '-miss' ); if ( is_array( $stat ) ) { // don't cache negatives $stat['latest'] = $latest; - $this->trimCache(); // limit memory - $this->cache[$path]['stat'] = $stat; + $this->cheapCache->set( $path, 'stat', $stat ); $this->setFileCache( $path, $stat ); // update persistent cache if ( isset( $stat['sha1'] ) ) { // some backends store SHA-1 as metadata - $this->trimCache(); // limit memory - $this->cache[$path]['sha1'] = - array( 'hash' => $stat['sha1'], 'latest' => $latest ); + $this->cheapCache->set( $path, 'sha1', + array( 'hash' => $stat['sha1'], 'latest' => $latest ) ); } } else { wfDebug( __METHOD__ . ": File $path does not exist.\n" ); @@ -653,14 +650,14 @@ abstract class FileBackendStore extends FileBackend { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); $latest = !empty( $params['latest'] ); // use latest data? - if ( isset( $this->cache[$path]['sha1'] ) ) { + if ( $this->cheapCache->has( $path, 'sha1' ) ) { + $stat = $this->cheapCache->get( $path, 'sha1' ); // If we want the latest data, check that this cached // value was in fact fetched with the latest available data. - if ( !$latest || $this->cache[$path]['sha1']['latest'] ) { - $this->pingCache( $path ); // LRU + if ( !$latest || $stat['latest'] ) { wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); - return $this->cache[$path]['sha1']['hash']; + return $stat['hash']; } } wfProfileIn( __METHOD__ . '-miss' ); @@ -669,8 +666,8 @@ abstract class FileBackendStore extends FileBackend { wfProfileOut( __METHOD__ . '-miss-' . $this->name ); wfProfileOut( __METHOD__ . '-miss' ); if ( $hash ) { // don't cache negatives - $this->trimCache(); // limit memory - $this->cache[$path]['sha1'] = array( 'hash' => $hash, 'latest' => $latest ); + $this->cheapCache->set( $path, 'sha1', + array( 'hash' => $hash, 'latest' => $latest ) ); } wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); @@ -716,21 +713,20 @@ abstract class FileBackendStore extends FileBackend { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); $latest = !empty( $params['latest'] ); // use latest data? - if ( isset( $this->expensiveCache[$path]['localRef'] ) ) { + if ( $this->expensiveCache->has( $path, 'localRef' ) ) { + $val = $this->expensiveCache->get( $path, 'localRef' ); // If we want the latest data, check that this cached // value was in fact fetched with the latest available data. - if ( !$latest || $this->expensiveCache[$path]['localRef']['latest'] ) { - $this->pingExpensiveCache( $path ); + if ( !$latest || $val['latest'] ) { wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); - return $this->expensiveCache[$path]['localRef']['object']; + return $val['object']; } } $tmpFile = $this->getLocalCopy( $params ); if ( $tmpFile ) { // don't cache negatives - $this->trimExpensiveCache(); // limit memory - $this->expensiveCache[$path]['localRef'] = - array( 'object' => $tmpFile, 'latest' => $latest ); + $this->expensiveCache->set( $path, 'localRef', + array( 'object' => $tmpFile, 'latest' => $latest ) ); } wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); @@ -1119,12 +1115,12 @@ abstract class FileBackendStore extends FileBackend { $paths = array_filter( $paths, 'strlen' ); // remove nulls } if ( $paths === null ) { - $this->cache = array(); - $this->expensiveCache = array(); + $this->cheapCache->clear(); + $this->expensiveCache->clear(); } else { foreach ( $paths as $path ) { - unset( $this->cache[$path] ); - unset( $this->expensiveCache[$path] ); + $this->cheapCache->clear( $path ); + $this->expensiveCache->clear( $path ); } } $this->doClearCache( $paths ); @@ -1149,58 +1145,6 @@ abstract class FileBackendStore extends FileBackend { */ abstract protected function directoriesAreVirtual(); - /** - * Move a cache entry to the top (such as when accessed) - * - * @param $path string Storage path - * @return void - */ - protected function pingCache( $path ) { - if ( isset( $this->cache[$path] ) ) { - $tmp = $this->cache[$path]; - unset( $this->cache[$path] ); - $this->cache[$path] = $tmp; - } - } - - /** - * Prune the inexpensive cache if it is too big to add an item - * - * @return void - */ - protected function trimCache() { - if ( count( $this->cache ) >= $this->maxCacheSize ) { - reset( $this->cache ); - unset( $this->cache[key( $this->cache )] ); - } - } - - /** - * Move a cache entry to the top (such as when accessed) - * - * @param $path string Storage path - * @return void - */ - protected function pingExpensiveCache( $path ) { - if ( isset( $this->expensiveCache[$path] ) ) { - $tmp = $this->expensiveCache[$path]; - unset( $this->expensiveCache[$path] ); - $this->expensiveCache[$path] = $tmp; - } - } - - /** - * Prune the expensive cache if it is too big to add an item - * - * @return void - */ - protected function trimExpensiveCache() { - if ( count( $this->expensiveCache ) >= $this->maxExpensiveCacheSize ) { - reset( $this->expensiveCache ); - unset( $this->expensiveCache[key( $this->expensiveCache )] ); - } - } - /** * Check if a container name is valid. * This checks for for length and illegal characters. @@ -1555,12 +1499,10 @@ abstract class FileBackendStore extends FileBackend { foreach ( $values as $cacheKey => $val ) { if ( is_array( $val ) ) { $path = $pathNames[$cacheKey]; - $this->trimCache(); // limit memory - $this->cache[$path]['stat'] = $val; + $this->cheapCache->set( $path, 'stat', $val ); if ( isset( $val['sha1'] ) ) { // some backends store SHA-1 as metadata - $this->trimCache(); // limit memory - $this->cache[$path]['sha1'] = - array( 'hash' => $val['sha1'], 'latest' => $val['latest'] ); + $this->cheapCache->set( $path, 'sha1', + array( 'hash' => $val['sha1'], 'latest' => $val['latest'] ) ); } } } diff --git a/tests/phpunit/includes/cache/ProcessCacheLRUTest.php b/tests/phpunit/includes/cache/ProcessCacheLRUTest.php new file mode 100644 index 0000000000..30bfb12475 --- /dev/null +++ b/tests/phpunit/includes/cache/ProcessCacheLRUTest.php @@ -0,0 +1,239 @@ +assertAttributeEquals( array(), 'cache', $cache, $msg ); + } + + /** + * Helper to fill a cache object passed by reference + */ + function fillCache( &$cache, $numEntries ) { + // Fill cache with three values + for( $i=1; $i<=$numEntries; $i++) { + $cache->set( "cache-key-$i", "prop-$i", "value-$i" ); + } + } + + /** + * Generates an array of what would be expected in cache for a given cache + * size and a number of entries filled in sequentially + */ + function getExpectedCache( $cacheMaxEntries, $entryToFill ) { + $expected = array(); + + if( $entryToFill === 0 ) { + # The cache is empty! + return array(); + } elseif( $entryToFill <= $cacheMaxEntries ) { + # Cache is not fully filled + $firstKey = 1; + } else { + # Cache overflowed + $firstKey = 1 + $entryToFill - $cacheMaxEntries; + } + + $lastKey = $entryToFill; + + for( $i=$firstKey; $i<=$lastKey; $i++ ) { + $expected["cache-key-$i"] = array( "prop-$i" => "value-$i" ); + } + return $expected; + } + + /** + * Highlight diff between assertEquals and assertNotSame + */ + function testPhpUnitArrayEquality() { + $one = array( 'A' => 1, 'B' => 2 ); + $two = array( 'B' => 2, 'A' => 1 ); + $this->assertEquals( $one, $two ); // == + $this->assertNotSame( $one, $two ); // === + } + + /** + * @dataProvider provideInvalidConstructorArg + * @expectedException MWException + */ + function testConstructorGivenInvalidValue( $maxSize ) { + $c = new ProcessCacheLRUTestable( $maxSize ); + } + + /** + * Value which are forbidden by the constructor + */ + function provideInvalidConstructorArg() { + return array( + array( null ), + array( array() ), + array( new stdClass() ), + array( 0 ), + array( '5' ), + array( -1 ), + ); + } + + function testAddAndGetAKey() { + $oneCache = new ProcessCacheLRUTestable( 1 ); + $this->assertCacheEmpty( $oneCache ); + + // First set just one value + $oneCache->set( 'cache-key', 'prop1', 'value1' ); + $this->assertEquals( 1, $oneCache->getEntriesCount() ); + $this->assertTrue( $oneCache->has( 'cache-key', 'prop1' ) ); + $this->assertEquals( 'value1', $oneCache->get( 'cache-key', 'prop1' ) ); + } + + function testDeleteOldKey() { + $oneCache = new ProcessCacheLRUTestable( 1 ); + $this->assertCacheEmpty( $oneCache ); + + $oneCache->set( 'cache-key', 'prop1', 'value1' ); + $oneCache->set( 'cache-key', 'prop1', 'value2' ); + $this->assertEquals( 'value2', $oneCache->get( 'cache-key', 'prop1' ) ); + } + + /** + * This test that we properly overflow when filling a cache with + * a sequence of always different cache-keys. Meant to verify we correclty + * delete the older key. + * + * @dataProvider provideCacheFilling + * @param $cacheMaxEntries Maximum entry the created cache will hold + * @param $entryToFill Number of entries to insert in the created cache. + */ + function testFillingCache( $cacheMaxEntries, $entryToFill, $msg = '' ) { + $cache = new ProcessCacheLRUTestable( $cacheMaxEntries ); + $this->fillCache( $cache, $entryToFill); + + $this->assertSame( + $this->getExpectedCache( $cacheMaxEntries, $entryToFill ), + $cache->getCache(), + "Filling a $cacheMaxEntries entries cache with $entryToFill entries" + ); + + } + + /** + * Provider for testFillingCache + */ + function provideCacheFilling() { + // ($cacheMaxEntries, $entryToFill, $msg='') + return array( + array( 1, 0 ), + array( 1, 1 ), + array( 1, 2 ), # overflow + array( 5, 33 ), # overflow + ); + + } + + /** + * Create a cache with only one remaining entry then update + * the first inserted entry. Should bump it to the top. + */ + function testReplaceExistingKeyShouldBumpEntryToTop() { + $maxEntries = 3; + + $cache = new ProcessCacheLRUTestable( $maxEntries ); + // Fill cache leaving just one remaining slot + $this->fillCache( $cache, $maxEntries - 1 ); + + // Set an existing cache key + $cache->set( "cache-key-1", "prop-1", "new-value-for-1" ); + + $this->assertSame( + array( + 'cache-key-2' => array( 'prop-2' => 'value-2' ), + 'cache-key-1' => array( 'prop-1' => 'new-value-for-1' ), + ), + $cache->getCache() + ); + } + + function testRecentlyAccessedKeyStickIn() { + $cache = new ProcessCacheLRUTestable( 2 ); + $cache->set( 'first' , 'prop1', 'value1' ); + $cache->set( 'second', 'prop2', 'value2' ); + + // Get first + $cache->get( 'first', 'prop1' ); + // Cache a third value, should invalidate the least used one + $cache->set( 'third', 'prop3', 'value3' ); + + $this->assertFalse( $cache->has( 'second', 'prop2' ) ); + } + + /** + * This first create a full cache then update the value for the 2nd + * filled entry. + * Given a cache having 1,2,3 as key, updating 2 should bump 2 to + * the top of the queue with the new value: 1,3,2* (* = updated). + */ + function testReplaceExistingKeyInAFullCacheShouldBumpToTop() { + $maxEntries = 3; + + $cache = new ProcessCacheLRUTestable( $maxEntries ); + $this->fillCache( $cache, $maxEntries ); + + // Set an existing cache key + $cache->set( "cache-key-2", "prop-2", "new-value-for-2" ); + $this->assertSame( + array( + 'cache-key-1' => array( 'prop-1' => 'value-1' ), + 'cache-key-3' => array( 'prop-3' => 'value-3' ), + 'cache-key-2' => array( 'prop-2' => 'new-value-for-2' ), + ), + $cache->getCache() + ); + $this->assertEquals( 'new-value-for-2', + $cache->get( 'cache-key-2', 'prop-2' ) + ); + } + + function testBumpExistingKeyToTop() { + $cache = new ProcessCacheLRUTestable( 3 ); + $this->fillCache( $cache, 3 ); + + // Set the very first cache key to a new value + $cache->set( "cache-key-1", "prop-1", "new value for 1" ); + $this->assertEquals( + array( + 'cache-key-2' => array( 'prop-2' => 'value-2' ), + 'cache-key-3' => array( 'prop-3' => 'value-3' ), + 'cache-key-1' => array( 'prop-1' => 'new value for 1' ), + ), + $cache->getCache() + ); + + } + +} + +/** + * Overrides some ProcessCacheLRU methods and properties accessibility. + */ +class ProcessCacheLRUTestable extends ProcessCacheLRU { + public $cache = array(); + + public function getCache() { + return $this->cache; + } + public function getEntriesCount() { + return count( $this->cache ); + } +}