Skip to content

Commit

Permalink
Fix generator handling on getItems(), add test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrerom
andrerom committed Oct 4, 2018
1 parent 6b632f3 commit ae0ee64
Show file tree
Hide file tree
Showing 6 changed files with 352 additions and 61 deletions.
4 changes: 0 additions & 4 deletions eZ/Bundle/EzPublishCoreBundle/Resources/config/cache.yml
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
services:
# This overrides default cache pool in eZ/Publish/Core/settings/storage_engines/cache.yml
ezpublish.cache_pool:
class: eZ\Publish\Core\Persistence\Cache\Adapter\InMemoryCacheAdapter
arguments: ["@ezpublish.cache_pool_inner"]

ezpublish.cache_pool_inner:
class: Symfony\Component\Cache\Adapter\TagAwareAdapterInterface
lazy: true
Expand Down
8 changes: 8 additions & 0 deletions eZ/Publish/API/Repository/Tests/Regression/EnvTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace eZ\Publish\API\Repository\Tests\Regression;

use eZ\Publish\API\Repository\Tests\BaseTest;
use eZ\Publish\Core\Persistence\Cache\Adapter\InMemoryCacheAdapter;
use Symfony\Component\Cache\Adapter\TagAwareAdapter;
use Symfony\Component\Cache\Adapter\TagAwareAdapterInterface;
use Symfony\Component\Cache\Adapter\RedisAdapter;
Expand All @@ -26,6 +27,13 @@ public function testVerifyCacheDriver()
{
$pool = $this->getSetupFactory()->getServiceContainer()->get('ezpublish.cache_pool');

$this->assertInstanceOf(TagAwareAdapterInterface::class, $pool);
$this->assertInstanceOf(InMemoryCacheAdapter::class, $pool);

$reflectionPool = new \ReflectionProperty($pool, 'pool');
$reflectionPool->setAccessible(true);
$pool = $reflectionPool->getValue($pool);

$this->assertInstanceOf(TagAwareAdapterInterface::class, $pool);
$this->assertInstanceOf(TagAwareAdapter::class, $pool);

Expand Down
71 changes: 38 additions & 33 deletions eZ/Publish/Core/Persistence/Cache/Adapter/InMemoryCacheAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ final class InMemoryCacheAdapter implements TagAwareAdapterInterface
/**
* @var TagAwareAdapterInterface
*/
private $inner;
private $pool;

/**
* @var int
Expand All @@ -70,9 +70,9 @@ final class InMemoryCacheAdapter implements TagAwareAdapterInterface
*/
private $ttl;

public function __construct(TagAwareAdapterInterface $inner, int $limit = self::LIMIT, int $ttl = self::TTL)
public function __construct(TagAwareAdapterInterface $pool, int $limit = self::LIMIT, int $ttl = self::TTL)
{
$this->inner = $inner;
$this->pool = $pool;
$this->limit = $limit;
$this->ttl = $ttl;
}
Expand All @@ -83,23 +83,34 @@ public function getItem($key)
return $items[$key];
}

$item = $this->inner->getItem($key);
$this->saveInMemoryCacheItems([$key => $item]);
$item = $this->pool->getItem($key);
if ($item->isHit()) {
$this->saveCacheHitsInMemory([$key => $item]);
}

return $item;
}

public function getItems(array $keys = [])
{
$missingKeys = [];
$items = $this->getValidInMemoryCacheItems($keys, $missingKeys);
foreach ($this->getValidInMemoryCacheItems($keys, $missingKeys) as $key => $item) {
yield $key => $item;
}

if (!empty($missingKeys)) {
$items += $newItems = $this->inner->getItems($missingKeys);
$this->saveInMemoryCacheItems($newItems);
}
$hits = [];
$items = $this->pool->getItems($missingKeys);
foreach ($items as $key => $item) {
yield $key => $item;

return $items;
if ($item->isHit()) {
$hits[$key] = $item;
}
}

$this->saveCacheHitsInMemory($hits);
}
}

public function hasItem($key)
Expand All @@ -109,15 +120,15 @@ public function hasItem($key)
return true;
}

return $this->inner->hasItem($key);
return $this->pool->hasItem($key);
}

public function clear()
{
$this->cacheItems = [];
$this->cacheItemsTS = [];

return $this->inner->clear();
return $this->pool->clear();
}

public function deleteItem($key)
Expand All @@ -126,7 +137,7 @@ public function deleteItem($key)
unset($this->cacheItems[$key], $this->cacheItemsTS[$key]);
}

return $this->inner->deleteItem($key);
return $this->pool->deleteItem($key);
}

public function deleteItems(array $keys)
Expand All @@ -137,28 +148,28 @@ public function deleteItems(array $keys)
}
}

return $this->inner->deleteItems($keys);
return $this->pool->deleteItems($keys);
}

public function save(CacheItemInterface $item)
{
$this->saveInMemoryCacheItems([$item->getKey() => $item]);
$this->saveCacheHitsInMemory([$item->getKey() => $item]);

return $this->inner->save($item);
return $this->pool->save($item);
}

public function saveDeferred(CacheItemInterface $item)
{
// Symfony commits the deferred items as soon as getItem(s) is called on it later or on destruct.
// So seems we can safely save in-memory, also we don't at the time of writing use saveDeferred().
$this->saveInMemoryCacheItems([$item->getKey() => $item]);
$this->saveCacheHitsInMemory([$item->getKey() => $item]);

return $this->inner->saveDeferred($item);
return $this->pool->saveDeferred($item);
}

public function commit()
{
return $this->inner->commit();
return $this->pool->commit();
}

public function invalidateTags(array $tags)
Expand All @@ -170,14 +181,15 @@ public function invalidateTags(array $tags)
}
}

return $this->inner->invalidateTags($tags);
return $this->pool->invalidateTags($tags);
}

/**
* @param \Psr\Cache\CacheItemInterface[] $items Cache items with cache key as array key.
* @param \Psr\Cache\CacheItemInterface[] $items Save Cache hits in-memory with cache key as array key.
*/
private function saveInMemoryCacheItems(array $items): void
private function saveCacheHitsInMemory(array $items): void
{
// Skip if empty
if (empty($items)) {
return;
}
Expand All @@ -187,13 +199,6 @@ private function saveInMemoryCacheItems(array $items): void
return;
}

// Filter out cache misses as they will immediately be re-generated
foreach ($items as $key => $item) {
if (!$item->isHit()) {
unset($items[$key]);
}
}

// Will we stay clear of the limit? If so return early
if (\count($items) + \count($this->cacheItems) < $this->limit) {
$this->cacheItems += $items;
Expand All @@ -205,7 +210,7 @@ private function saveInMemoryCacheItems(array $items): void
// # Vacuuming cache in bulk so we don't end up doing this all the time
// 1. Discriminate against content cache, remove up to 1/3 of max limit starting from oldest items
$removeCount = 0;
$removeTarget = floor($this->limit / 3);
$removeTarget = \floor($this->limit / 3);
foreach ($this->cacheItems as $key => $item) {
$cache = $item->get();
foreach (self::CONTENT_CLASSES as $className) {
Expand All @@ -222,10 +227,10 @@ private function saveInMemoryCacheItems(array $items): void
}
}

// 2. Does cache still exceed the 80% of limit? if so remove everything above 66%
// 2. Does cache still exceed the 66% of limit? if so remove everything above 66%
// NOTE: This on purpose keeps the oldest cache around, getValidInMemoryCacheItems() handles ttl checks on that
if (\count($this->cacheItems) >= $this->limit / 1.5) {
$this->cacheItems = \array_slice($this->cacheItems, 0, floor($this->limit / 1.5));
$this->cacheItems = \array_slice($this->cacheItems, 0, \floor($this->limit / 1.5));
}

$this->cacheItems += $items;
Expand All @@ -240,7 +245,7 @@ private function saveInMemoryCacheItems(array $items): void
*/
public function getValidInMemoryCacheItems(array $keys = [], array &$missingKeys = []): array
{
// 1. Validate TTL and remove items that have exceeded it
// 1. Validate TTL and remove items that have exceeded it (on purpose not prefixed for global scope, see tests)
$expiredTime = time() - $this->ttl;
foreach ($this->cacheItemsTS as $key => $ts) {
if ($ts <= $expiredTime) {
Expand Down
Loading

0 comments on commit ae0ee64

Please sign in to comment.