From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 20 Mar 2019 05:50:41 +0000 (-0700)
Subject: objectcache: optimize merge()/incr() for WinCacheBagOStuff
X-Git-Tag: 1.34.0-rc.0~2422^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/pie.php?a=commitdiff_plain;h=8c92400909c74dff595d24a092f5965155530a48;p=lhc%2Fweb%2Fwiklou.git

objectcache: optimize merge()/incr() for WinCacheBagOStuff

Do not hold WinCache locks during merge() callbacks, as that
could be seconds of being blocked (and some callers do not
expect indefinite blocking). Make merge() use the CAS method
which only uses wincache_lock() after callback runs in very
tight getWithToken()/set() cycle.

Make incr() use wincache_lock() since it also is very tight
doGet()/set() cycle with negligable work in between.

Change-Id: I0dde6f62c7e3d4a802470c181570ad4353d0e6ea
---

diff --git a/includes/libs/objectcache/WinCacheBagOStuff.php b/includes/libs/objectcache/WinCacheBagOStuff.php
index cae0280128..0ca3e4a5f9 100644
--- a/includes/libs/objectcache/WinCacheBagOStuff.php
+++ b/includes/libs/objectcache/WinCacheBagOStuff.php
@@ -29,12 +29,50 @@
  */
 class WinCacheBagOStuff extends BagOStuff {
 	protected function doGet( $key, $flags = 0 ) {
-		$val = wincache_ucache_get( $key );
-		if ( is_string( $val ) ) {
-			$val = unserialize( $val );
+		$blob = wincache_ucache_get( $key );
+
+		return is_string( $blob ) ? unserialize( $blob ) : false;
+	}
+
+	protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+		$casToken = null;
+
+		$blob = wincache_ucache_get( $key );
+		if ( !is_string( $blob ) ) {
+			return false;
+		}
+
+		$value = unserialize( $blob );
+		if ( $value === false ) {
+			return false;
+		}
+
+		$casToken = $blob; // don't bother hashing this
+
+		return $value;
+	}
+
+	protected function cas( $casToken, $key, $value, $exptime = 0, $flags = 0 ) {
+		if ( !wincache_lock( $key ) ) { // optimize with FIFO lock
+			return false;
+		}
+
+		$curCasToken = null; // passed by reference
+		$this->getWithToken( $key, $curCasToken, self::READ_LATEST );
+		if ( $casToken === $curCasToken ) {
+			$success = $this->set( $key, $value, $exptime, $flags );
+		} else {
+			$this->logger->info(
+				__METHOD__ . ' failed due to race condition for {key}.',
+				[ 'key' => $key ]
+			);
+
+			$success = false; // mismatched or failed
 		}
 
-		return $val;
+		wincache_unlock( $key );
+
+		return $success;
 	}
 
 	public function set( $key, $value, $expire = 0, $flags = 0 ) {
@@ -56,14 +94,7 @@ class WinCacheBagOStuff extends BagOStuff {
 	}
 
 	public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
-		if ( wincache_lock( $key ) ) { // optimize with FIFO lock
-			$ok = $this->mergeViaLock( $key, $callback, $exptime, $attempts, $flags );
-			wincache_unlock( $key );
-		} else {
-			$ok = false;
-		}
-
-		return $ok;
+		return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
 	}
 
 	/**
@@ -109,18 +140,20 @@ class WinCacheBagOStuff extends BagOStuff {
 	 * @return int|bool New value or false on failure
 	 */
 	public function incr( $key, $value = 1 ) {
-		if ( !$this->lock( $key ) ) {
+		if ( !wincache_lock( $key ) ) { // optimize with FIFO lock
 			return false;
 		}
-		$n = $this->get( $key );
-		if ( $this->isInteger( $n ) ) { // key exists?
-			$n += intval( $value );
+
+		$n = $this->doGet( $key );
+		if ( $this->isInteger( $n ) ) {
+			$n = max( $n + (int)$value, 0 );
 			$oldTTL = wincache_ucache_info( false, $key )["ucache_entries"][1]["ttl_seconds"];
-			$this->set( $key, max( 0, $n ), $oldTTL );
+			$this->set( $key, $n, $oldTTL );
 		} else {
 			$n = false;
 		}
-		$this->unlock( $key );
+
+		wincache_unlock( $key );
 
 		return $n;
 	}