-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add more classes to cache #7064
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
I'm 👎 on putting DataCollector namespace into a class cache because they are dev environment only. |
This is problematic. As soon as you add a class that might be extended, or implemented, and someone reads annotations from that class (for example like JMSDiExtraBundle does), you'll have a significant slow down. I'm wondering how much we gain by adding these classes? In my experience, this is quite insignificant as soon as you use an op code cache. |
@schmittjoh comment is important: the slow down comes from the fact that the annotation reader will then have to parse the cache file (which, by definition, is huge). That's why some of the classes were not added before. We cannot just add classes to the cache blindly, that must be an iterative process. |
Another idea might be to make this a deployment only feature in which case we could be more aggressive. For dev environment, you probably do not care about the small speed increase anyway. |
@fabpot comments are stripped now I think. Some other remarks: we should not add debug classes, the added class hierarchy & interfaces are automatically added so there might be some extra things added here. |
@ALL you are right but then there's the following issue :)
I'm not saying that this solves the problem. It just shifts the problem from the autoloader, which doesn't need to load 150+ classes anymore but only 30+ and brings the performance back to more acceptable levels. The problem that the debug classes are still used in production environment should be a good thing to debug/fix but I won't have time for this until the weekend comes. I've been saying that Symfony 2.2 has a major speed regression for almost two months now and I know that my Symfony skill is not as good as yours guys but maybe you could help me out a bit on this. 3% speed decrees is not good for some people, currently we are at ~25%+. I've did benchmarks for every change related to speed and I've provided links and documentation on how to replicate the issue. I'm willing to do all the work needed to fix the issues related to this but please read my previous comments before saying that this might not help much or it doesn't work and so on. Or if you know how to make the current situation better, please go ahead, close this, open a new issue which says what needs to be done or a PR with the needed changes and I'll gladly test and changes or help out in any way possible. Thank you. |
The above |
@dlsniper sorry if I can't help right now but as you should know we have other horses to fight here in france atm. |
Closing. |
This PR was merged into the master branch. Discussion ---------- Profiler activation | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | #7064, #7071 | License | MIT | Doc PR | symfony/symfony-docs#2565 As stated in #7071, there is no way to disable the profiler completely. Even when the `enabled` flag is set to `false`, the profiler is still registered but the data collectors are not activated. Now, when `enabled` is `false`, the profiler is disabled. To get the old `false` behavior, you now need to set `enabled` to `true` and set the new `collect` flag to `false`. Todo: - [x] update docs - [x] update Symfony SE -- not needed Commits ------- 88ebd62 fixed the registration of the web profiler when the profiler is disabled a11f901 [FrameworkBundle] added a way to disable the profiler f675dd8 Truly disabled profiler in prod
…andre GESLIN) This PR was merged into the 3.2-dev branch. Discussion ---------- [ExpressionLanguage] Making cache PSR6 compliant | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | [#7064](symfony/symfony-docs#7064) Adding Cache component compatible ParserCache in ExpressionLanguage component. I hope you will find it useful :) I would like to make tests also Commits ------- a7352ff [ExpressionLanguage] Making cache PSR6 compliant
This PR adds more classes to cache.
See the discussion in sensiolabs/SensioDistributionBundle#76