Skip to content

Commit

Permalink
minor #29777 [VarDumper] Improve performance of AbstractCloner (javie…
Browse files Browse the repository at this point in the history
…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.

![varcloner-addcasters](https://user-images.githubusercontent.com/73419/50694461-40a1bd80-103a-11e9-83c0-a28b8f8f161e.png)

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
  • Loading branch information
nicolas-grekas committed Jan 25, 2019
2 parents a25daa8 + cff23e5 commit cd4b031
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php
Expand Up @@ -69,7 +69,7 @@ abstract class AbstractCloner implements ClonerInterface
'DOMProcessingInstruction' => ['Symfony\Component\VarDumper\Caster\DOMCaster', 'castProcessingInstruction'],
'DOMXPath' => ['Symfony\Component\VarDumper\Caster\DOMCaster', 'castXPath'],

'XmlReader' => ['Symfony\Component\VarDumper\Caster\XmlReaderCaster', 'castXmlReader'],
'XMLReader' => ['Symfony\Component\VarDumper\Caster\XmlReaderCaster', 'castXmlReader'],

'ErrorException' => ['Symfony\Component\VarDumper\Caster\ExceptionCaster', 'castErrorException'],
'Exception' => ['Symfony\Component\VarDumper\Caster\ExceptionCaster', 'castException'],
Expand Down Expand Up @@ -177,7 +177,7 @@ public function __construct(array $casters = null)
public function addCasters(array $casters)
{
foreach ($casters as $type => $callback) {
$closure = &$this->casters['' === $type || ':' === $type[0] ? $type : strtolower($type)][];
$closure = &$this->casters[$type][];
$closure = $callback instanceof \Closure ? $callback : static function (...$args) use ($callback, &$closure) {
return ($closure = \Closure::fromCallable($callback))(...$args);
};
Expand Down Expand Up @@ -282,15 +282,15 @@ protected function castObject(Stub $stub, $isNested)
list($i, $parents, $hasDebugInfo) = $this->classInfo[$class];
} else {
$i = 2;
$parents = [strtolower($class)];
$parents = [$class];
$hasDebugInfo = method_exists($class, '__debugInfo');

foreach (class_parents($class) as $p) {
$parents[] = strtolower($p);
$parents[] = $p;
++$i;
}
foreach (class_implements($class) as $p) {
$parents[] = strtolower($p);
$parents[] = $p;
++$i;
}
$parents[] = '*';
Expand Down

0 comments on commit cd4b031

Please sign in to comment.