Skip to content

Commit 5fffd63

Browse files
author
Volodymyr Klymenko
authored
Merge pull request #952 from magento-east/MAGETWO-66400
[EAST] [Performance] Anomaly increase in redis traffic
2 parents 50356a8 + cabc501 commit 5fffd63

File tree

3 files changed

+180
-58
lines changed

3 files changed

+180
-58
lines changed

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

Lines changed: 122 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515

1616
/**
1717
* Class process source, cache them and retrieve value by path
18-
*
19-
* @package Magento\Config\App\Config\Type
2018
*/
2119
class System implements ConfigTypeInterface
2220
{
@@ -59,6 +57,16 @@ class System implements ConfigTypeInterface
5957
*/
6058
private $fallback;
6159

60+
/**
61+
* Key name for flag which displays whether configuration is cached or not.
62+
*
63+
* Once configuration is cached additional flag pushed to cache storage
64+
* to be able check cache existence without data load.
65+
*
66+
* @var string
67+
*/
68+
private $cacheExistenceKey = self::CONFIG_TYPE . '_CACHE_EXISTS';
69+
6270
/**
6371
* System constructor.
6472
* @param ConfigSourceInterface $source
@@ -92,20 +100,120 @@ public function get($path = '')
92100
if ($path === null) {
93101
$path = '';
94102
}
95-
if (!$this->data) {
96-
$data = $this->cache->load(self::CONFIG_TYPE);
97-
if (!$data) {
98-
$data = $this->preProcessor->process($this->source->get());
99-
$this->data = new DataObject($data);
100-
$data = $this->fallback->process($data);
101-
$this->data = new DataObject($data);
102-
$data = $this->postProcessor->process($data);
103-
$this->data = new DataObject($data);
104-
$this->cache->save(serialize($this->data), self::CONFIG_TYPE, [self::CACHE_TAG]);
105-
} else {
106-
$this->data = unserialize($data);
103+
104+
if ($this->isConfigRead($path)) {
105+
return $this->data->getData($path);
106+
}
107+
108+
if (!empty($path) && $this->isCacheExists()) {
109+
return $this->readFromCache($path);
110+
}
111+
112+
$config = $this->loadConfig();
113+
$this->cacheConfig($config);
114+
$this->data = new DataObject($config);
115+
return $this->data->getData($path);
116+
}
117+
118+
/**
119+
* Check whether configuration is cached
120+
* @return bool
121+
*/
122+
private function isCacheExists()
123+
{
124+
return $this->cache->load($this->cacheExistenceKey) !== false;
125+
}
126+
127+
/**
128+
* Explode path by '/'(forward slash) separator
129+
*
130+
* @param string $path
131+
* @return array
132+
*/
133+
private function getPathParts($path)
134+
{
135+
$pathParts = [];
136+
if (strpos($path, '/') !== false) {
137+
$pathParts = explode('/', $path);
138+
}
139+
return $pathParts;
140+
}
141+
142+
/**
143+
* Check whether requested configuration data is read to memory
144+
*
145+
* @param string $path
146+
* @return bool
147+
*/
148+
private function isConfigRead($path)
149+
{
150+
$pathParts = $this->getPathParts($path);
151+
return !empty($pathParts) && isset($this->data[$pathParts[0]][$pathParts[1]]);
152+
}
153+
154+
/**
155+
* Load configuration from all the sources
156+
*
157+
* @return array
158+
*/
159+
private function loadConfig()
160+
{
161+
$data = $this->preProcessor->process($this->source->get());
162+
$this->data = new DataObject($data);
163+
$data = $this->fallback->process($data);
164+
$this->data = new DataObject($data);
165+
166+
return $this->postProcessor->process($data);
167+
}
168+
169+
/**
170+
*
171+
* Load configuration and caching it by parts.
172+
*
173+
* To be cached configuration is loaded first.
174+
* Then it is cached by parts to minimize memory usage on load.
175+
* Additional flag cached as well to give possibility check cache existence without data load.
176+
*
177+
* @param array $data
178+
* @return void
179+
*/
180+
private function cacheConfig($data)
181+
{
182+
foreach ($data as $scope => $scopeData) {
183+
foreach ($scopeData as $key => $config) {
184+
$this->cache->save(
185+
serialize($config),
186+
self::CONFIG_TYPE . '_' . $scope . $key,
187+
[self::CACHE_TAG]
188+
);
107189
}
108190
}
191+
$this->cache->save('1', $this->cacheExistenceKey, [self::CACHE_TAG]);
192+
}
193+
194+
/**
195+
* Read cached configuration
196+
*
197+
* @param string $path
198+
* @return mixed
199+
*/
200+
private function readFromCache($path)
201+
{
202+
if ($this->data === null) {
203+
$this->data = new DataObject();
204+
}
205+
206+
$result = null;
207+
$pathParts = $this->getPathParts($path);
208+
if (!empty($pathParts)) {
209+
$result = $this->cache->load(self::CONFIG_TYPE . '_' . $pathParts[0] . $pathParts[1]);
210+
}
211+
212+
if ($result !== false && $result !== null) {
213+
$readData = $this->data->getData();
214+
$readData[$pathParts[0]][$pathParts[1]] = unserialize($result);
215+
$this->data = new DataObject($readData);
216+
}
109217

110218
return $this->data->getData($path);
111219
}

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

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,25 @@ public function setUp()
7272
);
7373
}
7474

75-
/**
76-
* @param bool $isCached
77-
* @dataProvider getDataProvider
78-
*/
79-
public function testGet($isCached)
75+
public function testGetCached()
76+
{
77+
$path = 'default/dev/unsecure/url';
78+
$url = 'http://magento.test/';
79+
$data = [
80+
'unsecure' => [
81+
'url' => $url
82+
]
83+
];
84+
85+
$this->cache->expects($this->any())
86+
->method('load')
87+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
88+
->willReturnOnConsecutiveCalls('1', serialize($data));
89+
90+
$this->assertEquals($url, $this->configType->get($path));
91+
}
92+
93+
public function testGetNotCached()
8094
{
8195
$path = 'default/dev/unsecure/url';
8296
$url = 'http://magento.test/';
@@ -89,47 +103,47 @@ public function testGet($isCached)
89103
]
90104
]
91105
];
92-
$this->cache->expects($this->once())
106+
$dataToCache = [
107+
'unsecure' => [
108+
'url' => $url
109+
]
110+
];
111+
$this->cache->expects($this->any())
93112
->method('load')
94-
->with(System::CONFIG_TYPE)
95-
->willReturn($isCached ? serialize(new DataObject($data)) : null);
96-
97-
if (!$isCached) {
98-
$this->source->expects($this->once())
99-
->method('get')
100-
->willReturn($data);
101-
$this->fallback->expects($this->once())
102-
->method('process')
103-
->with($data)
104-
->willReturnArgument(0);
105-
$this->preProcessor->expects($this->once())
106-
->method('process')
107-
->with($data)
108-
->willReturnArgument(0);
109-
$this->postProcessor->expects($this->once())
110-
->method('process')
111-
->with($data)
112-
->willReturnArgument(0);
113-
$this->cache->expects($this->once())
114-
->method('save')
115-
->with(
116-
serialize(new DataObject($data)),
117-
System::CONFIG_TYPE,
113+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
114+
->willReturnOnConsecutiveCalls(false, false);
115+
116+
$this->source->expects($this->once())
117+
->method('get')
118+
->willReturn($data);
119+
$this->fallback->expects($this->once())
120+
->method('process')
121+
->with($data)
122+
->willReturnArgument(0);
123+
$this->preProcessor->expects($this->once())
124+
->method('process')
125+
->with($data)
126+
->willReturnArgument(0);
127+
$this->postProcessor->expects($this->once())
128+
->method('process')
129+
->with($data)
130+
->willReturnArgument(0);
131+
132+
$this->cache->expects($this->any())
133+
->method('save')
134+
->withConsecutive(
135+
[
136+
serialize($dataToCache),
137+
'system_defaultdev',
138+
[System::CACHE_TAG]
139+
],
140+
[
141+
'1',
142+
'system_CACHE_EXISTS',
118143
[System::CACHE_TAG]
119-
);
120-
}
144+
]
145+
);
121146

122147
$this->assertEquals($url, $this->configType->get($path));
123148
}
124-
125-
/**
126-
* @return array
127-
*/
128-
public function getDataProvider()
129-
{
130-
return [
131-
[true],
132-
[false]
133-
];
134-
}
135149
}

‎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)