Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
minor #26984 [Cache] Inline some hot function calls (nicolas-grekas)
This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] Inline some hot function calls

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

The Cache component is typically found in critical code paths, let's inline some calls.
I didn't change all calls but only those in potential hot paths.
Submitted against 3.4 to reduce future merge conflicts.

The PR also embeds edge-case behavior fixes for `ChainAdapter`.

Commits
-------

52b4bfc [Cache] Inline some hot function calls
  • Loading branch information
nicolas-grekas committed Apr 20, 2018
2 parents d0928fc + 52b4bfc commit bf871f4
Show file tree
Hide file tree
Showing 15 changed files with 86 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
Expand Up @@ -237,7 +237,7 @@ public function commit()
if (true === $e || array() === $e) {
continue;
}
if (is_array($e) || 1 === count($values)) {
if (\is_array($e) || 1 === \count($values)) {
foreach (is_array($e) ? $e : array_keys($values) as $id) {
$ok = false;
$v = $values[$id];
Expand Down
39 changes: 24 additions & 15 deletions src/Symfony/Component/Cache/Adapter/ChainAdapter.php
Expand Up @@ -30,13 +30,13 @@ class ChainAdapter implements AdapterInterface, PruneableInterface, ResettableIn
{
private $adapters = array();
private $adapterCount;
private $saveUp;
private $syncItem;

/**
* @param CacheItemPoolInterface[] $adapters The ordered list of adapters used to fetch cached items
* @param int $maxLifetime The max lifetime of items propagated from lower adapters to upper ones
* @param CacheItemPoolInterface[] $adapters The ordered list of adapters used to fetch cached items
* @param int $defaultLifetime The default lifetime of items propagated from lower adapters to upper ones
*/
public function __construct(array $adapters, $maxLifetime = 0)
public function __construct(array $adapters, $defaultLifetime = 0)
{
if (!$adapters) {
throw new InvalidArgumentException('At least one adapter must be specified.');
Expand All @@ -55,16 +55,20 @@ public function __construct(array $adapters, $maxLifetime = 0)
}
$this->adapterCount = count($this->adapters);

$this->saveUp = \Closure::bind(
function ($adapter, $item) use ($maxLifetime) {
$origDefaultLifetime = $item->defaultLifetime;
$this->syncItem = \Closure::bind(
function ($sourceItem, $item) use ($defaultLifetime) {
$item->value = $sourceItem->value;
$item->expiry = $sourceItem->expiry;
$item->isHit = $sourceItem->isHit;

if (0 < $maxLifetime && ($origDefaultLifetime <= 0 || $maxLifetime < $origDefaultLifetime)) {
$item->defaultLifetime = $maxLifetime;
if (0 < $sourceItem->defaultLifetime && $sourceItem->defaultLifetime < $defaultLifetime) {
$defaultLifetime = $sourceItem->defaultLifetime;
}
if (0 < $defaultLifetime && ($item->defaultLifetime <= 0 || $defaultLifetime < $item->defaultLifetime)) {
$item->defaultLifetime = $defaultLifetime;
}

$adapter->save($item);
$item->defaultLifetime = $origDefaultLifetime;
return $item;
},
null,
CacheItem::class
Expand All @@ -76,18 +80,21 @@ function ($adapter, $item) use ($maxLifetime) {
*/
public function getItem($key)
{
$saveUp = $this->saveUp;
$syncItem = $this->syncItem;
$misses = array();

foreach ($this->adapters as $i => $adapter) {
$item = $adapter->getItem($key);

if ($item->isHit()) {
while (0 <= --$i) {
$saveUp($this->adapters[$i], $item);
$this->adapters[$i]->save($syncItem($item, $misses[$i]));
}

return $item;
}

$misses[$i] = $item;
}

return $item;
Expand All @@ -104,6 +111,7 @@ public function getItems(array $keys = array())
private function generateItems($items, $adapterIndex)
{
$missing = array();
$misses = array();
$nextAdapterIndex = $adapterIndex + 1;
$nextAdapter = isset($this->adapters[$nextAdapterIndex]) ? $this->adapters[$nextAdapterIndex] : null;

Expand All @@ -112,17 +120,18 @@ private function generateItems($items, $adapterIndex)
yield $k => $item;
} else {
$missing[] = $k;
$misses[$k] = $item;
}
}

if ($missing) {
$saveUp = $this->saveUp;
$syncItem = $this->syncItem;
$adapter = $this->adapters[$adapterIndex];
$items = $this->generateItems($nextAdapter->getItems($missing), $nextAdapterIndex);

foreach ($items as $k => $item) {
if ($item->isHit()) {
$saveUp($adapter, $item);
$adapter->save($syncItem($item, $misses[$k]));
}

yield $k => $item;
Expand Down
14 changes: 7 additions & 7 deletions src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php
Expand Up @@ -84,7 +84,7 @@ public static function create($file, CacheItemPoolInterface $fallbackPool)
*/
public function getItem($key)
{
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given.', is_object($key) ? get_class($key) : gettype($key)));
}
if (null === $this->values) {
Expand All @@ -99,7 +99,7 @@ public function getItem($key)

if ('N;' === $value) {
$value = null;
} elseif (is_string($value) && isset($value[2]) && ':' === $value[1]) {
} elseif (\is_string($value) && isset($value[2]) && ':' === $value[1]) {
try {
$e = null;
$value = unserialize($value);
Expand All @@ -123,7 +123,7 @@ public function getItem($key)
public function getItems(array $keys = array())
{
foreach ($keys as $key) {
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given.', is_object($key) ? get_class($key) : gettype($key)));
}
}
Expand All @@ -139,7 +139,7 @@ public function getItems(array $keys = array())
*/
public function hasItem($key)
{
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given.', is_object($key) ? get_class($key) : gettype($key)));
}
if (null === $this->values) {
Expand All @@ -154,7 +154,7 @@ public function hasItem($key)
*/
public function deleteItem($key)
{
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given.', is_object($key) ? get_class($key) : gettype($key)));
}
if (null === $this->values) {
Expand All @@ -173,7 +173,7 @@ public function deleteItems(array $keys)
$fallbackKeys = array();

foreach ($keys as $key) {
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given.', is_object($key) ? get_class($key) : gettype($key)));
}

Expand Down Expand Up @@ -240,7 +240,7 @@ private function generateItems(array $keys)

if ('N;' === $value) {
yield $key => $f($key, null, true);
} elseif (is_string($value) && isset($value[2]) && ':' === $value[1]) {
} elseif (\is_string($value) && isset($value[2]) && ':' === $value[1]) {
try {
yield $key => $f($key, unserialize($value), true);
} catch (\Error $e) {
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php
Expand Up @@ -107,7 +107,7 @@ function (AdapterInterface $tagsAdapter, array $tags) {
public function invalidateTags(array $tags)
{
foreach ($tags as $k => $tag) {
if ('' !== $tag && is_string($tag)) {
if ('' !== $tag && \is_string($tag)) {
$tags[$k] = $tag.static::TAGS_PREFIX;
}
}
Expand Down Expand Up @@ -161,7 +161,7 @@ public function getItems(array $keys = array())
$tagKeys = array();

foreach ($keys as $key) {
if ('' !== $key && is_string($key)) {
if ('' !== $key && \is_string($key)) {
$key = static::TAGS_PREFIX.$key;
$tagKeys[$key] = $key;
}
Expand Down Expand Up @@ -202,7 +202,7 @@ public function deleteItem($key)
public function deleteItems(array $keys)
{
foreach ($keys as $key) {
if ('' !== $key && is_string($key)) {
if ('' !== $key && \is_string($key)) {
$keys[] = static::TAGS_PREFIX.$key;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Cache/CacheItem.php
Expand Up @@ -89,7 +89,7 @@ public function expiresAfter($time)
$this->expiry = $this->defaultLifetime > 0 ? time() + $this->defaultLifetime : null;
} elseif ($time instanceof \DateInterval) {
$this->expiry = (int) \DateTime::createFromFormat('U', time())->add($time)->format('U');
} elseif (is_int($time)) {
} elseif (\is_int($time)) {
$this->expiry = $time + time();
} else {
throw new InvalidArgumentException(sprintf('Expiration date must be an integer, a DateInterval or null, "%s" given', is_object($time) ? get_class($time) : gettype($time)));
Expand All @@ -109,17 +109,17 @@ public function expiresAfter($time)
*/
public function tag($tags)
{
if (!is_array($tags)) {
if (!\is_array($tags)) {
$tags = array($tags);
}
foreach ($tags as $tag) {
if (!is_string($tag)) {
if (!\is_string($tag)) {
throw new InvalidArgumentException(sprintf('Cache tag must be string, "%s" given', is_object($tag) ? get_class($tag) : gettype($tag)));
}
if (isset($this->tags[$tag])) {
continue;
}
if (!isset($tag[0])) {
if ('' === $tag) {
throw new InvalidArgumentException('Cache tag length must be greater than zero');
}
if (false !== strpbrk($tag, '{}()/\@:')) {
Expand Down Expand Up @@ -152,10 +152,10 @@ public function getPreviousTags()
*/
public static function validateKey($key)
{
if (!is_string($key)) {
if (!\is_string($key)) {
throw new InvalidArgumentException(sprintf('Cache key must be string, "%s" given', is_object($key) ? get_class($key) : gettype($key)));
}
if (!isset($key[0])) {
if ('' === $key) {
throw new InvalidArgumentException('Cache key length must be greater than zero');
}
if (false !== strpbrk($key, '{}()/\@:')) {
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Cache/Simple/AbstractCache.php
Expand Up @@ -79,7 +79,7 @@ public function getMultiple($keys, $default = null)
{
if ($keys instanceof \Traversable) {
$keys = iterator_to_array($keys, false);
} elseif (!is_array($keys)) {
} elseif (!\is_array($keys)) {
throw new InvalidArgumentException(sprintf('Cache keys must be array or Traversable, "%s" given', is_object($keys) ? get_class($keys) : gettype($keys)));
}
$ids = array();
Expand All @@ -103,13 +103,13 @@ public function getMultiple($keys, $default = null)
*/
public function setMultiple($values, $ttl = null)
{
if (!is_array($values) && !$values instanceof \Traversable) {
if (!\is_array($values) && !$values instanceof \Traversable) {
throw new InvalidArgumentException(sprintf('Cache values must be array or Traversable, "%s" given', is_object($values) ? get_class($values) : gettype($values)));
}
$valuesById = array();

foreach ($values as $key => $value) {
if (is_int($key)) {
if (\is_int($key)) {
$key = (string) $key;
}
$valuesById[$this->getId($key)] = $value;
Expand All @@ -126,7 +126,7 @@ public function setMultiple($values, $ttl = null)
return true;
}
$keys = array();
foreach (is_array($e) ? $e : array_keys($valuesById) as $id) {
foreach (\is_array($e) ? $e : array_keys($valuesById) as $id) {
$keys[] = substr($id, strlen($this->namespace));
}
CacheItem::log($this->logger, 'Failed to save values', array('keys' => $keys, 'exception' => $e instanceof \Exception ? $e : null));
Expand All @@ -141,7 +141,7 @@ public function deleteMultiple($keys)
{
if ($keys instanceof \Traversable) {
$keys = iterator_to_array($keys, false);
} elseif (!is_array($keys)) {
} elseif (!\is_array($keys)) {
throw new InvalidArgumentException(sprintf('Cache keys must be array or Traversable, "%s" given', is_object($keys) ? get_class($keys) : gettype($keys)));
}

Expand All @@ -156,7 +156,7 @@ private function normalizeTtl($ttl)
if ($ttl instanceof \DateInterval) {
$ttl = (int) \DateTime::createFromFormat('U', 0)->add($ttl)->format('U');
}
if (is_int($ttl)) {
if (\is_int($ttl)) {
return 0 < $ttl ? $ttl : false;
}

Expand Down
10 changes: 5 additions & 5 deletions src/Symfony/Component/Cache/Simple/ArrayCache.php
Expand Up @@ -57,7 +57,7 @@ public function getMultiple($keys, $default = null)
{
if ($keys instanceof \Traversable) {
$keys = iterator_to_array($keys, false);
} elseif (!is_array($keys)) {
} elseif (!\is_array($keys)) {
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) {
Expand All @@ -72,7 +72,7 @@ public function getMultiple($keys, $default = null)
*/
public function deleteMultiple($keys)
{
if (!is_array($keys) && !$keys instanceof \Traversable) {
if (!\is_array($keys) && !$keys instanceof \Traversable) {
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) {
Expand All @@ -97,13 +97,13 @@ public function set($key, $value, $ttl = null)
*/
public function setMultiple($values, $ttl = null)
{
if (!is_array($values) && !$values instanceof \Traversable) {
if (!\is_array($values) && !$values instanceof \Traversable) {
throw new InvalidArgumentException(sprintf('Cache values must be array or Traversable, "%s" given', is_object($values) ? get_class($values) : gettype($values)));
}
$valuesArray = array();

foreach ($values as $key => $value) {
is_int($key) || CacheItem::validateKey($key);
\is_int($key) || CacheItem::validateKey($key);
$valuesArray[$key] = $value;
}
if (false === $ttl = $this->normalizeTtl($ttl)) {
Expand Down Expand Up @@ -139,7 +139,7 @@ private function normalizeTtl($ttl)
if ($ttl instanceof \DateInterval) {
$ttl = (int) \DateTime::createFromFormat('U', 0)->add($ttl)->format('U');
}
if (is_int($ttl)) {
if (\is_int($ttl)) {
return 0 < $ttl ? $ttl : false;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Cache/Simple/ChainCache.php
Expand Up @@ -58,7 +58,7 @@ public function __construct(array $caches, $defaultLifetime = 0)
*/
public function get($key, $default = null)
{
$miss = null !== $default && is_object($default) ? $default : $this->miss;
$miss = null !== $default && \is_object($default) ? $default : $this->miss;

foreach ($this->caches as $i => $cache) {
$value = $cache->get($key, $miss);
Expand All @@ -80,7 +80,7 @@ public function get($key, $default = null)
*/
public function getMultiple($keys, $default = null)
{
$miss = null !== $default && is_object($default) ? $default : $this->miss;
$miss = null !== $default && \is_object($default) ? $default : $this->miss;

return $this->generateItems($this->caches[0]->getMultiple($keys, $miss), 0, $miss, $default);
}
Expand Down

0 comments on commit bf871f4

Please sign in to comment.