[FrameworkBundle][HttpKernel] Add the ability to enable the profiler using a parameter #43138
Conversation
carsonbot
changed the title
dunglas
changed the title
dunglas
force-pushed
the
feat/profiler-collect-parameter
branch
2 times, most recently
from
161e027
to
5dcb5c5
Compare
dunglas
force-pushed
the
feat/profiler-collect-parameter
branch
from
9b3043a
to
f9d5d21
Compare
|
Symfony 2.0 used to have a config setting allowing to provide a RequestMatcher service to activate collecting. This is more flexible than giving a parameter name in the config. Also note that part of the overhead is caused by the fact that services are decorated with Traceable wrappers, and those would still be used if you enable the profiler with |
|
@stof most of the overhead looks caused by the extra IOs, at least on my configuration (Docker for Mac). I checked and just disabling the collection improves the performance of 30%. Regarding the request matcher, do you know why it has been removed? This probably more flexible but less practicable. Here you can just pass a query parameter to enable/disable the tool without having to change the config. We could even think about a tiny WebExtension to do that (similar to the Blackfire companion). |
|
Suggestion: why not enabling / disabling with a cookie or with some switch in the symfony debug bar ? |
|
The motivation for removing it was to discourage using the profiler in production, as the But as you can see in the PR discussion, it was already useful to save some resources in dev at the time, and the performance cost related to collectors is growing since then. |
This PR was merged into the 4.4 branch. Discussion ---------- [HttpKernel] Relax some transient tests | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - As observed in #42824 and more recently #43138, these tests randomly break for some reason. Replaces #42824 Commits ------- 30fa29f [HttpKernel] Relax some transient tests
|
Rebase needed to make the CI green |
dunglas
force-pushed
the
feat/profiler-collect-parameter
branch
from
4ed6ef2
to
c196897
Compare
| @@ -79,6 +81,10 @@ public function onKernelResponse(ResponseEvent $event) | |||
| } | |||
|
|
|||
| $request = $event->getRequest(); | |||
| if (null !== $this->collectParameter && null !== $collectParameterValue = $request->get($this->collectParameter)) { | |||
| true === $collectParameterValue || filter_var($collectParameterValue, \FILTER_VALIDATE_BOOLEAN) ? $this->profiler->enable() : $this->profiler->disable(); | |||
There was a problem hiding this comment.
no need for the true === $collectParameterValue || part, it's a µoptim that we can remove imho
…using a parameter
fabpot
force-pushed
the
feat/profiler-collect-parameter
branch
from
c196897
to
eecff07
Compare
|
Thank you @dunglas. |
The Symfony Profiler is a convenient development tool, however, in some cases, it has a huge overhead.
I used Blackfire to benchmark a development environment on a real-world project and noticed that the average response time was reduced by 30% just by disabling the profiler.
This PR adds a new configuration option allowing to enable and disable the profiler on a per request basis using a query parameter, a form field (for
POSTrequests), or a request attribute. This PR allows keeping the benefit of this tool while improving the developer experience.To enable it:
Note: it's also possible to enable the collection by default and to disable it using the parameter
Then, to have the profiler, the user just has to add
?profile=1to any URL.