From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 20 Mar 2019 17:57:02 +0000 (-0700)
Subject: objectcache: convert APC and hash BagOStuff to using mergeViaCas()
X-Git-Tag: 1.34.0-rc.0~2421^2
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/journal.php?a=commitdiff_plain;h=6812712bd69f8c52162bc4cc5e1df42b3c0cb46f;p=lhc%2Fweb%2Fwiklou.git

objectcache: convert APC and hash BagOStuff to using mergeViaCas()

This way, the $attempts parameter to merge() is respected and locks
are not held while possibly slow callbacks might run.

Also renamed confusingly named serialization functions in HashBagOStuff.

Change-Id: Id031d82e0a7c941936f04d2cdf590a6296777cf8
---

diff --git a/includes/libs/objectcache/APCBagOStuff.php b/includes/libs/objectcache/APCBagOStuff.php
index 847a1eb069..74dcddff02 100644
--- a/includes/libs/objectcache/APCBagOStuff.php
+++ b/includes/libs/objectcache/APCBagOStuff.php
@@ -73,24 +73,25 @@ class APCBagOStuff extends BagOStuff {
 	}
 
 	protected function doGet( $key, $flags = 0 ) {
-		return $this->getUnserialize(
-			apc_fetch( $key . self::KEY_SUFFIX )
-		);
+		return $this->unserialize( apc_fetch( $key . self::KEY_SUFFIX ) );
 	}
 
-	protected function getUnserialize( $value ) {
-		if ( is_string( $value ) && !$this->nativeSerialize ) {
-			$value = $this->isInteger( $value )
-				? intval( $value )
-				: unserialize( $value );
+	protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+		$casToken = null;
+
+		$blob = apc_fetch( $key . self::KEY_SUFFIX );
+		$value = $this->unserialize( $blob );
+		if ( $value !== false ) {
+			$casToken = $blob; // don't bother hashing this
 		}
+
 		return $value;
 	}
 
 	public function set( $key, $value, $exptime = 0, $flags = 0 ) {
 		apc_store(
 			$key . self::KEY_SUFFIX,
-			$this->setSerialize( $value ),
+			$this->serialize( $value ),
 			$exptime
 		);
 
@@ -100,18 +101,11 @@ class APCBagOStuff extends BagOStuff {
 	public function add( $key, $value, $exptime = 0, $flags = 0 ) {
 		return apc_add(
 			$key . self::KEY_SUFFIX,
-			$this->setSerialize( $value ),
+			$this->serialize( $value ),
 			$exptime
 		);
 	}
 
-	protected function setSerialize( $value ) {
-		if ( !$this->nativeSerialize && !$this->isInteger( $value ) ) {
-			$value = serialize( $value );
-		}
-		return $value;
-	}
-
 	public function delete( $key, $flags = 0 ) {
 		apc_delete( $key . self::KEY_SUFFIX );
 
@@ -125,4 +119,24 @@ class APCBagOStuff extends BagOStuff {
 	public function decr( $key, $value = 1 ) {
 		return apc_dec( $key . self::KEY_SUFFIX, $value );
 	}
+
+	public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
+		return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
+	}
+
+	protected function serialize( $value ) {
+		if ( !$this->nativeSerialize && !$this->isInteger( $value ) ) {
+			$value = serialize( $value );
+		}
+		return $value;
+	}
+
+	protected function unserialize( $value ) {
+		if ( is_string( $value ) && !$this->nativeSerialize ) {
+			$value = $this->isInteger( $value )
+				? intval( $value )
+				: unserialize( $value );
+		}
+		return $value;
+	}
 }
diff --git a/includes/libs/objectcache/APCUBagOStuff.php b/includes/libs/objectcache/APCUBagOStuff.php
index d5f1edc163..55e940503d 100644
--- a/includes/libs/objectcache/APCUBagOStuff.php
+++ b/includes/libs/objectcache/APCUBagOStuff.php
@@ -40,15 +40,25 @@ class APCUBagOStuff extends APCBagOStuff {
 	}
 
 	protected function doGet( $key, $flags = 0 ) {
-		return $this->getUnserialize(
-			apcu_fetch( $key . self::KEY_SUFFIX )
-		);
+		return $this->unserialize( apcu_fetch( $key . self::KEY_SUFFIX ) );
+	}
+
+	protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+		$casToken = null;
+
+		$blob = apcu_fetch( $key . self::KEY_SUFFIX );
+		$value = $this->unserialize( $blob );
+		if ( $value !== false ) {
+			$casToken = $blob; // don't bother hashing this
+		}
+
+		return $value;
 	}
 
 	public function set( $key, $value, $exptime = 0, $flags = 0 ) {
 		apcu_store(
 			$key . self::KEY_SUFFIX,
-			$this->setSerialize( $value ),
+			$this->serialize( $value ),
 			$exptime
 		);
 
@@ -58,7 +68,7 @@ class APCUBagOStuff extends APCBagOStuff {
 	public function add( $key, $value, $exptime = 0, $flags = 0 ) {
 		return apcu_add(
 			$key . self::KEY_SUFFIX,
-			$this->setSerialize( $value ),
+			$this->serialize( $value ),
 			$exptime
 		);
 	}
@@ -94,4 +104,8 @@ class APCUBagOStuff extends APCBagOStuff {
 			return false;
 		}
 	}
+
+	public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
+		return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
+	}
 }
diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index 4793ce046e..3c6520e1b3 100644
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -34,8 +34,15 @@ class HashBagOStuff extends BagOStuff {
 	/** @var int Max entries allowed */
 	protected $maxCacheKeys;
 
+	/** @var string CAS token prefix for this instance */
+	private $token;
+
+	/** @var int CAS token counter */
+	private static $casCounter = 0;
+
 	const KEY_VAL = 0;
 	const KEY_EXP = 1;
+	const KEY_CAS = 2;
 
 	/**
 	 * @param array $params Additional parameters include:
@@ -44,34 +51,13 @@ class HashBagOStuff extends BagOStuff {
 	function __construct( $params = [] ) {
 		parent::__construct( $params );
 
+		$this->token = microtime( true ) . ':' . mt_rand();
 		$this->maxCacheKeys = $params['maxKeys'] ?? INF;
 		if ( $this->maxCacheKeys <= 0 ) {
 			throw new InvalidArgumentException( '$maxKeys parameter must be above zero' );
 		}
 	}
 
-	protected function expire( $key ) {
-		$et = $this->bag[$key][self::KEY_EXP];
-		if ( $et == self::TTL_INDEFINITE || $et > $this->getCurrentTime() ) {
-			return false;
-		}
-
-		$this->delete( $key );
-
-		return true;
-	}
-
-	/**
-	 * Does this bag have a non-null value for the given key?
-	 *
-	 * @param string $key
-	 * @return bool
-	 * @since 1.27
-	 */
-	protected function hasKey( $key ) {
-		return isset( $this->bag[$key] );
-	}
-
 	protected function doGet( $key, $flags = 0 ) {
 		if ( !$this->hasKey( $key ) ) {
 			return false;
@@ -89,12 +75,24 @@ class HashBagOStuff extends BagOStuff {
 		return $this->bag[$key][self::KEY_VAL];
 	}
 
+	protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+		$casToken = null;
+
+		$value = $this->doGet( $key );
+		if ( $value !== false ) {
+			$casToken = $this->bag[$key][self::KEY_CAS];
+		}
+
+		return $value;
+	}
+
 	public function set( $key, $value, $exptime = 0, $flags = 0 ) {
 		// Refresh key position for maxCacheKeys eviction
 		unset( $this->bag[$key] );
 		$this->bag[$key] = [
 			self::KEY_VAL => $value,
-			self::KEY_EXP => $this->convertExpiry( $exptime )
+			self::KEY_EXP => $this->convertExpiry( $exptime ),
+			self::KEY_CAS => $this->token . ':' . ++self::$casCounter
 		];
 
 		if ( count( $this->bag ) > $this->maxCacheKeys ) {
@@ -132,7 +130,40 @@ class HashBagOStuff extends BagOStuff {
 		return false;
 	}
 
+	public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
+		return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
+	}
+
+	/**
+	 * Clear all values in cache
+	 */
 	public function clear() {
 		$this->bag = [];
 	}
+
+	/**
+	 * @param string $key
+	 * @return bool
+	 */
+	protected function expire( $key ) {
+		$et = $this->bag[$key][self::KEY_EXP];
+		if ( $et == self::TTL_INDEFINITE || $et > $this->getCurrentTime() ) {
+			return false;
+		}
+
+		$this->delete( $key );
+
+		return true;
+	}
+
+	/**
+	 * Does this bag have a non-null value for the given key?
+	 *
+	 * @param string $key
+	 * @return bool
+	 * @since 1.27
+	 */
+	protected function hasKey( $key ) {
+		return isset( $this->bag[$key] );
+	}
 }