Speed up setup:di:compile by removing repeated di.xml parsing - #41243
Speed up setup:di:compile by removing repeated di.xml parsing#41243jakwinkler wants to merge 1 commit into
Conversation
|
Hi @jakwinkler. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
Claude analysis — reviewed while cherry-picking this into Mage-OS Verified as correct:
Potential issues:
|
|
@rhoerr working on updates based on your review :-) |
|
@magento run all tests |
Reading a scope re-parses and DOM-merges every di.xml that contributes to it - 254 files and
about 12k nodes for 'global' on a stock install, roughly 0.6s. During one compile that read
happens five times for 'global' alone, from four independent call sites: the object manager
bootstrap, the interception configuration builder, the interception cache and the area
configuration reader. None of them share a result.
ObjectManager\Config\Reader\Dom now memoizes its parsed result per scope, normalising the scope
exactly as Config\Reader\Filesystem::read() does so that read() and read($defaultScope) share an
entry.
The cache is per instance on purpose. A result depends on the reader's file resolver, merge
rules, schema and validation state, so two differently configured readers must never see each
other's results, and a cached unvalidated read must never satisfy a reader that asked for
validation.
app/etc/di.xml wires this reader into the configuration loader, the interception config and the
plugin list, all of which outlive a single scope read, so the parsed scopes would otherwise be
held for the lifetime of the process. The reader implements ResetAfterRequestInterface to drop
them, for application server mode and other long-running processes.
Measured on a clean install, 337 modules, cold generated/, best of three:
before 13.63s
after 8.89s
Generated output is byte-identical: every metadata file and every interceptor, verified by
checksum.
Co-Authored-By: Claude <noreply@anthropic.com>
cb6baf5 to
799ae2d
Compare
|
@magento run all tests |
|
if only one could run B2B and EE tests ... ;-) |
2 participants
Description (*)
Reading a di.xml scope re-parses and DOM-merges every file that contributes to it — 254 files and about 12k nodes for
globalon a stock install, roughly 0.6s. During onesetup:di:compilethat read happens five times forglobalalone, from four independent call sites that never share a result: the object manager bootstrap, the interception configuration builder, the interception cache, and the area configuration reader.ObjectManager\Config\Reader\Domnow memoizes its parsed result per scope, normalising the scope exactly asConfig\Reader\Filesystem::read()does soread()andread($defaultScope)share an entry.The cache is per instance on purpose. A result depends on the reader's file resolver, merge rules, schema and validation state, so two differently configured readers must never see each other's results, and a cached unvalidated read must never satisfy a reader that asked for validation.
app/etc/di.xmlwires this reader into the configuration loader, the interception config and the plugin list, all of which outlive a single scope read — so the parsed scopes would otherwise be held for the lifetime of the process. The reader implementsResetAfterRequestInterfaceto drop them, for application server mode and other long-running processes.Measurements
Clean install, 337 modules, cold
generated/, best of three, idle machine:Generated output is byte-identical — every metadata file and every interceptor, verified by checksum.
Related Pull Requests
The parallel-compilation half of the original change has been split into a follow-up so this one can be reviewed on its own. It is on
qoliber/di-compile-paralleland adds worker processes for area configuration and interceptor generation (8.89s → 6.95s), which raises questions this PR does not — inherited connections acrossfork(), worker budgets under cgroup quotas, and failure reporting from a child. Those belong in their own review.Fixed Issues (if relevant)
None linked; found by profiling.
Manual testing scenarios (*)
rm -rf generated/code generated/metadata && time bin/magento setup:di:compilefind generated -name '*.php' | sort | xargs md5sum > /tmp/before.txt/tmp/after.txt.diff /tmp/before.txt /tmp/after.txt— expect no differences, and a faster compile.bin/magento cache:flush, then load a storefront page and the admin to confirm the compiled config is sound.Questions or comments
_resetState()caps that for long-running processes rather than removing the trade.DiCompileCommand::configureObjectManager()injectsexcludePatterns— containing the absolute install path — intoClassesScanner, and that argument is serialized into all seven compiled area configs. It makes the compiled output non-reproducible across build paths. Out of scope here, but happy to raise it separately.Thanks to the Mage-OS reviewers on mage-os/mageos-magento2#338, whose review of the original combined branch prompted this split.
Contribution checklist (*)