MDL-40903 cache: converted persistent into persistentdata

At the same time all cache instances were made persistent

cache/README.md

@@ -24,7 +24,7 @@ A definition:
             'overrideclassfile' => null,              // Optional
             'datasource' => null,                     // Optional
             'datasourcefile' => null,                 // Optional
-            'persistent' => false,                    // Optional
+            'persistentdata' => false,                // Optional
             'persistentmaxsize' => false,             // Optional
             'ttl' => 0,                               // Optional
             'mappingsonly' => false                   // Optional
@@ -144,8 +144,8 @@ The following optional settings can also be defined:
 * overrideclassfile - Included if required when using the overrideclass param.
 * datasource - If provided this class will be used as a data source for the definition. It must implement the cache_data_source interface.
 * datasourcefile - Included if required when using the datasource param.
-* persistent - If set to true the loader will be stored when first created and provided to subsequent requests. More on this later.
-* persistentmaxsize - If set to an int this will be the maximum number of items stored in the persistent cache.
+* persistentdata - Any data passing through the cache will be held onto to make subsequent requests for it faster.
+* persistentmaxsize - If set to an int this will be the maximum number of items stored in the persistent data cache.
 * ttl - Can be used to set a ttl value for data being set for this cache.
 * mappingsonly - This definition can only be used if there is a store mapping for it. More on this later.
 * invalidationevents - An array of events that should trigger this cache to invalidate.
@@ -154,11 +154,10 @@ The following optional settings can also be defined:
 
 It's important to note that internally the definition is also aware of the component. This is picked up when the definition is read, based upon the location of the caches.php file.
 
-The persistent option.
-As noted the persistent option causes the loader generated for this definition to be stored when first created. Subsequent requests for this definition will be given the original loader instance.
+The persistentdata option.
 Data passed to or retrieved from the loader and its chained loaders gets cached by the instance.
-This option should be used when you know you will require the loader several times and perhaps in different areas of code.
 Because it caches key=>value data it avoids the need to re-fetch things from stores after the first request. Its good for performance, bad for memory.
+Memeory use can be controlled by setting the persistentmaxsize option.
 It should be used sparingly.
 
 The mappingsonly option.

cache/classes/definition.php

@@ -77,15 +77,10 @@
  *     + datasourcefile
  *          [string] Suplements the above setting indicated the file containing the class to be used. This file is included when
  *          required.
- *     + persistent
- *          [bool] This setting does two important things. First it tells the cache API to only instantiate the cache structure for
- *          this definition once, further requests will be given the original instance.
- *          Second the cache loader will keep an array of the items set and retrieved to the cache during the request.
- *          This has several advantages including better performance without needing to start passing the cache instance between
- *          function calls, the downside is that the cache instance + the items used stay within memory.
- *          Consider using this setting when you know that there are going to be many calls to the cache for the same information
- *          or when you are converting existing code to the cache and need to access the cache within functions but don't want
- *          to add it as an argument to the function.
+ *     + persistentdata
+ *          The cache loader will keep an array of the items set and retrieved to the cache during the request.
+ *          Consider using this setting when you know that there are going to be many calls to the cache for the same information.
+ *          Requests for data in this array will be ultra fast, but it will cost memory.
  *     + persistentmaxsize
  *          [int] This supplements the above setting by limiting the number of items in the caches persistent array of items.
  *          Tweaking this setting lower will allow you to minimise the memory implications above while hopefully still managing to
@@ -253,10 +248,10 @@ class cache_definition {
     protected $datasourceaggregate = null;
 
     /**
-     * Set to true if the definitions cache should be persistent
+     * Set to true if the cache should hold onto items passing through it to speed up subsequent requests.
      * @var bool
      */
-    protected $persistent = false;
+    protected $persistentdata = false;
 
     /**
      * The persistent item array max size.
@@ -363,7 +358,7 @@ public static function load($id, array $definition, $datasourceaggregate = null)
         $overrideclassfile = null;
         $datasource = null;
         $datasourcefile = null;
-        $persistent = false;
+        $persistentdata = false;
         $persistentmaxsize = false;
         $ttl = 0;
         $mappingsonly = false;
@@ -419,7 +414,11 @@ public static function load($id, array $definition, $datasourceaggregate = null)
         }
 
         if (array_key_exists('persistent', $definition)) {
-            $persistent = (bool)$definition['persistent'];
+            // Ahhh this is the legacy persistent option.
+            $persistentdata = (bool)$definition['persistent'];
+        }
+        if (array_key_exists('persistentdata', $definition)) {
+            $persistentdata = (bool)$definition['persistentdata'];
         }
         if (array_key_exists('persistentmaxsize', $definition)) {
             $persistentmaxsize = (int)$definition['persistentmaxsize'];
@@ -518,7 +517,7 @@ public static function load($id, array $definition, $datasourceaggregate = null)
         $cachedefinition->datasource = $datasource;
         $cachedefinition->datasourcefile = $datasourcefile;
         $cachedefinition->datasourceaggregate = $datasourceaggregate;
-        $cachedefinition->persistent = $persistent;
+        $cachedefinition->persistentdata = $persistentdata;
         $cachedefinition->persistentmaxsize = $persistentmaxsize;
         $cachedefinition->ttl = $ttl;
         $cachedefinition->mappingsonly = $mappingsonly;
@@ -543,7 +542,8 @@ public static function load($id, array $definition, $datasourceaggregate = null)
      *   - simplekeys : Set to true if the keys you will use are a-zA-Z0-9_
      *   - simpledata : Set to true if the type of the data you are going to store is scalar, or an array of scalar vars
      *   - overrideclass : The class to use as the loader.
-     *   - persistent : If set to true the cache will persist construction requests.
+     *   - persistentdata : If set to true the cache will hold onto data passing through it.
+     *   - persistentmaxsize : Set it to an int to limit the size of the persistentdata cache.
      * @return cache_application|cache_session|cache_request
      */
     public static function load_adhoc($mode, $component, $area, array $options = array()) {
@@ -560,7 +560,14 @@ public static function load_adhoc($mode, $component, $area, array $options = arr
             $definition['simpledata'] = $options['simpledata'];
         }
         if (!empty($options['persistent'])) {
-            $definition['persistent'] = $options['persistent'];
+            // Ahhh this is the legacy persistent option.
+            $definition['persistentdata'] = (bool)$options['persistent'];
+        }
+        if (!empty($options['persistentdata'])) {
+            $definition['persistentdata'] = (bool)$options['persistentdata'];
+        }
+        if (!empty($options['persistentmaxsize'])) {
+            $definition['persistentmaxsize'] = (int)$options['persistentmaxsize'];
         }
         if (!empty($options['overrideclass'])) {
             $definition['overrideclass'] = $options['overrideclass'];
@@ -788,10 +795,26 @@ public function get_requirements_bin() {
 
     /**
      * Returns true if this definitions cache should be made persistent.
+     *
+     * Please call data_should_be_persistent instead.
+     *
+     * @deprecated since 2.6
      * @return bool
      */
     public function should_be_persistent() {
-        return $this->persistent || $this->mode === cache_store::MODE_SESSION;
+        return $this->data_should_be_persistent();
+    }
+
+    /**
+     * Returns true if the cache loader for this definition should be persistent.
+     * @return bool
+     */
+    public function data_should_be_persistent() {
+        if ($this->mode === cache_store::MODE_REQUEST) {
+            // Request caches should never use persistent data - it just doesn't make sense.
+            return false;
+        }
+        return $this->persistentdata || $this->mode === cache_store::MODE_SESSION;
     }
 
     /**

cache/classes/dummystore.php

@@ -112,7 +112,7 @@ public function initialise(cache_definition $definition) {
         //     store things in its persistent cache.
         //   - If the definition is not persistent then the cache loader won't try to store anything
         //     and we will need to store it here in order to make sure it is accessible.
-        $this->persist = !$definition->should_be_persistent();
+        $this->persist = !$definition->data_should_be_persistent();
     }
 
     /**

cache/classes/factory.php

@@ -150,16 +150,26 @@ protected function __construct() {
      */
     public static function reset() {
         $factory = self::instance();
-        $factory->cachesfromdefinitions = array();
-        $factory->cachesfromparams = array();
-        $factory->stores = array();
+        $factory->reset_cache_instances();
         $factory->configs = array();
         $factory->definitions = array();
         $factory->lockplugins = array(); // MUST be null in order to force its regeneration.
         // Reset the state to uninitialised.
         $factory->state = self::STATE_UNINITIALISED;
     }
 
+    /**
+     * Resets the stores, clearing the array of created stores.
+     *
+     * Cache objects still held onto by the code that initialised them will remain as is
+     * however all future requests for a cache/store will lead to a new instance being re-initialised.
+     */
+    public function reset_cache_instances() {
+        $this->cachesfromdefinitions = array();
+        $this->cachesfromparams = array();
+        $this->stores = array();
+    }
+
     /**
      * Creates a cache object given the parameters for a definition.
      *
@@ -181,9 +191,8 @@ public function create_cache_from_definition($component, $area, array $identifie
         $definition = $this->create_definition($component, $area, $aggregate);
         $definition->set_identifiers($identifiers);
         $cache = $this->create_cache($definition, $identifiers);
-        if ($definition->should_be_persistent()) {
-            $this->cachesfromdefinitions[$definitionname] = $cache;
-        }
+        // Loaders are always held onto to speed up subsequent requests.
+        $this->cachesfromdefinitions[$definitionname] = $cache;
         return $cache;
     }
 
@@ -210,9 +219,7 @@ public function create_cache_from_params($mode, $component, $area, array $identi
         $definition = cache_definition::load_adhoc($mode, $component, $area, $options);
         $definition->set_identifiers($identifiers);
         $cache = $this->create_cache($definition, $identifiers);
-        if ($definition->should_be_persistent()) {
-            $this->cachesfromparams[$key] = $cache;
-        }
+        $this->cachesfromparams[$key] = $cache;
         return $cache;
     }
 
@@ -585,7 +592,9 @@ public function stores_disabled() {
      * </code>
      */
     public static function disable_stores() {
+        // First reset to clear any persistent caches.
         $factory = self::instance();
+        $factory->reset_cache_instances();
         $factory->set_state(self::STATE_STORES_DISABLED);
     }
 }

cache/classes/helper.php

@@ -244,7 +244,8 @@ public static function invalidate_by_event($event, array $keys) {
                 // OK at this point we know that the definition has information to invalidate on the event.
                 // There are two routes, either its an application cache in which case we can invalidate it now.
                 // or it is a persistent cache that also needs to be invalidated now.
-                if ($definition->get_mode() === cache_store::MODE_APPLICATION || $definition->should_be_persistent()) {
+                $applicationcache = $definition->get_mode() === cache_store::MODE_APPLICATION;
+                if ($applicationcache || $definition->data_should_be_persistent()) {
                     $cache = $factory->create_cache_from_definition($definition->get_component(), $definition->get_area());
                     $cache->delete_many($keys);
                 }
@@ -315,8 +316,8 @@ public static function purge_by_event($event) {
         foreach ($instance->get_definitions() as $name => $definitionarr) {
             $definition = cache_definition::load($name, $definitionarr);
             if ($definition->invalidates_on_event($event)) {
-                // Check if this definition would result in a persistent loader being in use.
-                if ($definition->should_be_persistent()) {
+                // Check if this definition would result in a loader with persistent data being in use.
+                if ($definition->data_should_be_persistent()) {
                     // There may be a persistent cache loader. Lets purge that first so that any persistent data is removed.
                     $cache = $factory->create_cache_from_definition($definition->get_component(), $definition->get_area());
                     $cache->purge();

cache/classes/loaders.php

@@ -99,7 +99,7 @@ class cache implements cache_loader {
      * There are several other variables to control how this persist cache works.
      * @var bool
      */
-    private $persist = false;
+    private $persistdata = false;
 
     /**
      * The persist cache itself.
@@ -218,8 +218,8 @@ public function __construct(cache_definition $definition, cache_store $store, $l
             $this->datasource = $loader;
         }
         $this->definition->generate_definition_hash();
-        $this->persist = $this->definition->should_be_persistent();
-        if ($this->persist) {
+        $this->persistdata = $this->definition->data_should_be_persistent();
+        if ($this->persistdata) {
             $this->persistmaxsize = $this->definition->get_persistent_max_size();
         }
         $this->hasattl = ($this->definition->get_ttl() > 0);
@@ -239,12 +239,12 @@ protected function set_is_sub_loader($setting = true) {
         if ($setting) {
             $this->subloader = true;
             // Subloaders should not keep persistent data.
-            $this->persist = false;
+            $this->persistdata = false;
             $this->persistmaxsize = false;
         } else {
             $this->subloader = true;
-            $this->persist = $this->definition->should_be_persistent();
-            if ($this->persist) {
+            $this->persistdata = $this->definition->data_should_be_persistent();
+            if ($this->persistdata) {
                 $this->persistmaxsize = $this->definition->get_persistent_max_size();
             }
         }
@@ -913,7 +913,7 @@ protected function store_supports_native_locking() {
      * @return bool
      */
     protected function is_using_persist_cache() {
-        return $this->persist;
+        return $this->persistdata;
     }
 
     /**
@@ -929,7 +929,7 @@ protected function is_in_persist_cache($key) {
         }
         // This could be written as a single line, however it has been split because the ttl check is faster than the instanceof
         // and has_expired calls.
-        if (!$this->persist || !array_key_exists($key, $this->persistcache)) {
+        if (!$this->persistdata || !array_key_exists($key, $this->persistcache)) {
             return false;
         }
         if ($this->has_a_ttl() && $this->store_supports_native_ttl()) {
@@ -953,7 +953,7 @@ protected function get_from_persist_cache($key) {
         // for null values, meaning null values will come from backing store not
         // the persist cache. We think this okay because null usage should be
         // very rare (see comment in MDL-39472).
-        if (!$this->persist || !isset($this->persistcache[$key])) {
+        if (!$this->persistdata || !isset($this->persistcache[$key])) {
             $result = false;
         } else {
             $data = $this->persistcache[$key];

cache/tests/cache_test.php

@@ -149,7 +149,7 @@ public function test_default_application_cache() {
             'mode' => cache_store::MODE_APPLICATION,
             'component' => 'phpunit',
             'area' => 'test_default_application_cache',
-            'persistent' => true,
+            'persistentdata' => true,
             'persistentmaxsize' => 1
         ));
         $cache = cache::make('phpunit', 'test_default_application_cache');
@@ -189,7 +189,7 @@ public function test_on_cache_without_store() {
             'mode' => cache_store::MODE_APPLICATION,
             'component' => 'phpunit',
             'area' => 'nostoretest2',
-            'persistent' => true
+            'persistentdata' => true
         ));
         $instance->phpunit_remove_stores();
 
@@ -924,11 +924,11 @@ public function test_application_event_purge() {
                 'crazyevent'
             )
         ));
-        $instance->phpunit_add_definition('phpunit/eventpurgetestpersistent', array(
+        $instance->phpunit_add_definition('phpunit/eventpurgetestpersistentapp', array(
             'mode' => cache_store::MODE_APPLICATION,
             'component' => 'phpunit',
-            'area' => 'eventpurgetestpersistent',
-            'persistent' => true,
+            'area' => 'eventpurgetestpersistentapp',
+            'persistentdata' => true,
             'invalidationevents' => array(
                 'crazyevent'
             )
@@ -948,7 +948,7 @@ public function test_application_event_purge() {
         $this->assertFalse($cache->get('testkey2'));
 
         // Now test the persistent cache.
-        $cache = cache::make('phpunit', 'eventpurgetestpersistent');
+        $cache = cache::make('phpunit', 'eventpurgetestpersistentapp');
         $this->assertTrue($cache->set('testkey1', 'test data 1'));
         $this->assertEquals('test data 1', $cache->get('testkey1'));
         $this->assertTrue($cache->set('testkey2', 'test data 2'));
@@ -979,7 +979,7 @@ public function test_session_event_purge() {
             'mode' => cache_store::MODE_SESSION,
             'component' => 'phpunit',
             'area' => 'eventpurgetestpersistent',
-            'persistent' => true,
+            'persistentdata' => true,
             'invalidationevents' => array(
                 'crazyevent'
             )
@@ -1400,7 +1400,7 @@ public function test_application_locking() {
             'mode' => cache_store::MODE_APPLICATION,
             'component' => 'phpunit',
             'area' => 'test_application_locking',
-            'persistent' => true,
+            'persistentdata' => true,
             'persistentmaxsize' => 1,
             'requirelockingread' => true,
             'requirelockingwrite' => true
@@ -1535,4 +1535,4 @@ public function test_defaults_support_searching() {
         $this->assertInstanceOf('cache_request', $cache);
         $this->assertArrayHasKey('cache_is_searchable', $cache->phpunit_get_store_implements());
     }
-}
+}