Store non-global compiled DI areas as their diff from global - #319
Store non-global compiled DI areas as their diff from global#319jeanmarcos-dev wants to merge 1 commit into
Conversation
|
Interesting PR, thanks for sending it. Can you link to any related issues/modules/discussions? @jakwinkler I'm curious if you have any thoughts here from your speed suite work |
marcelmtz
left a comment
There was a problem hiding this comment.
I've tested this, its working good for me. I can see the benefits mentioned for current MageOS release. I think we should be able to proceed with this for MageOS 4 unless the community wants to add anything else.
|
I chatted with Jakub about this earlier. His main comments were 'not sure why core doesn't do this already' and 'has it been submitted upstream?' (not yet). It does help cold start time a bit. I like the idea, I'm generally in favor of including this for 4.0. I want to review/try it myself, but given Marcel reviewed and tested, don't consider me a blocker. |
|
I've used to work with this one Great PR btw! My research from last year: |
Description (*)
setup:di:compilewrites a complete DI configuration file per area. On a stock Mage-OS build each of them is ~6.5 MB and the whole set adds up to 44.6 MB, even though every non-global area is nearly identical toglobal.crontab.phpturns out to have zero differing entries — 6.5 MB that are an exact copy ofglobal.php.This makes each non-global area store only the entries that differ from
global:Why this is equivalent
At runtime an area is always applied on top of
global, andObjectManager\Config\Compiled::extend()merges with a top-levelarray_replaceper section. So applying only the entries that differ fromgloballeaves the object manager in a state that is identical to applying the complete file. This is an equivalence by construction, not an approximation, as long as the diff is computed with a strict (!==) comparison on whole top-level values.Two situations would break that reasoning, and both are handled rather than assumed away:
Config\Compiledtracks which area its state currently holds. When a delta extends an area other than the one applied, the base is loaded and merged first, restoring entries the previous area had overridden. This does happen in core — seedev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/ResolverCacheAbstract.php, which switches tographqland later restores the previous area.configure()before the area is applied — e.g.mockCache()inDeployStaticContentCommand. Any such call clears the tracked area, so the next area is rebuilt from its base.Files carry an explicit
_extendsmarker, so agenerated/directory produced before this change keeps working untouched, and the interception and plugin-list files that go through the same loader are returned as they are.Behaviour change worth calling out
ConfigLoader\Compileditself is unchanged —load()still returns the contents of the compiled file — but for a non-global area that file is now a delta rather than the complete configuration. Every core consumer passes the result straight toObjectManagerInterface::configure(), which resolves it; the two callers that are not areas (Interception\Config\CacheManager,PluginList) use keys whose files carry no marker and are unaffected; and the DI compiler anddev:di:infouse the uncompiled loader. Third-party code that callsload()on an area to inspect the configuration rather than to apply it would now receive only the differences.The alternative — rebuilding the complete configuration inside the loader — was measured: it costs an extra
array_replaceover the full configuration on every request and saves nothing in process memory, so the marker is resolved at the single place that applies it.Measured impact
includewithout OPcacheextend()per requestincludeper request, warm OPcacheSetting expectations honestly: this is not a page-latency optimisation. On a web request with a warm OPcache the gain is about half a millisecond, which is noise. Constant arrays live in shared memory and cost the worker almost nothing to load — measured, a 100 KB literal array costs 0.5 KB of process memory with OPcache on versus 469.8 KB with it off.
What it does buy is resources and process start-up:
opcache.memory_consumption. With PHP's 128 MB default that is over half the pool spent on DI configuration; freeing it removes a common source of evictions, which in turn cause recompilations and real latency spikes.opcache.enable_cliis off by default, so eachbin/magentoinvocation, cron job and queue consumer currently pays ~58 ms and ~30 MB purely to load its area.How it was verified
lib/internal/Magento/Frameworkandsetup/src/Magento/Setupsuites: 7545 tests, 23595 assertions, no regressions.array_replace(global, delta)is identical to the complete configuration for all six areas, and both situations above leave the object manager in the same state as today.AdminCreateSimpleProductTest(14 assertions) andStorefrontCategoryNavigationHighlightingTest(27 assertions).cron:runandbin/magentoexercised on a compiled install.Note: the build used PHP 8.3, where the
LazyTypeschain returns early, so thelazyTypessection of the delta is covered by unit tests but not by a real PHP 8.4 build.Related Pull Requests
None.
Fixed Issues (if relevant)
Manual testing scenarios (*)
bin/magento setup:di:compileon a stock install.generated/metadata/global.phpis unchanged; every other area is now a small file starting with'_extends' => 'global'.bin/magento cron:runand start a queue consumer.generated/directory compiled before this change and boot the application with the new code: those files have no marker and must load exactly as they do today.Questions or comments
The reference patch linked in the issue (
monogo-m2-optimize-object-manager) proved the idea works in production but was not integrable as-is: it computes the diff at runtime and writes it back intogenerated/metadata/on the first request (which breaks on read-only filesystems and races between concurrent workers), it merges a recursive diff witharray_replace_recursive— which is not whatextend()does and can silently keep stale entries fromglobal— and its diff drops any override to a falsy value ('',0,false,null,[]) through anif (!empty($value))check. This implementation moves the whole computation to compile time and mirrorsextend()'s flat merge exactly.Worth a follow-up, out of scope here: the compiled plugin-list files (
global|frontend|plugin-list.phpand friends) are also duplicated per scope, but they hold a different structure and their cache key is already cumulative, so they need their own approach.