perf(di:compile): cache ReflectionClass instantiations in Type, Config/Reader and ClassReader#40615
Open
SamJUK wants to merge 1 commit intomagento:2.4-developfrom
Open
perf(di:compile): cache ReflectionClass instantiations in Type, Config/Reader and ClassReader#40615SamJUK wants to merge 1 commit intomagento:2.4-developfrom
SamJUK wants to merge 1 commit intomagento:2.4-developfrom
Conversation
|
Hi @SamJUK. 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. |
…g/Reader, ClassReader Three hot-path locations in the DI compiler create new ReflectionClass instances on every call with no caching, causing tens of thousands of redundant reflection operations per compile run. 1. Type::isConcrete() — called on every class in every area with no cache. On a 400-module install: ~40,000 ReflectionClass instantiations per run. Fix: add $concreteCache[] keyed by class name. 2. Config/Reader::generateCachePerScope() — preference loop calls new ReflectionClass($preference) per preference per area (2,542 preferences × 8 areas = ~20,000 instantiations). Fix: add static $phpExtensionClassCache[] keyed by class name. 3. ClassReader::getConstructor() — no cache on constructor reflection despite being called repeatedly for the same class during compilation. Fix: add $constructorCache[] keyed by class name. Benchmark results (combined effect of all three caches): | Project | Area config before | Area config after | Saving | |-------------|-------------------|-------------------|--------| | sandbox | 2.8s | 1.9s | 32% | | ma-griggs | 5.1s | 3.2s | 37% | | elesi | 3.9s | 3.3s | 15% | Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1673beb to
49717d2
Compare
Contributor
Author
|
@magento run all tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Three hot-path functions in the DI compiler call
new ReflectionClass()on every invocation with no caching. On a mid-size install (390–470 modules) this adds up to roughly 60,000+ redundant instantiations per compile run.1.
Type::isConcrete()— ~40,000 uncached callsCalled for every class in
DefinitionsCollectionfor every compilation area, with no caching. With ~5,000 classes and 8 areas, the sameReflectionClassobjects are instantiated up to 8 times each.2.
Config/Reader::generateCachePerScope()— ~20,000 uncached callsThe preference resolution loop checks whether each preference is a PHP extension class via
new ReflectionClass($preference)->getExtension(). With 2,542 preferences across 8 areas that's ~20,000 instantiations, yet only ~10 classes (PDO, SplStack, etc.) actually are extension classes. 99.9% of these return null and are thrown away.3.
ClassReader::getConstructor()— repeated calls across the pipelinegetConstructor()is called from multiple pipeline stages for the same class. There is already a$parentsCacheonClassReaderbut no equivalent for constructor signatures.Fix: add a simple per-instance memoisation array to each of the three methods. All three are pure functions of their input for the duration of a compile run, so the caches are always valid and never need invalidating within a single process.
Note:
array_key_existsis used rather thanissetinClassReaderbecause classes with no constructor legitimately returnnull, whichissetwould treat as a cache miss.Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios
Timing comparison
To isolate the affected phase:
On a ~400-module project expect the Area configuration aggregation phase to drop by roughly 30–45%.
Output correctness
Benchmarks on real-world projects
Memory impact
Measured on a ~470-module install:
The increase is the memoised reflection results being retained in memory rather than discarded after each call. For a CLI compile tool that typically runs with
memory_limit=2048Mthis is well within normal bounds.Questions or comments
The three caches are independent and could be split into separate PRs if preferred. Kept together here as they follow the same pattern and affect the same compilation phase.
Contribution checklist