Skip to content

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

Closed
wants to merge 1 commit into from
Closed

Conversation

dlsniper
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets symfony/symfony-standard#464 , sensiolabs/SensioDistributionBundle#76
License MIT
Doc PR ~

This PR adds more classes to cache.
See the discussion in sensiolabs/SensioDistributionBundle#76

@mvrhov
Copy link

mvrhov commented Feb 14, 2013

I'm 👎 on putting DataCollector namespace into a class cache because they are dev environment only.

@schmittjoh
Copy link
Contributor

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.

@fabpot
Copy link
Member

fabpot commented Feb 14, 2013

@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.

@schmittjoh
Copy link
Contributor

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.

@vicb
Copy link
Contributor

vicb commented Feb 14, 2013

@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.

@dlsniper
Copy link
Contributor Author

@ALL you are right but then there's the following issue :)

  • Download 2.2 RC2 zip from the website.
  • unpack the archive into two different folders: sf22rc2_ori sf22rc2
  • apply this patch to sf22rc2
  • run /app.php/
  • notice the classes loaded using APC, it should be something like this

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.

@dlsniper
Copy link
Contributor Author

The above run /app.php/ should read: run /app.php/ 3-4 times then proceed to next step

@vicb
Copy link
Contributor

vicb commented Feb 14, 2013

@dlsniper sorry if I can't help right now but as you should know we have other horses to fight here in france atm.

@fabpot
Copy link
Member

fabpot commented Apr 21, 2013

Closing.

@fabpot fabpot closed this Apr 21, 2013
@fabpot fabpot mentioned this pull request Apr 26, 2013
2 tasks
fabpot added a commit that referenced this pull request Apr 26, 2013
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
fabpot added a commit that referenced this pull request Oct 17, 2016
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants