Skip to content

Commit 2a807f2

Browse files
Optimize config data traversal in Store/Config/Placeholder
Optimizations: - get rid of a lot of string concatenation and then explode() - replace recursion with iterative algorithm to avoid data copying via parameters - utilize builtin array iterator functionality to avoid creation of additional variables - read/set data directly to current $data item, skip loops in _getData() and _setData() My benchmarks for method process() on empty Magento cache have shown reduction in total time spent from 0.291 sec to 0.077 sec (-73%)
1 parent d271281 commit 2a807f2

File tree

2 files changed

+62
-13
lines changed

2 files changed

+62
-13
lines changed

app/code/Magento/Store/Model/Config/Placeholder.php

Lines changed: 45 additions & 12 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\Store\Model\Config;
78

89
/**
@@ -32,8 +33,8 @@ class Placeholder
3233
*/
3334
public function __construct(\Magento\Framework\App\RequestInterface $request, $urlPaths, $urlPlaceholder)
3435
{
35-
$this->request = $request;
36-
$this->urlPaths = $urlPaths;
36+
$this->request = $request;
37+
$this->urlPaths = $urlPaths;
3738
$this->urlPlaceholder = $urlPlaceholder;
3839
}
3940

@@ -45,15 +46,46 @@ public function __construct(\Magento\Framework\App\RequestInterface $request, $u
4546
*/
4647
public function process(array $data = [])
4748
{
48-
foreach (array_keys($data) as $key) {
49-
$this->_processData($data, $key);
49+
// check provided arguments
50+
if (empty($data)) {
51+
return [];
52+
}
53+
54+
// initialize $pointer, $parents and $level variable
55+
reset($data);
56+
$pointer = &$data;
57+
$parents = [];
58+
$level = 0;
59+
60+
while ($level >= 0) {
61+
$current = &$pointer[key($pointer)];
62+
if (is_array($current)) {
63+
reset($current);
64+
$parents[$level] = &$pointer;
65+
$pointer = &$current;
66+
$level++;
67+
} else {
68+
$current = $this->_processPlaceholders($current, $data);
69+
70+
// move pointer of last queue layer to next element
71+
// or remove layer if all path elements were processed
72+
while ($level >= 0 && next($pointer) === false) {
73+
$level--;
74+
// removal of last element of $parents is skipped here for better performance
75+
// on next iteration that element will be overridden
76+
$pointer = &$parents[$level];
77+
}
78+
}
5079
}
80+
5181
return $data;
5282
}
5383

5484
/**
5585
* Process array data recursively
5686
*
87+
* @deprecated This method isn't used in process() implementation anymore
88+
*
5789
* @param array &$data
5890
* @param string $path
5991
* @return void
@@ -90,7 +122,7 @@ protected function _processPlaceholders($value, $data)
90122

91123
if ($url) {
92124
$value = str_replace('{{' . $placeholder . '}}', $url, $value);
93-
} elseif (strpos($value, (string) $this->urlPlaceholder) !== false) {
125+
} elseif (strpos($value, (string)$this->urlPlaceholder) !== false) {
94126
$distroBaseUrl = $this->request->getDistroBaseUrl();
95127

96128
$value = str_replace($this->urlPlaceholder, $distroBaseUrl, $value);
@@ -113,10 +145,9 @@ protected function _getPlaceholder($value)
113145
{
114146
if (is_string($value) && preg_match('/{{(.*)}}.*/', $value, $matches)) {
115147
$placeholder = $matches[1];
116-
if ($placeholder == 'unsecure_base_url' || $placeholder == 'secure_base_url' || strpos(
117-
$value,
118-
(string) $this->urlPlaceholder
119-
) !== false
148+
if ($placeholder == 'unsecure_base_url' ||
149+
$placeholder == 'secure_base_url' ||
150+
strpos($value, (string)$this->urlPlaceholder) !== false
120151
) {
121152
return $placeholder;
122153
}
@@ -147,20 +178,22 @@ protected function _getValue($path, array $data)
147178
/**
148179
* Set array value by path
149180
*
181+
* @deprecated This method isn't used in process() implementation anymore
182+
*
150183
* @param array &$container
151184
* @param string $path
152185
* @param string $value
153186
* @return void
154187
*/
155188
protected function _setValue(array &$container, $path, $value)
156189
{
157-
$segments = explode('/', $path);
158-
$currentPointer = & $container;
190+
$segments = explode('/', $path);
191+
$currentPointer = &$container;
159192
foreach ($segments as $segment) {
160193
if (!isset($currentPointer[$segment])) {
161194
$currentPointer[$segment] = [];
162195
}
163-
$currentPointer = & $currentPointer[$segment];
196+
$currentPointer = &$currentPointer[$segment];
164197
}
165198
$currentPointer = $value;
166199
}

app/code/Magento/Store/Test/Unit/Model/Config/PlaceholderTest.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ protected function setUp()
2323
{
2424
$this->_requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class);
2525
$this->_requestMock->expects(
26-
$this->once()
26+
$this->any()
2727
)->method(
2828
'getDistroBaseUrl'
2929
)->will(
@@ -54,11 +54,27 @@ public function testProcess()
5454
],
5555
'path' => 'value',
5656
'some_url' => '{{base_url}}some',
57+
'level1' => [
58+
'level2' => [
59+
'level3' => [
60+
// test that all levels are processed (i.e. implementation is not hardcoded to 3 levels)
61+
'level4' => '{{secure_base_url}}level4'
62+
]
63+
]
64+
]
5765
];
5866
$expectedResult = $data;
5967
$expectedResult['web']['unsecure']['base_link_url'] = 'http://localhost/website/de';
6068
$expectedResult['web']['secure']['base_link_url'] = 'https://localhost/website/de';
69+
$expectedResult['level1']['level2']['level3']['level4'] = 'https://localhost/level4';
6170
$expectedResult['some_url'] = 'http://localhost/some';
6271
$this->assertEquals($expectedResult, $this->_model->process($data));
6372
}
73+
74+
public function testProcessEmptyArray()
75+
{
76+
$data = [];
77+
$expectedResult = [];
78+
$this->assertEquals($expectedResult, $this->_model->process($data));
79+
}
6480
}

0 commit comments

Comments
 (0)