diff --git a/cache/stores/session/lib.php b/cache/stores/session/lib.php index 1401ae5f767..82215023ef6 100644 --- a/cache/stores/session/lib.php +++ b/cache/stores/session/lib.php @@ -278,16 +278,21 @@ class cachestore_session extends session_data_store implements cache_is_key_awar * Sets an item in the cache given its key and data value. * * @param string $key The key to use. - * @param mixed $data The data to set.* @param bool $testmaxsize If set to true then we test the maxsize arg and reduce if required. + * @param mixed $data The data to set. + * @param bool $testmaxsize If set to true then we test the maxsize arg and reduce if required. * @return bool True if the operation was a success false otherwise. */ public function set($key, $data, $testmaxsize = true) { + $testmaxsize = ($testmaxsize && $this->maxsize !== false); + if ($testmaxsize) { + $increment = (!isset($this->store[$key])); + } if ($this->ttl == 0) { $this->store[$key][0] = $data; } else { $this->store[$key] = array($data, cache::now()); } - if ($testmaxsize && $this->maxsize !== false) { + if ($testmaxsize && $increment) { $this->storecount++; if ($this->storecount > $this->maxsize) { $this->reduce_for_maxsize(); @@ -426,6 +431,13 @@ class cachestore_session extends session_data_store implements cache_is_key_awar /** * Reduces the size of the array if maxsize has been hit. + * + * This function reduces the size of the store reducing it by 10% of its maxsize. + * It removes the oldest items in the store when doing this. + * The reason it does this an doesn't use a least recently used system is purely the overhead such a system + * requires. The current approach is focused on speed, MUC already adds enough overhead to static/session caches + * and avoiding more is of benefit. + * * @return int */ protected function reduce_for_maxsize() { @@ -433,6 +445,8 @@ class cachestore_session extends session_data_store implements cache_is_key_awar if ($diff < 1) { return 0; } + // Reduce it by an extra 10% to avoid calling this repetitively if we are in a loop. + $diff += floor($this->maxsize / 10); $this->store = array_slice($this->store, $diff, null, true); $this->storecount -= $diff; return $diff; diff --git a/cache/stores/session/tests/session_test.php b/cache/stores/session/tests/session_test.php index bfc24cca005..3ee20f2e2d1 100644 --- a/cache/stores/session/tests/session_test.php +++ b/cache/stores/session/tests/session_test.php @@ -92,6 +92,12 @@ class cachestore_session_test extends cachestore_tests { $this->assertTrue($instance->set('key7', 'value7')); $this->assertEquals('value4', $instance->get('key4')); + // Set the same key three times to make sure it doesn't count overrides. + for ($i = 0; $i < 3; $i++) { + $this->assertTrue($instance->set('key8', 'value8')); + } + $this->assertEquals('value7', $instance->get('key7'), 'Overrides are incorrectly incrementing size'); + // Test adding many. $this->assertEquals(3, $instance->set_many(array( array('key' => 'keyA', 'value' => 'valueA'), diff --git a/cache/stores/static/lib.php b/cache/stores/static/lib.php index d49ca35bd27..e54ee838466 100644 --- a/cache/stores/static/lib.php +++ b/cache/stores/static/lib.php @@ -279,12 +279,16 @@ class cachestore_static extends static_data_store implements cache_is_key_aware, * @return bool True if the operation was a success false otherwise. */ public function set($key, $data, $testmaxsize = true) { + $testmaxsize = ($testmaxsize && $this->maxsize !== false); + if ($testmaxsize) { + $increment = (!isset($this->store[$key])); + } if ($this->ttl == 0) { $this->store[$key][0] = $data; } else { $this->store[$key] = array($data, cache::now()); } - if ($testmaxsize && $this->maxsize !== false) { + if ($testmaxsize && $increment) { $this->storecount++; if ($this->storecount > $this->maxsize) { $this->reduce_for_maxsize(); @@ -425,6 +429,13 @@ class cachestore_static extends static_data_store implements cache_is_key_aware, /** * Reduces the size of the array if maxsize has been hit. + * + * This function reduces the size of the store reducing it by 10% of its maxsize. + * It removes the oldest items in the store when doing this. + * The reason it does this an doesn't use a least recently used system is purely the overhead such a system + * requires. The current approach is focused on speed, MUC already adds enough overhead to static/session caches + * and avoiding more is of benefit. + * * @return int */ protected function reduce_for_maxsize() { @@ -432,6 +443,8 @@ class cachestore_static extends static_data_store implements cache_is_key_aware, if ($diff < 1) { return 0; } + // Reduce it by an extra 10% to avoid calling this repetitively if we are in a loop. + $diff += floor($this->maxsize / 10); $this->store = array_slice($this->store, $diff, null, true); $this->storecount -= $diff; return $diff; diff --git a/cache/stores/static/tests/static_test.php b/cache/stores/static/tests/static_test.php index a1fb0469b25..c9795d176b8 100644 --- a/cache/stores/static/tests/static_test.php +++ b/cache/stores/static/tests/static_test.php @@ -1,5 +1,5 @@ assertTrue($instance->set('key7', 'value7')); $this->assertEquals('value4', $instance->get('key4')); + // Set the same key three times to make sure it doesn't count overrides. + for ($i = 0; $i < 3; $i++) { + $this->assertTrue($instance->set('key8', 'value8')); + } + + $this->assertEquals('value7', $instance->get('key7'), 'Overrides are incorrectly incrementing size'); + // Test adding many. $this->assertEquals(3, $instance->set_many(array( array('key' => 'keyA', 'value' => 'valueA'),