From ae63b9ae10cff9471cc32eb92dc29430bf3cd5cb Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 16 Nov 2012 12:02:42 -0800 Subject: [PATCH] [FileBackend] Stat caching improvements. Callers often tend to end up calling getFileStat(), at least indirectly, or in various successive function on the same path. This created RTTs when the file didn't exist since negatives were not cached. This change does the following: * Cache definitive negatives (404s) in the process cache. Nothing is cached on failure (like network problems). * Ignore process cache entries after a brief time period so long running scripts do not have overly stale entries. Change-Id: I356bd9f48281e3c7e7a273778b2aca59c521a0c7 --- includes/cache/ProcessCacheLRU.php | 19 +++++++++++++++---- includes/filebackend/FileBackendStore.php | 20 +++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/includes/cache/ProcessCacheLRU.php b/includes/cache/ProcessCacheLRU.php index f215ebd85d..76c76f3701 100644 --- a/includes/cache/ProcessCacheLRU.php +++ b/includes/cache/ProcessCacheLRU.php @@ -28,6 +28,8 @@ class ProcessCacheLRU { /** @var Array */ protected $cache = array(); // (key => prop => value) + /** @var Array */ + protected $cacheTimes = array(); // (key => prop => UNIX timestamp) protected $maxCacheKeys; // integer; max entries @@ -44,7 +46,7 @@ class ProcessCacheLRU { /** * Set a property field for a cache entry. - * This will prune the cache if it gets too large. + * 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. * * @param $key string @@ -57,9 +59,12 @@ class ProcessCacheLRU { $this->ping( $key ); // push to top } elseif ( count( $this->cache ) >= $this->maxCacheKeys ) { reset( $this->cache ); - unset( $this->cache[key( $this->cache )] ); + $evictKey = key( $this->cache ); + unset( $this->cache[$evictKey] ); + unset( $this->cacheTimes[$evictKey] ); } $this->cache[$key][$prop] = $value; + $this->cacheTimes[$key][$prop] = time(); } /** @@ -67,10 +72,14 @@ class ProcessCacheLRU { * * @param $key string * @param $prop string + * @param $maxAge integer Ignore items older than this many seconds (since 1.21) * @return bool */ - public function has( $key, $prop ) { - return isset( $this->cache[$key][$prop] ); + public function has( $key, $prop, $maxAge = 0 ) { + if ( isset( $this->cache[$key][$prop] ) ) { + return ( $maxAge <= 0 || ( time() - $this->cacheTimes[$key][$prop] ) <= $maxAge ); + } + return false; } /** @@ -100,9 +109,11 @@ class ProcessCacheLRU { public function clear( $keys = null ) { if ( $keys === null ) { $this->cache = array(); + $this->cacheTimes = array(); } else { foreach ( (array)$keys as $key ) { unset( $this->cache[$key] ); + unset( $this->cacheTimes[$key] ); } } } diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 97e49a5a09..43e6cb3ed8 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -48,6 +48,8 @@ abstract class FileBackendStore extends FileBackend { protected $maxFileSize = 4294967296; // integer bytes (4GiB) + const CACHE_TTL = 10; // integer; TTL in seconds for process cache entries + /** * @see FileBackend::__construct() * @@ -601,14 +603,15 @@ abstract class FileBackendStore extends FileBackend { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); $latest = !empty( $params['latest'] ); // use latest data? - if ( !$this->cheapCache->has( $path, 'stat' ) ) { + if ( !$this->cheapCache->has( $path, 'stat', self::CACHE_TTL ) ) { $this->primeFileCache( array( $path ) ); // check persistent cache } - if ( $this->cheapCache->has( $path, 'stat' ) ) { + if ( $this->cheapCache->has( $path, 'stat', self::CACHE_TTL ) ) { $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 || $stat['latest'] ) { + // value was in fact fetched with the latest available data + // (the process cache is ignored if it contains a negative). + if ( !$latest || ( is_array( $stat ) && $stat['latest'] ) ) { wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); return $stat; @@ -619,7 +622,7 @@ abstract class FileBackendStore extends FileBackend { $stat = $this->doGetFileStat( $params ); wfProfileOut( __METHOD__ . '-miss-' . $this->name ); wfProfileOut( __METHOD__ . '-miss' ); - if ( is_array( $stat ) ) { // don't cache negatives + if ( is_array( $stat ) ) { // file exists $stat['latest'] = $latest; $this->cheapCache->set( $path, 'stat', $stat ); $this->setFileCache( $path, $stat ); // update persistent cache @@ -627,8 +630,11 @@ abstract class FileBackendStore extends FileBackend { $this->cheapCache->set( $path, 'sha1', array( 'hash' => $stat['sha1'], 'latest' => $latest ) ); } - } else { + } elseif ( $stat === false ) { // file does not exist + $this->cheapCache->set( $path, 'stat', false ); wfDebug( __METHOD__ . ": File $path does not exist.\n" ); + } else { // an error occurred + wfDebug( __METHOD__ . ": Could not stat file $path.\n" ); } wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); @@ -682,7 +688,7 @@ abstract class FileBackendStore extends FileBackend { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); $latest = !empty( $params['latest'] ); // use latest data? - if ( $this->cheapCache->has( $path, 'sha1' ) ) { + if ( $this->cheapCache->has( $path, 'sha1', self::CACHE_TTL ) ) { $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. -- 2.20.1