[HttpKernel][VarDumper] Truncate profiler data & optim perf #23465

nicolas-grekas · 2017-07-10T08:58:50Z

Q	A
Branch?	3.3
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#23415, #21547 and hopefully #23110 and #23175
License	MIT
Doc PR	-

nicolas-grekas · 2017-07-10T09:01:08Z

src/Symfony/Component/HttpKernel/DataCollector/DataCollector.php

+    protected function getCasters()
+    {
+        return array(
+            '*' => function ($v, array $a, Stub $s, $isNested) {


should be the main fix: nested objects are truncated, which prevent deep recursion into nested structures by default (eg "object" of the security decision log, requests attributes, etc.)

nicolas-grekas · 2017-07-10T09:02:12Z

src/Symfony/Component/VarDumper/Caster/Caster.php

            $i = 0;
            $prefixedKeys = array();
            foreach ($a as $k => $v) {
-                if (isset($k[0]) && "\0" !== $k[0] && !property_exists($class, $k)) {


as shown by profiles in linked issue, this is a hot code path and calling property_exists() should be prevented

nicolas-grekas · 2017-07-10T09:04:03Z

src/Symfony/Component/VarDumper/Cloner/AbstractCloner.php

@@ -210,14 +210,12 @@ public function cloneVar($var, $filter = 0)
        $this->filter = $filter;

        try {
+            gc_disable();


shown also in linked profiles: cloning involves lots of Stub objects, thus can trigger lots of GC calls, for no benefit.

Shouldnt we only re-enable based on gc_enabled()?

szymach · 2017-07-10T09:08:00Z

Hey, I have just tried this fix and it indeed improves performance a lot. Basically it runs as fast as 3.2 did. Good job :)

stof · 2017-07-10T10:04:43Z

src/Symfony/Component/Form/Extension/DataCollector/FormDataCollector.php

+                    Caster::PREFIX_VIRTUAL.'type_class' => new ClassStub(get_class($f->getConfig()->getType()->getInnerType())),
+                );
+            },
+            ConstraintViolationInterface::class => function (ConstraintViolationInterface $v, array $a) {


should we add this caster in the new Validator panel in 3.4 too ?

that's for you @ogizanagi

Sure, I'll check this once this is merged on 3.4.

stof · 2017-07-10T10:48:03Z

src/Symfony/Component/HttpKernel/DataCollector/DataCollector.php

+        return array(
+            '*' => function ($v, array $a, Stub $s, $isNested) {
+                if (!$v instanceof Stub) {
+                    foreach ($a as &$v) {


@nicolas-grekas have you checked whether the usage of a reference here could be the reason why #23110 and #23175 happen (replacing the EntityManager somewhere it should not be replaced, and replacing the content of $_SESSION) ?

I removed the reference so that if this is correct, then it might be fixed

…f (nicolas-grekas) This PR was merged into the 3.3 branch. Discussion ---------- [HttpKernel][VarDumper] Truncate profiler data & optim perf | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23415, #21547 and hopefully #23110 and #23175 | License | MIT | Doc PR | - Commits ------- 754d3a7 [HttpKernel][VarDumper] Truncate profiler data & optim perf

This PR was merged into the 3.3 branch. Discussion ---------- [Profiler] Fix data collector getCasters() call | Q | A | ------------- | --- | Branch? | 3.3  | Bug fix? | yes | New feature? | no  | BC breaks? | no | Deprecations? | no  | Tests pass? | yes | Fixed tickets | N/A  | License | MIT | Doc PR | N/A Relates to #23465. Calling `DataCollector::getCasters()` using self results into overridden methods in child classes never been called. Also removes an unused property. Commits ------- 34e7094 [Profiler] Fix data collector getCasters() call

…taCollector::getCasters() method (ogizanagi) This PR was merged into the 3.4 branch. Discussion ---------- [Profiler][Validator] ValidatorDataCollector: use new DataCollector::getCasters() method | Q | A | ------------- | --- | Branch? | 3.4  | Bug fix? | no | New feature? | no  | BC breaks? | no | Deprecations? | no  | Tests pass? | yes | Fixed tickets | #23465 (comment)  | License | MIT | Doc PR | N/A ~~First commit targets 3.3; see #23516 I didn't re-used the `ConstraintViolationInterface` caster used in the form collector, as it's the purpose of the validator collector to show the constraints data. Commits ------- c725a70 [Profiler][Validator] ValidatorDataCollector: use new DataCollector::getCasters() method

sroze · 2017-11-28T09:37:19Z

Could we have an opt-in for this? As we can see in a few issues, this loses some information that might be useful to the developer. It could be nice to be able to "tune" a "deep debug" or "simple debug" something like that... 🤔

… objects (ogizanagi) This PR was merged into the 4.4 branch. Discussion ---------- [Messenger][Profiler] Remove cutting caster to dump full objects | Q | A | ------------- | --- | Branch? | 4.4  | Bug fix? | no | New feature? | yes  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | N/A  | License | MIT | Doc PR | N/A Obviously, not the ideal solution, but might open the discussion. Not being able to inspect more than 1 level deep in the profiler when dealing with DTOs and VOs in your messages is a pain 😕. You can't either access the `HandledStamp` `result` for instance. This caster truncating collectors data was originally added in #23465 to mitigate performances issues, especially with the Form profiler. But actually a lot of useful information are now hidden to the developper. Opting-out per collector might be a start. Either allowing to do it through config, or directly in code where sensible. Commits ------- e4fc5c0 [Messenger][Profiler] Remove cutting caster to dump full objects

nicolas-grekas added this to the 3.3 milestone Jul 10, 2017

carsonbot added Status: Needs Review HttpKernel VarDumper Bug labels Jul 10, 2017

nicolas-grekas mentioned this pull request Jul 10, 2017

Symfony Profiler performance bogged down by deprecation notices #23415

Closed

nicolas-grekas commented Jul 10, 2017

View reviewed changes

nicolas-grekas force-pushed the profile-sz branch 2 times, most recently from 02c613e to 0b3ee2d Compare July 10, 2017 09:24

stof reviewed Jul 10, 2017

View reviewed changes

nicolas-grekas force-pushed the profile-sz branch 2 times, most recently from 13d571a to 1037f82 Compare July 10, 2017 10:13

stof reviewed Jul 10, 2017

View reviewed changes

nicolas-grekas force-pushed the profile-sz branch from 1037f82 to 1d00ba8 Compare July 10, 2017 12:36

stof approved these changes Jul 10, 2017

View reviewed changes

nicolas-grekas force-pushed the profile-sz branch from 1d00ba8 to 22c0e5d Compare July 10, 2017 13:15

[HttpKernel][VarDumper] Truncate profiler data & optim perf

754d3a7

nicolas-grekas force-pushed the profile-sz branch from 22c0e5d to 754d3a7 Compare July 10, 2017 14:18

This was referenced Jul 10, 2017

Catchable Fatal Error: Argument 1 passed to AttributeBag::initialize() #23175

Closed

Call to undefined method Symfony\Component\VarDumper\Caster\CutStub::clear() #23110

Closed

ogizanagi approved these changes Jul 10, 2017

View reviewed changes

szymach approved these changes Jul 10, 2017

View reviewed changes

nicolas-grekas merged commit 754d3a7 into symfony:3.3 Jul 11, 2017

nicolas-grekas deleted the profile-sz branch July 11, 2017 09:18

This was referenced Jul 11, 2017

Big memory usage increase since Symfony 3.2 #21547

Closed

Symfony 3.3 unsusable with windows ubuntu bash #23202

Closed

This was referenced Jul 15, 2017

[Profiler] Fix data collector getCasters() call #23516

Merged

[Profiler][Validator] ValidatorDataCollector: use new DataCollector::getCasters() method #23517

Merged

javiereguiluz mentioned this pull request Jul 17, 2017

failed to open stream: Resource temporarily unavailable #23097

Closed

fabpot mentioned this pull request Jul 17, 2017

Release v3.3.5 #23555

Merged

ogizanagi mentioned this pull request Nov 10, 2017

Logged exceptions not showing up in the exception tab #24877

Closed

ogizanagi mentioned this pull request Jul 23, 2019

[Messenger][Profiler] Remove cutting caster to dump full objects #32681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HttpKernel][VarDumper] Truncate profiler data & optim perf #23465

[HttpKernel][VarDumper] Truncate profiler data & optim perf #23465

nicolas-grekas commented Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017

ro0NL Jul 10, 2017

stof Jul 10, 2017

nicolas-grekas Jul 10, 2017

szymach commented Jul 10, 2017

stof Jul 10, 2017

nicolas-grekas Jul 10, 2017

ogizanagi Jul 10, 2017 •

edited

Loading

stof Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017

sroze commented Nov 28, 2017

[HttpKernel][VarDumper] Truncate profiler data & optim perf #23465

[HttpKernel][VarDumper] Truncate profiler data & optim perf #23465

Conversation

nicolas-grekas commented Jul 10, 2017 • edited Loading

nicolas-grekas Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Jul 10, 2017

Choose a reason for hiding this comment

ro0NL Jul 10, 2017

Choose a reason for hiding this comment

stof Jul 10, 2017

Choose a reason for hiding this comment

nicolas-grekas Jul 10, 2017

Choose a reason for hiding this comment

szymach commented Jul 10, 2017

stof Jul 10, 2017

Choose a reason for hiding this comment

nicolas-grekas Jul 10, 2017

Choose a reason for hiding this comment

ogizanagi Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

stof Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Jul 10, 2017

Choose a reason for hiding this comment

sroze commented Nov 28, 2017

nicolas-grekas commented Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017 •

edited

Loading

nicolas-grekas Jul 10, 2017 •

edited

Loading

ogizanagi Jul 10, 2017 •

edited

Loading

stof Jul 10, 2017 •

edited

Loading