Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Consider deprecating ClassCollectionLoader & co. #20668

Closed
nicolas-grekas opened this issue Nov 28, 2016 · 8 comments
Closed

Consider deprecating ClassCollectionLoader & co. #20668

nicolas-grekas opened this issue Nov 28, 2016 · 8 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Milestone
3.3

Comments

@nicolas-grekas
Copy link
Member

Calling for feedback + experiment from the community on this topic:

on a Symfony standard edition, prod env, no-debug, PHP 7, composer -o (ie normal setup for prod),
if I comment the line requiring bootstrap.php.cache and the call to $kernel->loadClassCache(); in web/app.php, it looks like my setup is not slowed down, if not a bit faster.
I used ab for benching.

That could mean we don't need these optims in PHP 7 era.

If this is confirmed, it means we could consider deprecating ClassCollectionLoader and $kernel->loadClassCache().
Could any of you try the same and report some resulting numbers?
Thanks in advance!
@nicolas-grekas nicolas-grekas added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Nov 28, 2016
@stof
Copy link
Member

stof commented Nov 28, 2016

Yeah, the composer optimized autoloader kinds of defeat this, as it does not need any stat call to identify the file to be loaded.
And then, OPCache takes care of the loading of the file itself.

I'm all 👍 for deprecating this.

The question then is whether we should really ship the new addAnnotatedClassesToCompile in 3.2 if we want to deprecate the whole system in 3.3

Sorry, something went wrong.

@javiereguiluz
Copy link
Member

Even if we don't use it much, I'd say it's too late to remove addAnnotatedClassesToCompile() for 3.2 (which is due in just a few days).

Sorry, something went wrong.

@dmaicher
Copy link
Contributor

I just tested it on one of my bigger projects (so not on a Symfony Standard Edition).

I used composer install --optimize-autoloader --classmap-authoritative --no-dev and ab -n 5000 -c 1 on PHP 7.0.13 with the following (dev) opcache settings:

opcache.enable = 1
opcache.max_accelerated_files = 4000
opcache.revalidate_freq = 3

First 3 runs without any changes:

  1. Time per request: 36.808 [ms] (mean)
  2. Time per request: 36.782 [ms] (mean)
  3. Time per request: 36.809 [ms] (mean)

Then 3 runs with the changes:

  1. Time per request: 35.805 [ms] (mean)
  2. Time per request: 35.782 [ms] (mean)
  3. Time per request: 35.742 [ms] (mean)

So it seems in my tests its around 1ms faster 🎉

@linaori
Copy link
Contributor

linaori commented Nov 29, 2016

If it will be deprecated, I will remove the class load features for the kernel in #20107

@fbourigault
Copy link
Contributor

Same results here. Time per request: 10.540 [ms] (mean) => Time per request: 9.437 [ms] (mean) on a Symfony/API Platform/Sonata application.

@stof
Copy link
Member

stof commented Nov 29, 2016

Then deprecating looks sensible to me.

This was referenced Nov 30, 2016
Closed
Merged
@dbrumann dbrumann mentioned this issue Dec 3, 2016
Merged
@derrabus derrabus mentioned this issue Dec 3, 2016
Merged
@Ocramius
Copy link
Contributor

Ocramius commented Dec 5, 2016

O say kill it with 🔥. This particular cache has only caused a huge load of bugs on my end, so far...

@lisachenko
Copy link

It's end of epoch of pre-compiled class cache files! Burn them all! 🔥

@lisachenko lisachenko mentioned this issue Dec 5, 2016
Closed
fabpot added a commit to symfony/symfony-standard that referenced this issue Dec 6, 2016
@fabpot
feature #1030 Don't load class cache and bootstrap file on php 7 (der…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e21e941 
…rabus)

This PR was merged into the 3.3-dev branch.

Discussion
----------

Don't load class cache and bootstrap file on php 7

As discussed in symfony/symfony#20668, the class cache features should be deprecated. Since the class cache and the bootstrap file are still useful on php 5, we load them only when running on php 5.

A future php-7-only release of Symfony Standard should remove those blocks completely.

Commits
-------

9b2e12f Don't load class cache and bootstrap file on php 7.
@iquito iquito mentioned this issue Dec 10, 2016
Closed
@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Dec 27, 2016
fabpot added a commit that referenced this issue Jan 6, 2017
@fabpot
feature #20735 Deprecate ClassCollectionLoader and Kernel::loadClassC…
dda3760 
…ache (dbrumann)

This PR was merged into the 3.3-dev branch.

Discussion
----------

Deprecate ClassCollectionLoader and Kernel::loadClassCache

| Q             | A
| ------------- | ---
| Branch?       | "master"
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #20668
| License       | MIT

As suggested by @nicolas-grekas in #20668 I added deprecation notices to ClassCollectionLoader and Kernel::loadClassCache.

Commits
-------

660d79a Deprecates ClassCache-cache warmer.
@fabpot fabpot closed this as completed Jan 6, 2017
@nicolas-grekas nicolas-grekas mentioned this issue Jan 20, 2017
Merged
fabpot added a commit that referenced this issue Jan 20, 2017
@fabpot
feature #21353 [ClassLoader] Deprecated the component (nicolas-grekas)
065ac8b 
This PR was merged into the 3.3-dev branch.

Discussion
----------

[ClassLoader] Deprecated the component

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Follow up of #20668

Commits
-------

8a37722 [ClassLoader] Deprecated the component
@xabbuh xabbuh mentioned this issue Jan 22, 2017
Merged
@patrickjahns patrickjahns mentioned this issue Mar 27, 2017
Merged
@nicolas-grekas nicolas-grekas mentioned this issue Apr 17, 2017
Merged
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 11, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
782051e 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 11, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
b971818 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 11, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
b244ca3 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 12, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
7994b85 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 12, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
6813d92 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
bf8e6ce 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
3035676 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
a6cee1d 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
963101b 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
69e3416 
symfony/symfony#21353
symfony/symfony#20668
cmfcmf added a commit to cmfcmf/AdventureLookup that referenced this issue Aug 15, 2017
@cmfcmf
Remove usage of ClassLoader / ClassCache
a447d94 
symfony/symfony#21353
symfony/symfony#20668
@seferov seferov mentioned this issue Sep 1, 2017
Closed
@bobvandevijver bobvandevijver mentioned this issue Oct 27, 2017
Open
@Tobion Tobion mentioned this issue Nov 15, 2018
Closed
imjoehaines added a commit to mention-me/memcached-bundle that referenced this issue Apr 12, 2019
@imjoehaines
Remove call to deprecated method
b866fbb 
See symfony/symfony#20668
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Milestone
3.3
Development

No branches or pull requests
9 participants
@fabpot @javiereguiluz @Ocramius @nicolas-grekas @stof @lisachenko @dmaicher @fbourigault @linaori