-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Reset Profiler data after each request for PHP-PM #18244
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
Comments
I'm unfamiliar with what you're doing but if my assumption is correct, it means that you don't close the request/terminate symfony? Could you explain a bit more about the challenge you're facing? |
What do you mean with 'close the request' or 'terminate symfony'? We do close the request by calling |
@marcj not shutting down the kernel also means that you reuse the same Container instance all the time. This is what could cause issues in several places (and it is exactly what causes issues here). Several collector services are stateful, and so would keep the data and mix them with the next request in the profiler. You will also have issues with the FingersCrossedHandler of Monolog for instance. Rather than trying to list all places where a cleanup would be necessary at the end of the request (which is quite painful as there is no such listing today, and any bundle could contain such a place), have you tried to look at the performance impact of calling |
Instead of doing https://github.com/php-pm/php-pm-httpkernel/blob/master/Bootstraps/Symfony.php#L90-L99 I tried |
ah right, the kernel sets itself only during the initial creation. Try this then: $app->getContainer()->reset();
$app->getContainer()->set('kernel', $app); |
It is indeed painful and quite a bit of work, but I think it's the only way to keep this whole approach alive and to keep the good performance. But I believe it's time to make this list now :) Maybe PHP-PM is the starting shot for it. I just tried the latest comment with Any other hints where I can reset the profiler? |
Ok, I know now why those stats don't get reseted. The |
I have done now some really nasty hacks, but works so far. What do you guys think about making this more pretty by introducing some new API methods to the DataCollector stuff? FrameworkBundle/ProfilerListener could call a method like |
You could solve it with something like |
@marcj maybe you can try to reset state of data collectors using reflection? |
@marcj Making Symfony work better with solution like php-pm is always a good idea. I spent a lot of time a few years back to make sure that Symfony does not leak memory, and we should continue this work. Adding some clean method in the master branch would not solve your immediate problem but makes Symfony stronger in the long term, which is great. |
…EFERENCE (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Add ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - As spotted in #23984 and #18244, supporting php-pm and the likes requires us to have a way to reset services, typically on `kernel.terminate`. Yet, only actually instantiated services need to be resetted. Knowing if a service is initialized or not is doable for public ones thanks to `Container::initialized()`. But for private ones, there is no way to achieve it. I propose to add a new way to reference services via this new const. When a reference has this behavior, two things happens: - at runtime, if the referenced service is not initialized, it behave the same as "ignore on invalid", pretending the service doesn't exist - at compile time, such references are weak, meaning their referenced services *can* be removed, but *cannot* be inlined Commits ------- 0db3358 [DI] Add ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE
This PR was merged into the 1.x branch. Discussion ---------- Added pssibility to reset the profiler In order to implement symfony/symfony#18244 properly, I needed the ability to reset a Twig profile. Commits ------- ea922bb Added pssibility to reset the profiler.
This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle][HttpKernel] Reset profiler | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #18244 | License | MIT | Doc PR | N/A This PR adds the ability to reset the profiler between requests. Furthermore, the profiler service has been tagged with the new `kernel.reset` tag from #24155. For this, I had to readd the ability to define multiple reset methods for a service. Note: This PR requires twigphp/Twig#2560. Commits ------- 8c39bf7 Reset profiler.
Hi!
I'm currently trying to optimize Symfony usage in https://github.com/php-pm/php-pm.
One of those optimizations is to get the correct debugging information in the profiler. Currently after each request only the execution time is correctly calculated. All other stuff like logs, template render times, query logging etc are not reseted after Kernel::terminate(). What we currently do is along these lines: https://github.com/php-pm/php-pm-httpkernel/blob/master/Bootstraps/Symfony.php#L71-L100
I know it's a hack and don't get any support through semver as we hack into the object, but for the moment the only fast way to get things working. If it's working we could provide a pull requests to get this inside Symfony with well defined interfaces.
First Request:

9th Request

Any ideas?
//Edit
We get for each new request a new Debug-Token, as expected.
The text was updated successfully, but these errors were encountered: