Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
161e027 to5dcb5c5Compare
chalasr left a comment
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
9b3043a tof9d5d21Comparestof commentedSep 22, 2021
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 |
dunglas commentedSep 23, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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). |
florentdestremau commentedSep 23, 2021
Suggestion: why not enabling / disabling with a cookie or with some switch in the symfony debug bar ? |
chalasr commentedSep 23, 2021
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#42824Commits-------30fa29f [HttpKernel] Relax some transient tests
chalasr commentedOct 8, 2021
Rebase needed to make the CI green |
4ed6ef2 toc196897Compare| $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 thetrue === $collectParameterValue || part, it's a µoptim that we can remove imho
nicolas-grekas left a comment
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)
c196897 toeecff07Comparefabpot commentedOct 30, 2021
Thank you@dunglas. |
Uh oh!
There was an error while loading.Please reload this page.
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.