Skip to content

Commit

Permalink
feature #27563 [Cache] Improve perf of array-based pools (nicolas-gre…
Browse files Browse the repository at this point in the history 
…kas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Cache] Improve perf of array-based pools

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

- skip key validation when key is already known
- remove overhead in `ArrayCache::get()` via inlining
- don't store simple values in serialized format when not needed, preserving COW

~~Needs #27565 to be green.~~

Commits
-------

92a2d47 [Cache] Improve perf of array-based pools
  • Loading branch information
@fabpot
fabpot committed Jun 18, 2018
2 parents 137dd76 + 92a2d47 commit d075d0c
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 63 deletions.
33 changes: 8 additions & 25 deletions src/Symfony/Component/Cache/Adapter/ArrayAdapter.php
View file
Expand Up @@ -56,22 +56,10 @@ function ($key, $value, $isHit) use ($defaultLifetime) {
*/
public function getItem($key)
{
$isHit = $this->hasItem($key);
try {
if (!$isHit) {
$this->values[$key] = $value = null;
} elseif (!$this->storeSerialized) {
$value = $this->values[$key];
} elseif ('b:0;' === $value = $this->values[$key]) {
$value = false;
} elseif (false === $value = unserialize($value)) {
$this->values[$key] = $value = null;
$isHit = false;
}
} catch (\Exception $e) {
CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e));
if (!$isHit = $this->hasItem($key)) {
$this->values[$key] = $value = null;
$isHit = false;
} else {
$value = $this->storeSerialized ? $this->unfreeze($key, $isHit) : $this->values[$key];
}
$f = $this->createCacheItem;

Expand All @@ -84,7 +72,9 @@ public function getItem($key)
public function getItems(array $keys = array())
{
foreach ($keys as $key) {
CacheItem::validateKey($key);
if (!\is_string($key) || !isset($this->expiries[$key])) {
CacheItem::validateKey($key);
}
}

return $this->generateItems($keys, microtime(true), $this->createCacheItem);
Expand Down Expand Up @@ -120,15 +110,8 @@ public function save(CacheItemInterface $item)

return true;
}
if ($this->storeSerialized) {
try {
$value = serialize($value);
} catch (\Exception $e) {
$type = is_object($value) ? get_class($value) : gettype($value);
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));

return false;
}
if ($this->storeSerialized && null === $value = $this->freeze($value)) {
return false;
}
if (null === $expiry && 0 < $item["\0*\0defaultLifetime"]) {
$expiry = microtime(true) + $item["\0*\0defaultLifetime"];
Expand Down
42 changes: 25 additions & 17 deletions src/Symfony/Component/Cache/Simple/ArrayCache.php
View file
Expand Up @@ -45,9 +45,20 @@ public function __construct(int $defaultLifetime = 0, bool $storeSerialized = tr
*/
public function get($key, $default = null)
{
foreach ($this->getMultiple(array($key), $default) as $v) {
return $v;
if (!\is_string($key) || !isset($this->expiries[$key])) {
CacheItem::validateKey($key);
}
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] > microtime(true) || !$this->delete($key))) {
$this->values[$key] = null;

return $default;
}
if (!$this->storeSerialized) {
return $this->values[$key];
}
$value = $this->unfreeze($key, $isHit);

return $isHit ? $value : $default;
}

/**
Expand All @@ -61,7 +72,9 @@ public function getMultiple($keys, $default = null)
throw new InvalidArgumentException(sprintf('Cache keys must be array or Traversable, "%s" given', is_object($keys) ? get_class($keys) : gettype($keys)));
}
foreach ($keys as $key) {
CacheItem::validateKey($key);
if (!\is_string($key) || !isset($this->expiries[$key])) {
CacheItem::validateKey($key);
}
}

return $this->generateItems($keys, microtime(true), function ($k, $v, $hit) use ($default) { return $hit ? $v : $default; });
Expand All @@ -87,7 +100,9 @@ public function deleteMultiple($keys)
*/
public function set($key, $value, $ttl = null)
{
CacheItem::validateKey($key);
if (!\is_string($key)) {
CacheItem::validateKey($key);
}

return $this->setMultiple(array($key => $value), $ttl);
}
Expand All @@ -103,27 +118,20 @@ public function setMultiple($values, $ttl = null)
$valuesArray = array();

foreach ($values as $key => $value) {
\is_int($key) || CacheItem::validateKey($key);
if (!\is_int($key) && !(\is_string($key) && isset($this->expiries[$key]))) {
CacheItem::validateKey($key);
}
$valuesArray[$key] = $value;
}
if (false === $ttl = $this->normalizeTtl($ttl)) {
return $this->deleteMultiple(array_keys($valuesArray));
}
if ($this->storeSerialized) {
foreach ($valuesArray as $key => $value) {
try {
$valuesArray[$key] = serialize($value);
} catch (\Exception $e) {
$type = is_object($value) ? get_class($value) : gettype($value);
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));

return false;
}
}
}
$expiry = 0 < $ttl ? microtime(true) + $ttl : PHP_INT_MAX;

foreach ($valuesArray as $key => $value) {
if ($this->storeSerialized && null === $value = $this->freeze($value)) {
return false;
}
$this->values[$key] = $value;
$this->expiries[$key] = $expiry;
}
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Cache/Tests/Adapter/ArrayAdapterTest.php
View file
Expand Up @@ -36,12 +36,12 @@ public function testGetValuesHitAndMiss()

// Hit
$item = $cache->getItem('foo');
$item->set('4711');
$item->set('::4711');
$cache->save($item);

$fooItem = $cache->getItem('foo');
$this->assertTrue($fooItem->isHit());
$this->assertEquals('4711', $fooItem->get());
$this->assertEquals('::4711', $fooItem->get());

// Miss (should be present as NULL in $values)
$cache->getItem('bar');
Expand All @@ -50,7 +50,7 @@ public function testGetValuesHitAndMiss()

$this->assertCount(2, $values);
$this->assertArrayHasKey('foo', $values);
$this->assertSame(serialize('4711'), $values['foo']);
$this->assertSame(serialize('::4711'), $values['foo']);
$this->assertArrayHasKey('bar', $values);
$this->assertNull($values['bar']);
}
Expand Down
92 changes: 74 additions & 18 deletions src/Symfony/Component/Cache/Traits/ArrayTrait.php
View file
Expand Up @@ -34,17 +34,34 @@ trait ArrayTrait
*/
public function getValues()
{
return $this->values;
if (!$this->storeSerialized) {
return $this->values;
}

$values = $this->values;
foreach ($values as $k => $v) {
if (null === $v || 'N;' === $v) {
continue;
}
if (!\is_string($v) || !isset($v[2]) || ':' !== $v[1]) {
$values[$k] = serialize($v);
}
}

return $values;
}

/**
* {@inheritdoc}
*/
public function hasItem($key)
{
if (\is_string($key) && isset($this->expiries[$key]) && $this->expiries[$key] > microtime(true)) {
return true;
}
CacheItem::validateKey($key);

return isset($this->expiries[$key]) && ($this->expiries[$key] > microtime(true) || !$this->deleteItem($key));
return isset($this->expiries[$key]) && !$this->deleteItem($key);
}

/**
Expand All @@ -62,8 +79,9 @@ public function clear()
*/
public function deleteItem($key)
{
CacheItem::validateKey($key);

if (!\is_string($key) || !isset($this->expiries[$key])) {
CacheItem::validateKey($key);
}
unset($this->values[$key], $this->expiries[$key]);

return true;
Expand All @@ -80,21 +98,10 @@ public function reset()
private function generateItems(array $keys, $now, $f)
{
foreach ($keys as $i => $key) {
try {
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] > $now || !$this->deleteItem($key))) {
$this->values[$key] = $value = null;
} elseif (!$this->storeSerialized) {
$value = $this->values[$key];
} elseif ('b:0;' === $value = $this->values[$key]) {
$value = false;
} elseif (false === $value = unserialize($value)) {
$this->values[$key] = $value = null;
$isHit = false;
}
} catch (\Exception $e) {
CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e));
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] > $now || !$this->deleteItem($key))) {
$this->values[$key] = $value = null;
$isHit = false;
} else {
$value = $this->storeSerialized ? $this->unfreeze($key, $isHit) : $this->values[$key];
}
unset($keys[$i]);

Expand All @@ -105,4 +112,53 @@ private function generateItems(array $keys, $now, $f)
yield $key => $f($key, null, false);
}
}

private function freeze($value)
{
if (null === $value) {
return 'N;';
}
if (\is_string($value)) {
// Serialize strings if they could be confused with serialized objects or arrays
if ('N;' === $value || (isset($value[2]) && ':' === $value[1])) {
return serialize($value);
}
} elseif (!\is_scalar($value)) {
try {
$serialized = serialize($value);
} catch (\Exception $e) {
$type = is_object($value) ? get_class($value) : gettype($value);
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));

return;
}
// Keep value serialized if it contains any objects or any internal references
if ('C' === $serialized[0] || 'O' === $serialized[0] || preg_match('/;[OCRr]:[1-9]/', $serialized)) {
return $serialized;
}
}

return $value;
}

private function unfreeze(string $key, bool &$isHit)
{
if ('N;' === $value = $this->values[$key]) {
return null;
}
if (\is_string($value) && isset($value[2]) && ':' === $value[1]) {
try {
$value = unserialize($value);
} catch (\Exception $e) {
CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e));
$value = false;
}
if (false === $value) {
$this->values[$key] = $value = null;
$isHit = false;
}
}

return $value;
}
}

0 comments on commit d075d0c

Please sign in to comment.