-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle][HttpKernel] Add the ability to enable the profiler using a parameter #43138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
161e027
to
5dcb5c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some tests are broken
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 |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for the true === $collectParameterValue ||
part, it's a µoptim that we can remove imho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(just one minor comment)
…using a parameter
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
POST
requests), 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=1
to any URL.