Skip to content

Build flat table column and index maps in one array_merge call (#41098) - #41096

Open
lbajsarowicz wants to merge 1 commit into
magento:2.4-developfrom
lbajsarowicz:perf/catalog-flat-indexer-array-merge
Open

Build flat table column and index maps in one array_merge call (#41098)#41096
lbajsarowicz wants to merge 1 commit into
magento:2.4-developfrom
lbajsarowicz:perf/catalog-flat-indexer-array-merge

Conversation

@lbajsarowicz

@lbajsarowicz lbajsarowicz commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixed Issues (if relevant)

  1. Relates to array_merge() accumulators in loops make cache tag cleaning, flat indexing and Analytics collection quadratic #41098 — scope analysis and measurements for the whole pattern.
    That issue covers three modules, so this pull request is one of three and does not close it on its own.

Description (*)

Catalog\Helper\Product\Flat\Indexer::getFlatColumns() and ::getFlatIndexes() build the flat
table structure by merging each attribute's contribution into the map accumulated so far:

$this->_columns = $this->getFlatColumnsDdlDefinition();
foreach ($this->getAttributes() as $attribute) {
    $columns = $attribute->...->getFlatColumns();
    if ($columns !== null) {
        $this->_columns = array_merge($this->_columns, $columns);   // copies the whole map
    }
}

array_merge() allocates a new array and copies both operands, so a loop of this shape copies
the entire partial map once per attribute. The work grows with the square of the attribute
count while the useful output grows linearly. Both loops run once per flat attribute, and the
methods are called per store while (re)building catalog_product_flat_*.

Both loops now collect the per-attribute maps and merge them once with argument unpacking:

$columnsPerAttribute = [$this->getFlatColumnsDdlDefinition()];
foreach ($this->getAttributes() as $attribute) {
    $columns = $attribute->...->getFlatColumns();
    if ($columns !== null) {
        $columnsPerAttribute[] = $columns;
    }
}
$this->_columns = array_merge(...$columnsPerAttribute);

The merge order is unchanged, so a later attribute still overrides an earlier key exactly as
before. The // phpcs:ignore Magento2.Performance.ForeachArrayMerge annotations on both lines
are no longer needed.

Measured effect

Merge cost only (attribute loading excluded, so this isolates what the change touches), PHP
8.5, median of 5 runs, one column per attribute:

flat attributes accumulating merge single merge
200 0.18 ms 0.006 ms
500 1.05 ms 0.015 ms
1 000 4.13 ms 0.032 ms
2 000 16.04 ms 0.060 ms

Modest per call in absolute terms, and it is paid per store on every flat rebuild — a
multi-store installation with a wide attribute set pays it repeatedly. The shape is also the
thing the coding standard asks contributors not to write, so the annotations disappearing is
part of the point.

Related

Magento2.Performance.ForeachArrayMerge is the sniff meant to catch this shape, but it warns
on every array_merge inside a for/foreach body — including calls that copy a constant
amount of data — and never inspects while/do bodies. 2.4-develop carries 77
phpcs:ignore Magento2.Performance.ForeachArrayMerge annotations as a result, these two among
them.

magento/magento-coding-standard#503 narrows the sniff to the accumulating shape and adds
while/do coverage (126 raw detections in 2.4-develop → 74, plus one new true finding).
Reviewing and merging it would make the remaining suppressions meaningful again — input from
the core team there is very welcome.

Manual testing scenarios (*)

  1. Enable Stores > Configuration > Catalog > Catalog > Storefront > Use Flat Catalog Product.
  2. Run bin/magento indexer:reindex catalog_product_flat and confirm the generated
    catalog_product_flat_* tables have the same columns and indexes as before the change
    (SHOW CREATE TABLE catalog_product_flat_1).
  3. Add a filterable attribute to the default attribute set, reindex again, confirm the new
    column and its index appear.
  4. vendor/bin/phpunit -c dev/tests/unit/phpunit.xml.dist app/code/Magento/Catalog/Test/Unit/Helper/
    — 44 tests green.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

getFlatColumns() and getFlatIndexes() merged every attribute's contribution
into the accumulated map on each iteration, so each iteration copied the whole
map built so far. Both loops run once per flat attribute, which makes the cost
grow with the square of the attribute count.

Collect the per-attribute maps and merge them once with argument unpacking. The
merge order is unchanged, so a later attribute still overrides an earlier key
exactly as before, and the phpcs:ignore annotations for
Magento2.Performance.ForeachArrayMerge are no longer needed.
@m2-assistant

m2-assistant Bot commented Aug 5, 2026

Copy link
Copy Markdown

Hi @lbajsarowicz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@lbajsarowicz

Copy link
Copy Markdown
Contributor Author

@magento run all tests

@lbajsarowicz lbajsarowicz changed the title Build flat table column and index maps in one array_merge call Build flat table column and index maps in one array_merge call (#41098) Aug 5, 2026
@engcom-Hotel engcom-Hotel added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Aug 11, 2026
@github-project-automation github-project-automation Bot moved this to Pending Review in Pull Requests Dashboard Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: pending review

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

2 participants