[PropertyInfo] add static cache to ContextFactory #32188
Merged
Conversation
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
bcfdbf1 to
865af28
Compare
This was referenced Jun 26, 2019
|
Caching is normally done with decoration, isn't it? |
|
@bendavies maybe ;). I wanted to show that there is indeed a problem and a quickfix implementation. |
|
Should have been fixed by #31452, no? |
|
Not that I'm aware of. The cache added in the PR seems to be at property level. But for each property/method extraction you need to parse the original file. That's what i'm trying to avoid. |
|
Just seen that it was in 4.3-rc1, so nope not fixed! |
dunglas
approved these changes
Jun 26, 2019
lyrixx
approved these changes
Jun 27, 2019
nicolas-grekas
requested changes
Jun 27, 2019
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
da89214 to
8aabb78
Compare
xabbuh
reviewed
Jun 28, 2019
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php
Outdated
Show resolved
Hide resolved
ContextFactory::createFromReflector is heavy, and it's called redundanlty by Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor for each property and methods. Avoid that by parsing it only once and then use static cache
8aabb78 to
063e880
Compare
xabbuh
reviewed
Jun 30, 2019
nicolas-grekas
approved these changes
Jul 1, 2019
fabpot
approved these changes
Jul 3, 2019
|
Thank you @bastnic. |
fabpot
added a commit
that referenced
this pull request
Jul 3, 2019
This PR was merged into the 4.4 branch. Discussion ---------- [PropertyInfo] add static cache to ContextFactory | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no (performance...) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #32073, api-platform/api-platform#1159 | License | MIT | Doc PR | no The issue is very very well described here #32073, and was also discussed a few weeks ago with @dunglas here api-platform/api-platform#1159. `ContextFactory::createFromReflector` is heavy, and it's called redundanlty by `Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor` for each property and methods. Avoid that by parsing it only once and then use static cache This is a quite big performance problem.  Commits ------- 063e880 [PropertyInfo] add static cache to ContextFactory
|
ah shame this didn't go into 4.3. thanks anyway! |
fabpot
added a commit
that referenced
this pull request
May 31, 2024
… (mvhirsch) This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [PropertyInfo] Adds static cache to `PhpStanExtractor` | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no (performance) | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Issues | <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead --> | License | MIT I was able to detect a performance penalty when using dozens of traits in even more classes. The `PhpStanExtractor ` creates a `NameScope` every time, but afaik this can be cached like in `PhpDocExtractor` (see #32188). The performance impact is impressive, as it reduces the time needed for my `TestCase` by 30%. See [Blackfire profile comparison](https://blackfire.io/profiles/compare/9eb78784-fa68-4721-9c87-f2be52da2d63/graph). <!-- Replace this notice by a description of your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> Commits ------- 9697351 [PropertyInfo] Adds static cache to `PhpStanExtractor`
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.
The issue is very very well described here #32073, and was also discussed a few weeks ago with @dunglas here api-platform/api-platform#1159.
ContextFactory::createFromReflectoris heavy, and it's called redundanltyby
Symfony\Component\PropertyInfo\Extractor\PhpDocExtractorfor each propertyand methods. Avoid that by parsing it only once and then use static cache
This is a quite big performance problem.