From 60608c12749a6c503bd340cd14f081f9250e9ec3 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 14 May 2015 22:52:35 -0700
Subject: [PATCH] Cleanups to WANObjectCache::getWithSetCallback code

* Do not use lock() for tombstoned keys unless lockTSE was set
* Moved the is_callable() check down a bit further
* Also removed one inaccurate comment

Change-Id: I904e0681faa48b1dc2bc2a3c005a29d2f8347065
---
 includes/libs/objectcache/WANObjectCache.php | 30 +++++++++++---------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index 8d202c74ca..6ec7e483b0 100755
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -404,16 +404,12 @@ class WANObjectCache {
 			return $value;
 		}
 
-		if ( !is_callable( $callback ) ) {
-			throw new InvalidArgumentException( "Invalid cache miss callback provided." );
-		}
-
+		$isTombstone = ( $curTTL !== null && $value === false );
 		// Assume a key is hot if requested soon after invalidation
 		$isHot = ( $curTTL !== null && $curTTL <= 0 && abs( $curTTL ) <= $lockTSE );
-		$isTombstone = ( $curTTL !== null && $value === false );
 
 		$locked = false;
-		if ( $isHot || $isTombstone ) {
+		if ( $isHot ) {
 			// Acquire a cluster-local non-blocking lock
 			if ( $this->cache->lock( $key, 0, self::LOCK_TTL ) ) {
 				// Lock acquired; this thread should update the key
@@ -421,17 +417,23 @@ class WANObjectCache {
 			} elseif ( $value !== false ) {
 				// If it cannot be acquired; then the stale value can be used
 				return $value;
-			} else {
-				// Either another thread has the lock or the lock failed.
-				// Use the stash value, which is likely from the prior thread.
-				$value = $this->cache->get( self::STASH_KEY_PREFIX . $key );
-				// Regenerate on timeout or if the other thread failed
-				if ( $value !== false ) {
-					return $value;
-				}
 			}
 		}
 
+		if ( !$locked && ( $isTombstone || $isHot ) ) {
+			// Use the stash value for tombstoned keys to reduce regeneration load.
+			// For hot keys, either another thread has the lock or the lock failed;
+			// use the stash value from the last thread that regenerated it.
+			$value = $this->cache->get( self::STASH_KEY_PREFIX . $key );
+			if ( $value !== false ) {
+				return $value;
+			}
+		}
+
+		if ( !is_callable( $callback ) ) {
+			throw new InvalidArgumentException( "Invalid cache miss callback provided." );
+		}
+
 		// Generate the new value from the callback...
 		$value = call_user_func_array( $callback, array( $cValue, &$ttl ) );
 		// When delete() is called, writes are write-holed by the tombstone,
-- 
2.20.1