Skip to content

Commit 5cd850b

Browse files
miakushaxmav
miakusha
authored and
xmav
committed
MAGETWO-66343: [Performance] Anomaly increase in redis traffic
1 parent f1033b2 commit 5cd850b

File tree

3 files changed

+184
-68
lines changed

3 files changed

+184
-68
lines changed

‎app/code/Magento/Config/App/Config/Type/System.php

Lines changed: 118 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Config\App\Config\Type;
78

89
use Magento\Framework\App\Config\ConfigTypeInterface;
@@ -57,6 +58,16 @@ class System implements ConfigTypeInterface
5758
*/
5859
private $serializer;
5960

61+
/**
62+
* Key name for flag which displays whether configuration is cached or not.
63+
*
64+
* Once configuration is cached additional flag pushed to cache storage
65+
* to be able check cache existence without data load.
66+
*
67+
* @var string
68+
*/
69+
private $cacheExistenceKey = self::CONFIG_TYPE . '_CACHE_EXISTS';
70+
6071
/**
6172
* The type of config.
6273
*
@@ -102,25 +113,119 @@ public function get($path = '')
102113
if ($path === null) {
103114
$path = '';
104115
}
105-
if (!$this->data) {
106-
$data = $this->cache->load($this->configType);
107-
if (!$data) {
108-
$data = $this->preProcessor->process($this->source->get());
109-
$this->data = new DataObject($data);
110-
$data = $this->fallback->process($data);
111-
$this->data = new DataObject($data);
112-
//Placeholder processing need system config - so we need to save intermediate result
113-
$data = $this->postProcessor->process($data);
114-
$this->data = new DataObject($data);
116+
if ($this->isConfigRead($path)) {
117+
return $this->data->getData($path);
118+
}
119+
120+
if (!empty($path) && $this->isCacheExists()) {
121+
return $this->readFromCache($path);
122+
}
123+
124+
$config = $this->loadConfig();
125+
$this->cacheConfig($config);
126+
$this->data = new DataObject($config);
127+
return $this->data->getData($path);
128+
}
129+
130+
/**
131+
* Check whether configuration is cached
132+
* @return bool
133+
*/
134+
private function isCacheExists()
135+
{
136+
return $this->cache->load($this->cacheExistenceKey) !== false;
137+
}
138+
139+
/**
140+
* Explode path by '/'(forward slash) separator
141+
*
142+
* @param string $path
143+
* @return array
144+
*/
145+
private function getPathParts($path)
146+
{
147+
$pathParts = [];
148+
if (strpos($path, '/') !== false) {
149+
$pathParts = explode('/', $path);
150+
}
151+
return $pathParts;
152+
}
153+
154+
/**
155+
* Check whether requested configuration data is read to memory
156+
*
157+
* @param string $path
158+
* @return bool
159+
*/
160+
private function isConfigRead($path)
161+
{
162+
$pathParts = $this->getPathParts($path);
163+
return !empty($pathParts) && isset($this->data[$pathParts[0]][$pathParts[1]]);
164+
}
165+
166+
/**
167+
* Load configuration from all the sources
168+
*
169+
* @return array
170+
*/
171+
private function loadConfig()
172+
{
173+
$data = $this->preProcessor->process($this->source->get());
174+
$this->data = new DataObject($data);
175+
$data = $this->fallback->process($data);
176+
$this->data = new DataObject($data);
177+
178+
return $this->postProcessor->process($data);
179+
}
180+
181+
/**
182+
*
183+
* Load configuration and caching it by parts.
184+
*
185+
* To be cached configuration is loaded first.
186+
* Then it is cached by parts to minimize memory usage on load.
187+
* Additional flag cached as well to give possibility check cache existence without data load.
188+
*
189+
* @param array $data
190+
* @return void
191+
*/
192+
private function cacheConfig($data)
193+
{
194+
foreach ($data as $scope => $scopeData) {
195+
foreach ($scopeData as $key => $config) {
115196
$this->cache->save(
116197
$this->serializer->serialize($this->data->getData()),
117-
$this->configType,
198+
self::CONFIG_TYPE,
118199
[self::CACHE_TAG]
119200
);
120-
} else {
121-
$this->data = new DataObject($this->serializer->unserialize($data));
122201
}
123202
}
203+
$this->cache->save('1', $this->cacheExistenceKey, [self::CACHE_TAG]);
204+
}
205+
206+
/**
207+
* Read cached configuration
208+
*
209+
* @param string $path
210+
* @return mixed
211+
*/
212+
private function readFromCache($path)
213+
{
214+
if ($this->data === null) {
215+
$this->data = new DataObject();
216+
}
217+
218+
$result = null;
219+
$pathParts = $this->getPathParts($path);
220+
if (!empty($pathParts)) {
221+
$result = $this->cache->load(self::CONFIG_TYPE . '_' . $pathParts[0] . $pathParts[1]);
222+
}
223+
224+
if ($result !== false && $result !== null) {
225+
$readData = $this->data->getData();
226+
$readData[$pathParts[0]][$pathParts[1]] = $this->serializer->unserialize($result);
227+
$this->data = new DataObject($readData);
228+
}
124229

125230
return $this->data->getData($path);
126231
}

‎app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php

Lines changed: 65 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Config\Test\Unit\App\Config\Type;
78

89
use Magento\Config\App\Config\Type\System;
@@ -69,6 +70,7 @@ public function setUp()
6970
->getMockForAbstractClass();
7071
$this->serializer = $this->getMockBuilder(SerializerInterface::class)
7172
->disableOriginalConstructor()
73+
->setMethods(['serialize', 'unserialize'])
7274
->getMock();
7375
$this->configType = new System(
7476
$this->source,
@@ -80,15 +82,32 @@ public function setUp()
8082
);
8183
}
8284

83-
/**
84-
* @param bool $isCached
85-
* @dataProvider getDataProvider
86-
*/
87-
public function testGet($isCached)
85+
public function testGetCached()
86+
{
87+
$path = 'default/dev/unsecure/url';
88+
$url = 'http://magento.test/';
89+
$data = [
90+
'unsecure' => [
91+
'url' => $url
92+
]
93+
];
94+
$serializedDataString = '{"serialized-data"}';
95+
96+
$this->cache->expects($this->any())
97+
->method('load')
98+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
99+
->willReturnOnConsecutiveCalls('1', serialize($data));
100+
$this->serializer->expects($this->once())
101+
->method('unserialize')
102+
->willReturn($data);
103+
$this->assertEquals($url, $this->configType->get($path));
104+
}
105+
106+
public function testGetNotCached()
88107
{
89108
$path = 'default/dev/unsecure/url';
90109
$url = 'http://magento.test/';
91-
$unserializedData = [
110+
$data = [
92111
'default' => [
93112
'dev' => [
94113
'unsecure' => [
@@ -97,58 +116,50 @@ public function testGet($isCached)
97116
]
98117
]
99118
];
100-
$serializedDataString = '{"serialized-data"}';
101119

102-
$this->cache->expects($this->once())
120+
$dataToCache = [
121+
'unsecure' => [
122+
'url' => $url
123+
]
124+
];
125+
$this->cache->expects($this->any())
103126
->method('load')
104-
->with(System::CONFIG_TYPE)
105-
->willReturn($isCached ? $unserializedData : null);
106-
107-
if ($isCached) {
108-
$this->serializer->expects($this->once())
109-
->method('unserialize')
110-
->willReturn($unserializedData);
111-
}
112-
113-
if (!$isCached) {
114-
$this->serializer->expects($this->once())
115-
->method('serialize')
116-
->willReturn($serializedDataString);
117-
$this->source->expects($this->once())
118-
->method('get')
119-
->willReturn($unserializedData);
120-
$this->fallback->expects($this->once())
121-
->method('process')
122-
->with($unserializedData)
123-
->willReturnArgument(0);
124-
$this->preProcessor->expects($this->once())
125-
->method('process')
126-
->with($unserializedData)
127-
->willReturnArgument(0);
128-
$this->postProcessor->expects($this->once())
129-
->method('process')
130-
->with($unserializedData)
131-
->willReturnArgument(0);
132-
$this->cache->expects($this->once())
133-
->method('save')
134-
->with(
135-
$serializedDataString,
136-
System::CONFIG_TYPE,
127+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
128+
->willReturnOnConsecutiveCalls(false, false);
129+
130+
$this->serializer->expects($this->once())
131+
->method('serialize')
132+
->willReturn(serialize($dataToCache));
133+
$this->source->expects($this->once())
134+
->method('get')
135+
->willReturn($data);
136+
$this->fallback->expects($this->once())
137+
->method('process')
138+
->with($data)
139+
->willReturnArgument(0);
140+
$this->preProcessor->expects($this->once())
141+
->method('process')
142+
->with($data)
143+
->willReturnArgument(0);
144+
$this->postProcessor->expects($this->once())
145+
->method('process')
146+
->with($data)
147+
->willReturnArgument(0);
148+
$this->cache->expects($this->any())
149+
->method('save')
150+
->withConsecutive(
151+
[
152+
serialize($dataToCache),
153+
'system_defaultdev',
154+
[System::CACHE_TAG]
155+
],
156+
[
157+
'1',
158+
'system_CACHE_EXISTS',
137159
[System::CACHE_TAG]
138-
);
139-
}
160+
]
161+
);
140162

141163
$this->assertEquals($url, $this->configType->get($path));
142164
}
143-
144-
/**
145-
* @return array
146-
*/
147-
public function getDataProvider()
148-
{
149-
return [
150-
[true],
151-
[false]
152-
];
153-
}
154165
}

‎lib/internal/Magento/Framework/App/Config/ConfigTypeInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ interface ConfigTypeInterface
1616
* Retrieve configuration raw data array.
1717
*
1818
* @param string $path
19-
* @return array
19+
* @return mixed
2020
*/
2121
public function get($path = '');
2222

0 commit comments

Comments
 (0)