Skip to content

Commit c6a7bef

Browse files
dunglasdunglas
authored
Merge pull request #1257 from dunglas/improve_cached_identifiers
Improve the performance of CachedIdentifiersExtractor
2 parents 6ac0834 + b68cbbf commit c6a7bef

File tree

2 files changed

+55
-40
lines changed

2 files changed

+55
-40
lines changed

‎src/Api/CachedIdentifiersExtractor.php

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ final class CachedIdentifiersExtractor implements IdentifiersExtractorInterface
3333
private $cacheItemPool;
3434
private $propertyAccessor;
3535
private $decorated;
36+
private $memoryCache = [];
3637

3738
public function __construct(CacheItemPoolInterface $cacheItemPool, IdentifiersExtractorInterface $decorated, PropertyAccessorInterface $propertyAccessor = null)
3839
{
@@ -46,60 +47,66 @@ public function __construct(CacheItemPoolInterface $cacheItemPool, IdentifiersEx
4647
*/
4748
public function getIdentifiersFromItem($item): array
4849
{
49-
$identifiers = [];
50-
$resourceClass = $this->getObjectClass($item);
51-
52-
$cacheKey = self::CACHE_KEY_PREFIX.md5($resourceClass);
53-
54-
// This is to avoid setting the cache twice in the case where the related item cache doesn't exist
55-
$cacheIsHit = false;
50+
$keys = $this->getKeys($item, function ($item) use (&$identifiers) {
51+
return $identifiers = $this->decorated->getIdentifiersFromItem($item);
52+
});
5653

57-
try {
58-
$cacheItem = $this->cacheItemPool->getItem($cacheKey);
59-
$isRelationCached = true;
60-
61-
if ($cacheIsHit = $cacheItem->isHit()) {
62-
foreach ($cacheItem->get() as $propertyName) {
63-
$identifiers[$propertyName] = $this->propertyAccessor->getValue($item, $propertyName);
54+
if (null !== $identifiers) {
55+
return $identifiers;
56+
}
6457

65-
if (!is_object($identifiers[$propertyName])) {
66-
continue;
67-
}
58+
$identifiers = [];
59+
foreach ($keys as $propertyName) {
60+
$identifiers[$propertyName] = $this->propertyAccessor->getValue($item, $propertyName);
6861

69-
$relatedItem = $identifiers[$propertyName];
70-
$relatedCacheKey = self::CACHE_KEY_PREFIX.md5($this->getObjectClass($relatedItem));
62+
if (!is_object($identifiers[$propertyName])) {
63+
continue;
64+
}
7165

66+
$relatedResourceClass = $this->getObjectClass($identifiers[$propertyName]);
67+
if (!$relatedIdentifiers = $this->memoryCache[$relatedResourceClass] ?? false) {
68+
$relatedCacheKey = self::CACHE_KEY_PREFIX.md5($relatedResourceClass);
69+
try {
7270
$relatedCacheItem = $this->cacheItemPool->getItem($relatedCacheKey);
73-
7471
if (!$relatedCacheItem->isHit()) {
75-
$isRelationCached = false;
76-
break;
72+
return $this->decorated->getIdentifiersFromItem($item);
7773
}
74+
} catch (CacheException $e) {
75+
return $this->decorated->getIdentifiersFromItem($item);
76+
}
77+
78+
$relatedIdentifiers = $relatedCacheItem->get();
79+
}
7880

79-
unset($identifiers[$propertyName]);
81+
$identifiers[$propertyName] = $this->propertyAccessor->getValue($identifiers[$propertyName], $relatedIdentifiers[0]);
82+
}
8083

81-
$identifiers[$propertyName] = $this->propertyAccessor->getValue($relatedItem, $relatedCacheItem->get()[0]);
82-
}
84+
return $identifiers;
85+
}
8386

84-
if (true === $isRelationCached) {
85-
return $identifiers;
86-
}
87+
private function getKeys($item, callable $retriever): array
88+
{
89+
$resourceClass = $this->getObjectClass($item);
90+
if (isset($this->memoryCache[$resourceClass])) {
91+
return $this->memoryCache[$resourceClass];
92+
}
93+
94+
try {
95+
$cacheItem = $this->cacheItemPool->getItem(self::CACHE_KEY_PREFIX.md5($resourceClass));
96+
if ($cacheItem->isHit()) {
97+
return $this->memoryCache[$resourceClass] = $cacheItem->get();
8798
}
8899
} catch (CacheException $e) {
89100
// do nothing
90101
}
91102

92-
$identifiers = $this->decorated->getIdentifiersFromItem($item);
103+
$keys = array_keys($retriever($item));
93104

94-
if (isset($cacheItem) && false === $cacheIsHit) {
95-
try {
96-
$cacheItem->set(array_keys($identifiers));
97-
$this->cacheItemPool->save($cacheItem);
98-
} catch (CacheException $e) {
99-
// do nothing
100-
}
105+
if (isset($cacheItem)) {
106+
$cacheItem->set($keys);
107+
$this->cacheItemPool->save($cacheItem);
101108
}
102109

103-
return $identifiers;
110+
return $this->memoryCache[$resourceClass] = $keys;
104111
}
105112
}

‎tests/Api/CachedIdentifiersExtractorTest.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ public function testFirstPass()
4545

4646
$identifiersExtractor = new CachedIdentifiersExtractor($cacheItemPool->reveal(), $decoration->reveal(), null);
4747

48-
$this->assertEquals(['id' => 1], $identifiersExtractor->getIdentifiersFromItem($dummy));
48+
$expectedResult = ['id' => 1];
49+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy));
50+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy), 'Trigger the local cache');
4951
}
5052

5153
public function testSecondPass()
@@ -67,7 +69,9 @@ public function testSecondPass()
6769

6870
$identifiersExtractor = new CachedIdentifiersExtractor($cacheItemPool->reveal(), $decoration->reveal(), null);
6971

70-
$this->assertEquals(['id' => 1], $identifiersExtractor->getIdentifiersFromItem($dummy));
72+
$expectedResult = ['id' => 1];
73+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy));
74+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy), 'Trigger the local cache');
7175
}
7276

7377
public function testSecondPassWithRelatedNotCached()
@@ -98,7 +102,9 @@ public function testSecondPassWithRelatedNotCached()
98102

99103
$identifiersExtractor = new CachedIdentifiersExtractor($cacheItemPool->reveal(), $decoration->reveal(), null);
100104

105+
$expectedResult = ['id' => 1, 'relatedDummy' => 1];
101106
$this->assertEquals(['id' => 1, 'relatedDummy' => 1], $identifiersExtractor->getIdentifiersFromItem($dummy));
107+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy), 'Trigger the local cache');
102108
}
103109

104110
public function testSecondPassWithRelatedCached()
@@ -130,6 +136,8 @@ public function testSecondPassWithRelatedCached()
130136

131137
$identifiersExtractor = new CachedIdentifiersExtractor($cacheItemPool->reveal(), $decoration->reveal(), null);
132138

133-
$this->assertEquals(['id' => 1, 'relatedDummy' => 1], $identifiersExtractor->getIdentifiersFromItem($dummy));
139+
$expectedResult = ['id' => 1, 'relatedDummy' => 1];
140+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy));
141+
$this->assertEquals($expectedResult, $identifiersExtractor->getIdentifiersFromItem($dummy), 'Trigger the local cache');
134142
}
135143
}

0 commit comments

Comments
 (0)