[PropertyInfo] Use a single cache item per method #30523
Closed
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
9700ee0 to
fa65dfa
Compare
|
The patch looks good to me. |
|
However, it should target master, we don't consider performance fixes as bug fixes. |
One case we found is that if a new webserver is added to a pool due to autoscaling, having memcache + APC is useful in reducing first-load times. Thanks for the review. I will open a new PR so this is available in case anyone in the future wants to patch this in. |
symfony-splitter
pushed a commit
to symfony/property-info
that referenced
this pull request
Apr 3, 2019
…ntintegral) This PR was merged into the 4.3-dev branch. Discussion ---------- [PropertyInfo] Use a single cache item per method | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29977 | License | MIT | Doc PR | none Replaces symfony/symfony#30523 with a rebase to master. This PR changes how property metadata is cached, significantly reducing the number of calls made between PHP and the backend cache. Instead of storing one cache item per method and set of arguments, a single cache item is stored per method. This matches well with real-world use, where most properties in an object will need to be inspected. Note that the absolute numbers in the above PR are best case. In production environments where memcache is on a remote server, we were seeing multiple seconds consumed by memcache calls. Commits ------- 2a4f8a11d4 [PropertyInfo] Use a single cache item per method
fabpot
added a commit
that referenced
this pull request
Apr 3, 2019
…ntintegral) This PR was merged into the 4.3-dev branch. Discussion ---------- [PropertyInfo] Use a single cache item per method | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29977 | License | MIT | Doc PR | none Replaces #30523 with a rebase to master. This PR changes how property metadata is cached, significantly reducing the number of calls made between PHP and the backend cache. Instead of storing one cache item per method and set of arguments, a single cache item is stored per method. This matches well with real-world use, where most properties in an object will need to be inspected. Note that the absolute numbers in the above PR are best case. In production environments where memcache is on a remote server, we were seeing multiple seconds consumed by memcache calls. Commits ------- 2a4f8a1 [PropertyInfo] Use a single cache item per method
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.
This PR changes how property metadata is cached, significantly reducing the number of calls made between PHP and the backend cache. Instead of storing one cache item per method and set of arguments, a single cache item is stored per method. This matches well with real-world use, where most properties in an object will need to be inspected.
It's not clear to me if performance improvements fall under "bug fix", though I will say that we would have had to rewrite our code to remove PropertyInfo without this change.
Note that the absolute numbers in the above PR are best case. In production environments where memcache is on a remote server, we were seeing multiple seconds consumed by memcache calls.