[VarDumper] Improve performance of AbstractCloner #29777
Conversation
|
Alternatively we could lower all at once, before the foreach: |
|
This breaks case insensitivity of PHP identifiers (class names). |
|
If we can't reduce the number of calls to addCaster, we could instead maintain a cache of type => lowercase(type) - there aren't many of them |
|
@nicolas-grekas you are right ... but we already broke PHP class name insensitivity in 3.3 when we made service IDs (= PHP FQCN) case sensitive. That's why I was wondering if we could do the same here to gain a bit of performance (combining #29762 and this PR it gave me an 8% improvement in the Symfony Demo app). |
|
@javiereguiluz we broke case insensitivity of service identifiers, not of PHP classes as they're out of reach: that's the behavior of the PHP engine, no ways around. |
5928b35 to
cff23e5
Compare
|
Thank you @javiereguiluz. |
…reguiluz) This PR was merged into the 4.3-dev branch. Discussion ---------- [VarDumper] Improve performance of AbstractCloner | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | - <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | - While profiling Symfony in "dev" environment (see #29762) I found that `VarCloner::addCasters()` was making thousands of `strtolower()` calls.  In this PR I propose to remove all those calls. I think it's possible to do it ... but I could be completely wrong, so please review. ----- As a side note, in the past we did the same `strtolower()` to service IDs and parameter names. We stopped doing that in Symfony 3.3 and it gave us a small performance improvement (same as we could gain here). If the `strtolower()` calls of `VarCloner::addCasters()` are made just to apply the caster even if the class name is wrongly spelled, I think we could make this change. My guess is that nothing would break for the user (unlike removing the `strtolower()` in DependencyInjection, which broke wrongly spelled services and params). Thanks! Commits ------- cff23e5 [VarDumper] Improve performance of AbstractCloner
|
As we rely on |
While profiling Symfony in "dev" environment (see #29762) I found that
VarCloner::addCasters()was making thousands ofstrtolower()calls.In this PR I propose to remove all those calls. I think it's possible to do it ... but I could be completely wrong, so please review.
As a side note, in the past we did the same
strtolower()to service IDs and parameter names. We stopped doing that in Symfony 3.3 and it gave us a small performance improvement (same as we could gain here).If the
strtolower()calls ofVarCloner::addCasters()are made just to apply the caster even if the class name is wrongly spelled, I think we could make this change. My guess is that nothing would break for the user (unlike removing thestrtolower()in DependencyInjection, which broke wrongly spelled services and params). Thanks!