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 847a1eb..74dcddf 100644 (file)
--- 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 d5f1edc..55e9405 100644 (file)
--- 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 4793ce0..3c6520e 100644 (file)
--- 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] );
+       }
 }