Skip to content

Commit

Permalink
minor #20675 [VarDumper][HttpKernel] Enhance perf of ExceptionCaster …
Browse files Browse the repository at this point in the history
…& DataCollector (nicolas-grekas)

This PR was merged into the 3.2 branch.

Discussion
----------

[VarDumper][HttpKernel] Enhance perf of ExceptionCaster & DataCollector

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

In **dev** on 3.2 , serializing collectors' data is slow because VarDumper is called so many times.
Here is a PR to make it a bit faster, with its Blackfire profile:
https://blackfire.io/profiles/compare/6f0fdc7a-9157-4dad-bee4-4c98a96184b2/graph

Note that it is possible to make things fast again by replacing these multiple calls by a single one juste before serializing the data. Yet, it's not trivial and VarDumper misses a few features to allow dealing with an unserialized Data object and make it look like a real data structure to other code. I'll look at it for 3.3.

Commits
-------

be2b7df [VarDumper][HttpKernel] Enhance perf of ExceptionCaster & DataCollector
  • Loading branch information
fabpot committed Nov 29, 2016
2 parents cb03103 + be2b7df commit ee4ae55
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 53 deletions.
Expand Up @@ -42,6 +42,8 @@ abstract class DataCollector implements DataCollectorInterface, \Serializable
*/
private $cloner;

private static $stubsCache = array();

public function serialize()
{
return serialize($this->data);
Expand Down Expand Up @@ -124,14 +126,17 @@ private function decorateVar($var)
return $var;
}
if (is_string($var)) {
if (isset(self::$stubsCache[$var])) {
return self::$stubsCache[$var];
}
if (false !== strpos($var, '\\')) {
$c = (false !== $i = strpos($var, '::')) ? substr($var, 0, $i) : $var;
if (class_exists($c, false) || interface_exists($c, false) || trait_exists($c, false)) {
return new ClassStub($var);
return self::$stubsCache[$var] = new ClassStub($var);
}
}
if (false !== strpos($var, DIRECTORY_SEPARATOR) && false === strpos($var, '://') && false === strpos($var, "\0") && is_file($var)) {
return new LinkStub($var);
return self::$stubsCache[$var] = new LinkStub($var);
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/VarDumper/Caster/Caster.php
Expand Up @@ -57,15 +57,20 @@ public static function castObject($obj, \ReflectionClass $reflector)
}

if ($a) {
$combine = false;
$p = array_keys($a);
foreach ($p as $i => $k) {
if (isset($k[0]) && "\0" !== $k[0] && !$reflector->hasProperty($k)) {
$combine = true;
$p[$i] = self::PREFIX_DYNAMIC.$k;
} elseif (isset($k[16]) && "\0" === $k[16] && 0 === strpos($k, "\0class@anonymous\0")) {
$combine = true;
$p[$i] = "\0".$reflector->getParentClass().'@anonymous'.strrchr($k, "\0");
}
}
$a = array_combine($p, $a);
if ($combine) {
$a = array_combine($p, $a);
}
}

return $a;
Expand Down
86 changes: 49 additions & 37 deletions src/Symfony/Component/VarDumper/Caster/ExceptionCaster.php
Expand Up @@ -41,6 +41,8 @@ class ExceptionCaster
E_STRICT => 'E_STRICT',
);

private static $framesCache = array();

public static function castError(\Error $e, array $a, Stub $stub, $isNested, $filter = 0)
{
return self::filterExceptionArray($stub->class, $a, "\0Error\0", $filter);
Expand Down Expand Up @@ -142,43 +144,52 @@ public static function castFrameStub(FrameStub $frame, array $a, Stub $stub, $is
$prefix = Caster::PREFIX_VIRTUAL;

if (isset($f['file'], $f['line'])) {
if (preg_match('/\((\d+)\)(?:\([\da-f]{32}\))? : (?:eval\(\)\'d code|runtime-created function)$/', $f['file'], $match)) {
$f['file'] = substr($f['file'], 0, -strlen($match[0]));
$f['line'] = (int) $match[1];
}
$caller = isset($f['function']) ? sprintf('in %s() on line %d', (isset($f['class']) ? $f['class'].$f['type'] : '').$f['function'], $f['line']) : null;
$src = $f['line'];
$srcKey = $f['file'];
$ellipsis = explode(DIRECTORY_SEPARATOR, $srcKey);
$ellipsis = 3 < count($ellipsis) ? 2 + strlen(implode(array_slice($ellipsis, -2))) : 0;

if (file_exists($f['file']) && 0 <= self::$srcContext) {
if (!empty($f['class']) && is_subclass_of($f['class'], 'Twig_Template') && method_exists($f['class'], 'getDebugInfo')) {
$template = isset($f['object']) ? $f['object'] : unserialize(sprintf('O:%d:"%s":0:{}', strlen($f['class']), $f['class']));

$ellipsis = 0;
$templateSrc = method_exists($template, 'getSourceContext') ? $template->getSourceContext()->getCode() : (method_exists($template, 'getSource') ? $template->getSource() : '');
$templateInfo = $template->getDebugInfo();
if (isset($templateInfo[$f['line']])) {
$templatePath = method_exists($template, 'getSourceContext') ? $template->getSourceContext()->getPath() : null;

if ($templateSrc) {
$templateSrc = explode("\n", $templateSrc);
$src = self::extractSource($templateSrc, $templateInfo[$f['line']], self::$srcContext, $caller, 'twig', $templatePath);
$srcKey = ($templatePath ?: $template->getTemplateName()).':'.$templateInfo[$f['line']];
$cacheKey = $f;
unset($cacheKey['object'], $cacheKey['args']);
$cacheKey[] = self::$srcContext;
$cacheKey = implode('-', $cacheKey);

if (isset(self::$framesCache[$cacheKey])) {
$a[$prefix.'src'] = self::$framesCache[$cacheKey];
} else {
if (preg_match('/\((\d+)\)(?:\([\da-f]{32}\))? : (?:eval\(\)\'d code|runtime-created function)$/', $f['file'], $match)) {
$f['file'] = substr($f['file'], 0, -strlen($match[0]));
$f['line'] = (int) $match[1];
}
$caller = isset($f['function']) ? sprintf('in %s() on line %d', (isset($f['class']) ? $f['class'].$f['type'] : '').$f['function'], $f['line']) : null;
$src = $f['line'];
$srcKey = $f['file'];
$ellipsis = explode(DIRECTORY_SEPARATOR, $srcKey);
$ellipsis = 3 < count($ellipsis) ? 2 + strlen(implode(array_slice($ellipsis, -2))) : 0;

if (file_exists($f['file']) && 0 <= self::$srcContext) {
if (!empty($f['class']) && is_subclass_of($f['class'], 'Twig_Template') && method_exists($f['class'], 'getDebugInfo')) {
$template = isset($f['object']) ? $f['object'] : unserialize(sprintf('O:%d:"%s":0:{}', strlen($f['class']), $f['class']));

$ellipsis = 0;
$templateSrc = method_exists($template, 'getSourceContext') ? $template->getSourceContext()->getCode() : (method_exists($template, 'getSource') ? $template->getSource() : '');
$templateInfo = $template->getDebugInfo();
if (isset($templateInfo[$f['line']])) {
if (!method_exists($template, 'getSourceContext') || !file_exists($templatePath = $template->getSourceContext()->getPath())) {
$templatePath = null;
}
if ($templateSrc) {
$src = self::extractSource($templateSrc, $templateInfo[$f['line']], self::$srcContext, $caller, 'twig', $templatePath);
$srcKey = ($templatePath ?: $template->getTemplateName()).':'.$templateInfo[$f['line']];
}
}
}
}
if ($srcKey == $f['file']) {
$src = self::extractSource(explode("\n", file_get_contents($f['file'])), $f['line'], self::$srcContext, $caller, 'php', $f['file']);
$srcKey .= ':'.$f['line'];
if ($ellipsis) {
$ellipsis += 1 + strlen($f['line']);
if ($srcKey == $f['file']) {
$src = self::extractSource(file_get_contents($f['file']), $f['line'], self::$srcContext, $caller, 'php', $f['file']);
$srcKey .= ':'.$f['line'];
if ($ellipsis) {
$ellipsis += 1 + strlen($f['line']);
}
}
}
$srcAttr = $ellipsis ? 'ellipsis='.$ellipsis : '';
self::$framesCache[$cacheKey] = $a[$prefix.'src'] = new EnumStub(array("\0~$srcAttr\0$srcKey" => $src));
}
$srcAttr = $ellipsis ? 'ellipsis='.$ellipsis : '';
$a[$prefix.'src'] = new EnumStub(array("\0~$srcAttr\0$srcKey" => $src));
}

unset($a[$prefix.'args'], $a[$prefix.'line'], $a[$prefix.'file']);
Expand Down Expand Up @@ -232,14 +243,16 @@ private static function traceUnshift(&$trace, $class, $file, $line)
));
}

private static function extractSource(array $srcArray, $line, $srcContext, $title, $lang, $file = null)
private static function extractSource($srcLines, $line, $srcContext, $title, $lang, $file = null)
{
$srcLines = explode("\n", $srcLines);
$src = array();

for ($i = $line - 1 - $srcContext; $i <= $line - 1 + $srcContext; ++$i) {
$src[] = (isset($srcArray[$i]) ? $srcArray[$i] : '')."\n";
$src[] = (isset($srcLines[$i]) ? $srcLines[$i] : '')."\n";
}

$srcLines = array();
$ltrim = 0;
do {
$pad = null;
Expand All @@ -257,7 +270,6 @@ private static function extractSource(array $srcArray, $line, $srcContext, $titl
} while (0 > $i && null !== $pad);

--$ltrim;
$srcArray = array();

foreach ($src as $i => $c) {
if ($ltrim) {
Expand All @@ -274,9 +286,9 @@ private static function extractSource(array $srcArray, $line, $srcContext, $titl
}
}
$c->attr['lang'] = $lang;
$srcArray[sprintf("\0~%d\0", $i + $line - $srcContext)] = $c;
$srcLines[sprintf("\0~%d\0", $i + $line - $srcContext)] = $c;
}

return new EnumStub($srcArray);
return new EnumStub($srcLines);
}
}
5 changes: 3 additions & 2 deletions src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php
Expand Up @@ -252,14 +252,15 @@ protected function castObject(Stub $stub, $isNested)
new \ReflectionClass($class),
array_reverse(array($class => $class) + class_parents($class) + class_implements($class) + array('*' => '*')),
);
$classInfo[1] = array_map('strtolower', $classInfo[1]);

$this->classInfo[$class] = $classInfo;
}

$a = $this->callCaster('Symfony\Component\VarDumper\Caster\Caster::castObject', $obj, $classInfo[0], null, $isNested);
$a = Caster::castObject($obj, $classInfo[0]);

foreach ($classInfo[1] as $p) {
if (!empty($this->casters[$p = strtolower($p)])) {
if (!empty($this->casters[$p])) {
foreach ($this->casters[$p] as $p) {
$a = $this->callCaster($p, $obj, $a, $stub, $isNested);
}
Expand Down
Expand Up @@ -179,14 +179,14 @@ public function testFrameWithTwig()
$f = array(
new FrameStub(array(
'file' => dirname(__DIR__).'/Fixtures/Twig.php',
'line' => 21,
'line' => 20,
'class' => '__TwigTemplate_VarDumperFixture_u75a09',
)),
new FrameStub(array(
'file' => dirname(__DIR__).'/Fixtures/Twig.php',
'line' => 21,
'class' => '__TwigTemplate_VarDumperFixture_u75a09',
'object' => new \__TwigTemplate_VarDumperFixture_u75a09(null, false),
'object' => new \__TwigTemplate_VarDumperFixture_u75a09(null, __FILE__),
)),
);

Expand All @@ -195,10 +195,10 @@ public function testFrameWithTwig()
0 => {
class: "__TwigTemplate_VarDumperFixture_u75a09"
src: {
bar.twig:2: {
%sTwig.php:1: {
:
: foo bar
: twig source
:
}
}
}
Expand All @@ -208,7 +208,7 @@ class: "__TwigTemplate_VarDumperFixture_u75a09"
%A
}
src: {
foo.twig:2: {
%sExceptionCasterTest.php:2: {
: foo bar
: twig source
:
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/VarDumper/Tests/CliDumperTest.php
Expand Up @@ -271,7 +271,7 @@ public function testThrowingCaster()
⚠: Symfony\Component\VarDumper\Exception\ThrowingCasterException {{$r}
#message: "Unexpected Exception thrown from a caster: Foobar"
-trace: {
bar.twig:%d: {
%sTwig.php:2: {
: foo bar
: twig source
:
Expand Down
10 changes: 5 additions & 5 deletions src/Symfony/Component/VarDumper/Tests/Fixtures/Twig.php
Expand Up @@ -3,16 +3,16 @@
/* foo.twig */
class __TwigTemplate_VarDumperFixture_u75a09 extends Twig_Template
{
private $filename;
private $path;

public function __construct(Twig_Environment $env = null, $filename = null)
public function __construct(Twig_Environment $env = null, $path = null)
{
if (null !== $env) {
parent::__construct($env);
}
$this->parent = false;
$this->blocks = array();
$this->filename = $filename;
$this->path = $path;
}

protected function doDisplay(array $context, array $blocks = array())
Expand All @@ -28,11 +28,11 @@ public function getTemplateName()

public function getDebugInfo()
{
return array(21 => 2);
return array(20 => 1, 21 => 2);
}

public function getSourceContext()
{
return new Twig_Source(" foo bar\n twig source\n\n", 'foo.twig', false === $this->filename ? null : ($this->filename ?: 'bar.twig'));
return new Twig_Source(" foo bar\n twig source\n\n", 'foo.twig', $this->path ?: __FILE__);
}
}

0 comments on commit ee4ae55

Please sign in to comment.