Preconditions and environment
- Magento version: 2.4-develop (verified at
b977e369455)
- PHP 8.5, no third-party modules involved — this is a property of the core code, not of a
particular installation
Steps to reproduce
array_merge() allocates a new array and copies both operands. Merging into an accumulator
inside a loop therefore copies the whole partial result on every iteration, so the total work
grows with the square of the number of iterations while the output grows linearly:
$result = [];
foreach ($chunks as $chunk) {
$result = array_merge($result, $chunk); // copies everything collected so far
}The equivalent single merge is array_merge(...$chunks).
To see the scale of it in the core code base:
- Check out
2.4-develop.
- Run the sniff that is supposed to catch this shape over the whole tree:
vendor/bin/phpcs --standard=Magento2 --sniffs=Magento2.Performance.ForeachArrayMerge \
--extensions=php --warning-severity=1 --ignore-annotations \
app/code/Magento lib/internal/Magento setup/src
- Count the suppressions that hide the same finding in day-to-day runs:
grep -rn "phpcs:ignore Magento2.Performance.ForeachArrayMerge" app/code lib/internal setup dev | wc -l
Expected result
Accumulating merges are collected and merged once, so the cost of building a list is linear in
the amount of data.
Actual result
- 126
array_merge calls inside loop bodies are detected by the sniff across
app/code/Magento, lib/internal/Magento and setup/src.
- 77 of the findings are hidden behind
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
annotations, so only 46 warnings actually surface in a normal run.
- An AST pass over the same tree finds 52 genuine accumulators — loops where the merge
target really does grow on every iteration. The rest are calls that copy a constant amount
of data per iteration, or targets that are rebuilt from scratch inside the loop.
Most of the 52 are bounded by small counts (bundle options, validation rules, config nodes) and
are not worth touching. Some are bounded by merchant data, and there the cost is real. Measured
on this branch, median of 5 runs, PHP 8.5:
| Location |
Loop runs once per |
before |
after |
Framework\Cache\...\FilesystemTagAdapter::getIdsMatchingAnyTags() |
cache tag |
33 271 ms @ 20 000 tags |
4 185 ms |
Framework\Cache\...\FilesystemTagAdapter::getAllIds() (via getIdsNotMatchingTags()) |
tag file |
27 823 ms @ 20 000 tags |
1 686 ms |
Catalog\Helper\Product\Flat\Indexer::getFlatColumns() / ::getFlatIndexes() |
flat attribute |
16.0 ms @ 2 000 attributes |
0.06 ms |
Analytics\Model\StoreConfigurationProvider::getReport() |
website / store view |
77.4 ms @ 1 000 store views |
0.25 ms |
The first two sit on the cache invalidation path: Symfony::clean() calls
getIdsMatchingAnyTags() for CLEANING_MODE_MATCHING_ANY_TAG and getIdsNotMatchingTags()
for CLEANING_MODE_NOT_MATCHING_TAG, which is what happens whenever cache is cleaned by tag.
Additional information
Why the existing guard did not prevent this. Magento2.Performance.ForeachArrayMerge
(magento/magento-coding-standard) warns on every array_merge token inside a for or
foreach body, whether or not anything accumulates, and its register() returns only
T_FOREACH and T_FOR, so while and do/while bodies are never inspected. Two
consequences:
- 53 of the 126 detections (42 %) cannot grow anything, so
phpcs:ignore becomes the natural
response — and once the annotation is there, a genuine finding on the same line is hidden too.
All three fixes referenced below were sitting behind such an annotation.
- Accumulators in
while loops were never reported at all, for example
Framework\View\Design\Fallback\Rule\Theme::getPatternDirs().
magento/magento-coding-standard#503 narrows the sniff to the accumulating shape and adds
while/do coverage: 126 raw detections → 74, one new true finding, no accumulating call lost.
Merging it is what keeps the remaining suppressions in this repository meaningful.
Fixes opened for the three measured cases, one per module:
Each replaces the accumulator with a collected list and a single array_merge(...), preserving
array order and key-override behaviour, and removes the suppression annotations that are no
longer needed.
Remaining accumulators outside those three are bounded by small counts on the paths I checked;
they are consistency cleanups rather than performance work, and are deliberately not part of
those pull requests.
Release note
Cache cleaning by tag, flat catalog structure building and Advanced Reporting configuration
collection no longer grow quadratically with the number of tags, attributes or store views.
Triage and priority
Preconditions and environment
b977e369455)particular installation
Steps to reproduce
array_merge()allocates a new array and copies both operands. Merging into an accumulatorinside a loop therefore copies the whole partial result on every iteration, so the total work
grows with the square of the number of iterations while the output grows linearly:
The equivalent single merge is
array_merge(...$chunks).To see the scale of it in the core code base:
2.4-develop.Expected result
Accumulating merges are collected and merged once, so the cost of building a list is linear in
the amount of data.
Actual result
array_mergecalls inside loop bodies are detected by the sniff acrossapp/code/Magento,lib/internal/Magentoandsetup/src.// phpcs:ignore Magento2.Performance.ForeachArrayMergeannotations, so only 46 warnings actually surface in a normal run.
target really does grow on every iteration. The rest are calls that copy a constant amount
of data per iteration, or targets that are rebuilt from scratch inside the loop.
Most of the 52 are bounded by small counts (bundle options, validation rules, config nodes) and
are not worth touching. Some are bounded by merchant data, and there the cost is real. Measured
on this branch, median of 5 runs, PHP 8.5:
Framework\Cache\...\FilesystemTagAdapter::getIdsMatchingAnyTags()Framework\Cache\...\FilesystemTagAdapter::getAllIds()(viagetIdsNotMatchingTags())Catalog\Helper\Product\Flat\Indexer::getFlatColumns()/::getFlatIndexes()Analytics\Model\StoreConfigurationProvider::getReport()The first two sit on the cache invalidation path:
Symfony::clean()callsgetIdsMatchingAnyTags()forCLEANING_MODE_MATCHING_ANY_TAGandgetIdsNotMatchingTags()for
CLEANING_MODE_NOT_MATCHING_TAG, which is what happens whenever cache is cleaned by tag.Additional information
Why the existing guard did not prevent this.
Magento2.Performance.ForeachArrayMerge(magento/magento-coding-standard) warns on every
array_mergetoken inside afororforeachbody, whether or not anything accumulates, and itsregister()returns onlyT_FOREACHandT_FOR, sowhileanddo/whilebodies are never inspected. Twoconsequences:
phpcs:ignorebecomes the naturalresponse — and once the annotation is there, a genuine finding on the same line is hidden too.
All three fixes referenced below were sitting behind such an annotation.
whileloops were never reported at all, for exampleFramework\View\Design\Fallback\Rule\Theme::getPatternDirs().magento/magento-coding-standard#503 narrows the sniff to the accumulating shape and adds
while/docoverage: 126 raw detections → 74, one new true finding, no accumulating call lost.Merging it is what keeps the remaining suppressions in this repository meaningful.
Fixes opened for the three measured cases, one per module:
Framework\Cachetag adaptersCatalogflat table column and index mapsAnalyticsstore configuration reportEach replaces the accumulator with a collected list and a single
array_merge(...), preservingarray order and key-override behaviour, and removes the suppression annotations that are no
longer needed.
Remaining accumulators outside those three are bounded by small counts on the paths I checked;
they are consistency cleanups rather than performance work, and are deliberately not part of
those pull requests.
Release note
Cache cleaning by tag, flat catalog structure building and Advanced Reporting configuration
collection no longer grow quadratically with the number of tags, attributes or store views.
Triage and priority