Add --apcu-autoloader option to enable APCu caching of found/not-found classes #5559

nicolas-grekas · 2016-07-28T09:51:23Z

APCu is near perfect for caching classes: it is really fast.

Even when using static arrays on PHP 7 (see #5174), findFile can be slow in non authoritative mode because of not-found classes. And when the class map is not dumped (esp. on PHP 5.5 where dumped classes don't scale with the number of classes), using e.g. Symfony's ApcClassLoader in front of composer provides a significant speed boost. Let's inline this knowledge into composer, the only loader that should rule them all :)

This PR isn't the same as #5172: instead of decorating the loader, we hook apcu between the class map lookup and the filesystem lookup.

As advised by @lisachenko in https://twitter.com/lisachenko/status/719105126439497728, some use case of APCu can lead to slowness because of memory fragmentation. Yet, not everyone is using APCu in a way that led to fragmentation. And for those (I expect the majority), the speedup is significant.

To keep on the safe side, apcu is enabled as an option with an --apcu command line flag.

alcohol · 2016-07-28T11:36:49Z

Needs to be rebased now.

nicolas-grekas · 2016-07-28T11:41:02Z

Rebased

alcohol · 2016-07-28T12:21:53Z

Cheers. LGTM :)

stof · 2016-07-28T12:30:04Z

src/Composer/Autoload/AutoloadGenerator.php

@@ -624,6 +639,14 @@ public static function getLoader()
 CLASSMAPAUTHORITATIVE;
        }

+        if ($this->apcu) {
+            $apcuPrefix = substr(base64_encode(md5(uniqid('', true), true)), 0, -3);


shouldn't we use the $suffix variable instead of generating another randomized string ?

I think that is a valid suggestion. On the other hand, the user can choose to specify a specific value for $suffix if I am not mistaken. Any chance that could cause issues?

You made the same suggestion on #5172, and I have the same answer :) : suffix doesn't change very often since composer tries to reuse it once dumped. Which means it won't work here.

The suffix is there for cache busting APC (#963), the reuse of the suffix was implemented to address #3701. Using a non-deterministic method of generating the cache busting string will introduce a regression of #3701.

Perhaps the autoload generation should be fixed to prevent similar problems down the line, or the prefix should be created deterministically such as a shasum of a generated file.

This would be problematic for the PSR loader, but I feel if you are dynamically adding files to the autoloader, you wouldn't want to cache misses or you should take care to clear the apc entries when the file is created.

As far as I can tell, people should use either apcu or authoritative classmaps if they want further optimizations than regular classmap generation. They are mutually exclusive it seems, it'd be good to document this but we probably need a new doc/article/autoloading.md or so that details all this because we have slowly a lot of options and trade-offs..

Anyway regarding this point in particular, I don't think it's a regression to #3701 as long as people don't turn on apcu usage, which is fine IMO.

Seldaek · 2016-08-04T12:56:36Z

For php5.6+, is there a benefit to APC vs using --optimize-autoloader --classmap-authoritative? It sounds to me like there isn't, so in that case the code is just unneeded cruft I think (I don't really care about users stuck on 5.5 and below for this, if they want performance they'd better upgrade php first).

nicolas-grekas · 2016-08-04T13:02:41Z

If --optimize-autoloader --classmap-authoritative are both enabled, the added code path in not run at all. I'm sure many run with --optimize-autoloader, but a lot less also enable --classmap-authoritative (we don't). For the rest of us not using it (whatever the PHP version is), then yes, this PR makes a difference.

Seldaek · 2016-08-04T13:06:57Z

I see that the code path is optional but that doesn't make it less crufty in the composer source :) I am just trying to avoid adding more stuff to the autoloader which is already quite complex.

I also see that many people don't add the --classmap-authoritative, but as --apcu is not on by default either, that won't solve the problem that many people don't use either of the flags. If using the former is enough to get the added performance then we need to better educate people, not add more unused flags that do the "same".

nicolas-grekas · 2016-08-04T13:09:43Z

The issue is: is --classmap-authoritative safe? Then what about enabling it by default in 1.3?

nicolas-grekas · 2016-08-04T13:12:08Z

Or what about enabling --apcu by default (and rename it to --no-apcu?)

Seldaek · 2016-08-04T13:15:30Z

It is safe as long as you don't use exclude-from-classmap to exclude classes that you need, because then it can't fallback to the PSR-x autoloader. So for most people it's safe. But for most people I think it's also kinda useless, as it really is only useful if you have a lot of attempts at loading classes that don't exist.. Is that such a common case?

nicolas-grekas · 2016-08-04T14:13:37Z

E.g. SwiftMailer generates misses in composer since it has its own autoloader, or all forward compatible code that do some if (class_exists(...)) checks. Most people won't enable --classmap-authoritative because they don't understand exactly when it will fail, and can't afford a fatal error in prod just because of this (confirmed around me).
So yes, these apcu lines make a difference: they provide performance without compromising safety (or perceived safety).

alcohol · 2016-08-04T14:35:06Z

SwiftMailer generates misses in composer since it has its own autoloader

Pull-request material right there :p

tgalopin · 2016-08-04T15:01:59Z

@alcohol That's not really possible unless refactoring a lot SwiftMailer (or using a different library) as the library execute callbacks on class autoload (not possible using Composer autolaoder).

alcohol · 2016-08-04T15:04:52Z

Fair enough :-)

alcohol · 2016-11-01T14:34:41Z

Also, just curious, but can the same effect not be achieved using https://symfony.com/doc/current/components/class_loader/cache_class_loader.html ?

nicolas-grekas · 2016-11-01T14:42:07Z

Nope: there is no faster cache than the static array when classes exist. This PR is only for classes that are not found, whereas the linked cache is for both (thus slower & less memory efficient).

alcohol · 2016-11-01T14:49:21Z

Right, my bad. I totally overlooked the fact that this is for caching of found/not found only.

stof · 2016-11-02T13:25:29Z

src/Composer/Command/ConfigCommand.php

@@ -341,6 +341,7 @@ function ($val) {
            'sort-packages' => array($booleanValidator, $booleanNormalizer),
            'optimize-autoloader' => array($booleanValidator, $booleanNormalizer),
            'classmap-authoritative' => array($booleanValidator, $booleanNormalizer),
+            'apcu' => array($booleanValidator, $booleanNormalizer),


apcu is too generic as config key IMO. It is not clear this is about the autoloader

maybe switch to apcu-autoloader?

Naming things... if anyone has a better name, please propose. Personally, I'm fine with that name + associated doc :)

Haha I missed your comment while adding mine :) Let's go for apcu-autoloader

stof · 2016-11-02T13:57:42Z

Pull-request material right there :p

@alcohol the fact that other autoloaders can be registered alongside the composer one must be kept.
For instance, for people having dynamic PSR-X registrations, we encourage them to register a separate instance of the Composer ClassLoader, to avoid mutating the generated classmap and config which would break optimizations (by copying out of the OPCache shared memory) and would not work if the class was already checked (due to in-memory caching of misses).
Having autoloading misses is not an exception for an autoloader. It has to be expected.

@nicolas-grekas classmap-authoritative is safe as long as your classmap is not missing classes. Here are the things breaking autoloading when enabling it:

accessing a class which was excluded from the classmap by exclude-from-classmap. This case should be considered a bug in your config IMO, and so we should teach people about how to fix the issue instead of avoiding the usage of the authoritative mode
accessing a class which was added on the filesystem (in a PSR-X location) after the classmap was generated. This can happen in 3 cases:
- if you use code generation -> the proper way is to register a separate ClassLoader instance responsible to autoload these classes instead of expecting them to be handled by the generated autoloader. It also avoids issues related to the in-memory caching of misses in case the code generation happens after the first class check.
- if you change the PSR-X config at runtime. But this is already not working well anyway due to the in-memory caching of misses (see Cached ClassMap entries not reset after registering new prefix #5619) and the clean solution is to use a separate ClassLoader instance instead.
- if you are working on the codebase to edit it -> this should never happen in production. But it means that authoritative classmaps should not be enabled for development environments (otherwise, we loose all benefits of PSR-X class loading compared to the classmap-based class loading available in Composer)

IMO, the right fix is to encourage people to enable authoritative mode in production when they are concerned about performance.

nicolas-grekas · 2016-11-02T14:09:10Z

IMO, the right fix is to encourage people to enable authoritative mode in production

I don't expect people to understand the failing-cases-inventory you just posted above (applies for me: while understand it now, I don't want to remember it).

Which means I far prefer a fail safe configuration than a fail hard one (authoritative mode enabled).

lisachenko · 2016-11-02T14:15:29Z

For the current PR I can imagine another way of using APCU: do not ask APCU each time when we need a class name to be resolved. This should be integrated with classmap functionality: we initialize a loader, then ask one APCU/WinCache/etc single key that returns us an array with classmap and set it to the loader, so loader will quickly look at provided classmap and can bypass all the logic of file searching.

New keys can be added to the APCU if we don't have them in our classmap. Thoughts?

stof · 2016-11-02T14:32:07Z

@nicolas-grekas the list I posted can be simplified like this:

you excluded it from the classmap (obviously not compatible with the fact of using the classmap only)
you alter the source code after the classmap generation => should either not be done in production, or should be autoloaded in a dedicated way.

Note that the second case already works bad today in case the source code changes are done in the middle of the process, due to the missing class caching (the difference is that the next PHP process would see your generated class).

nicolas-grekas · 2016-11-02T14:40:20Z

@lisachenko how would that be better than the current patch? Loading a classmap from apcu is going to be way slower that using PHP7 static array (because it needs unserializing that big array and create it in local memory).

stof · 2016-11-02T15:26:10Z

@lisachenko loading the classmap from APCu won't make things faster than loading the classmap from the OPCache shared memory (where it stays for PHP 5.6+ thanks to optimizations done by @nicolas-grekas).

The goal of this PR is to optimize the case of class loader misses, not of class loader matches.
Storing a single APCu key containing an array would imply storing the missingClasses array, not the classmap. And then, the initialization would require an apcu_fetch call even when you don't have a cache miss (unless we initialize it from APCu only for the first classmap miss, which adds more complexity too).
And it would also involve updating the array for each missing class detected, which is subjects to locks (if 2 processes are mutating the array in parallel) and potentially fragmentation.

lisachenko · 2016-11-02T15:28:40Z

@nicolas-grekas I'm not sure about the benefit of using small keys for RAM.

Typically, one FPM serves several apps (or one app, but from several directories after symlink switching during), each app will use then APCU memory for allocation simultaneously and each allocated block can be aligned not very well, so access time will grow due to slow fragmentation. You argument about big array unserialization probably is not valid because your solution also requires N*string number unserializations. I can bet that one single big array unserialization should be faster.

To prevent locks apcu_cas() function can be used somehow to atomically change one version of classmap to another without locks. But this should be developed properly...

lisachenko · 2016-11-02T15:32:45Z

@stof loading the classmap from APCu won't make things faster than loading the classmap from the OPCache shared memory (where it stays for PHP 5.6+ thanks to optimizations done by @nicolas-grekas

Yes, it's the best way )

nicolas-grekas · 2016-11-02T15:36:47Z

Memory fragmentation occurs when allocating, then unallocating, then reusing the unallocated space again but for a different data size.
When the cache is cold, allocating+unallocating your big array will happen at least N times! (were N == total number of class misses).
With the current implem, unallocating will never happen, which means it won't contribute to fragmentation at all by itself.
About N*string number, you should consider that N here is the number of class misses for the current request. Your big array has an N-size where N is the total number of classes missing in the class map.
For these reasons, I'd rather stick on the current implem :)

nicolas-grekas · 2016-12-05T12:30:25Z

~~Closing: this can be done in user land by using Symfony's ApcClassLoader:~~

use Symfony\Component\ClassLoader\ApcClassLoader;

$loader = require __DIR__.'/../app/autoload.php';
(new ApcClassLoader('missing_class_', $loader))->register();

Seldaek · 2016-12-05T12:54:48Z

Do you get the same perf with the decorating loader? Because I'd guess you're hitting apcu for every class being loaded that way instead of an isset() in the classmap. Maybe this doesn't result in a noticeable difference, but I'm curious to know.

nicolas-grekas · 2016-12-05T13:09:48Z

In fact, this is decorating the loader only for misses:

the composer loader is registered first, so it is the only called one on a hit!
on a miss, the apc class loader is called so it can only get a "false" when calling findFile (and this "false" is cached in the composer loader so there is no extra cost when apc is cold).

Seldaek · 2016-12-05T23:04:38Z

But in that case I don't understand what the apc loader is good for if you get the hit of figuring out that composer doesn't know how to load the class anyway?

nicolas-grekas · 2016-12-06T07:44:27Z

Ιndeed, sometimes I feel stupid :)
Please advise what's missing to this PR to get it merged!

JasonChilds · 2016-12-06T09:24:20Z

Why are you in my email

…

On Dec 5, 2016 5:04 PM, "Jordi Boggiano" ***@***.***> wrote: But in that case I don't understand what the apc loader is good for if you get the hit of figuring out that composer doesn't know how to load the class anyway? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#5559 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVokIzhGkSfx3b5ZyQYkMChvqi-7PFwbks5rFJiOgaJpZM4JXCyv> .

alcohol · 2016-12-06T09:32:54Z

You are receiving this because you are subscribed to this thread.

Check the notifications box in the sidebar on this thread, you should be able to unsubscribe from there (or use the link in the email).

Seldaek · 2016-12-06T09:39:01Z

src/Composer/Command/InstallCommand.php

@@ -47,6 +47,7 @@ protected function configure()
                new InputOption('verbose', 'v|vv|vvv', InputOption::VALUE_NONE, 'Shows more details including new commits pulled in when updating packages.'),
                new InputOption('optimize-autoloader', 'o', InputOption::VALUE_NONE, 'Optimize autoloader during autoloader dump'),
                new InputOption('classmap-authoritative', 'a', InputOption::VALUE_NONE, 'Autoload classes from the classmap only. Implicitly enables `--optimize-autoloader`.'),
+                new InputOption('apcu', null, InputOption::VALUE_NONE, 'Use APCu to cache found/not-found classes.'),


--apcu-autoloader here too (in all except dump-autoload) would be clearer I guess.

Seldaek · 2016-12-06T09:44:06Z

I don't know Jason. Do you also hear the voices in your email? :p

Anyway joking aside click the link below and then unsubscribe on the right of this comment..

@nicolas-grekas OK I was kinda hoping you found a magical solution and I wouldn't have to merge this, but I guess not ;) Wrote down a few comments about naming, I think if those are addressed it's mergeable, would be great to have the docs article if you have time too but otherwise no biggie.

nicolas-grekas · 2016-12-06T10:07:41Z

PR updated (test failure unrelated IMHO)

staabm · 2016-12-06T10:18:39Z

doc/03-cli.md

@@ -105,6 +105,7 @@ resolution.
  a bit of time to run so it is currently not done by default.
 * **--classmap-authoritative (-a):** Autoload classes from the classmap only.
  Implicitly enables `--optimize-autoloader`.
+* **--apcu-autloader:** Use APCu to cache found/not-found classes.


typo: autoloader

…d classes

…ers (nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [ClassLoader] Deprecate Apc/WinCache/Xcache class loaders | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | no | Fixed tickets | - | License | MIT | Doc PR | - See composer/composer#5559 Commits ------- fa36e1d [ClassLoader] Deprecate Apc/WinCache/Xcache class loaders

Seldaek · 2016-12-06T23:27:35Z

Thanks! Please add some details about apcu tradeoffs (fragmentation, do not combine with --classmap-authoritative, less memory use than optimized classmap?, ..?) to #5947 if you get a chance so it's not lost and someone else can carry on with docs @nicolas-grekas.

nicolas-grekas force-pushed the apcu branch from 9038462 to 0f8598a Compare July 28, 2016 11:40

alcohol added the Reviewed label Jul 28, 2016

stof reviewed Jul 28, 2016
View reviewed changes

nicolas-grekas changed the title ~~Add --apcu option to enable APCu caching of found/not-found classes~~ Add --no-apcu option to turn APCu on/off for caching of found/not-found classes Aug 7, 2016

nicolas-grekas changed the title ~~Add --no-apcu option to turn APCu on/off for caching of found/not-found classes~~ Add --no-apcu option to turn APCu on/off for caching found/not-found classes Aug 7, 2016

nicolas-grekas force-pushed the apcu branch from 5643b46 to 0f8598a Compare August 7, 2016 08:42

nicolas-grekas changed the title ~~Add --no-apcu option to turn APCu on/off for caching found/not-found classes~~ Add --apcu option to enable APCu caching of found/not-found classes Aug 7, 2016

Seldaek added this to the 1.3 milestone Sep 12, 2016

stof reviewed Nov 2, 2016

View reviewed changes

nicolas-grekas closed this Dec 5, 2016

nicolas-grekas deleted the apcu branch December 5, 2016 12:30

nicolas-grekas restored the apcu branch December 6, 2016 07:45

nicolas-grekas reopened this Dec 6, 2016

Seldaek reviewed Dec 6, 2016

View reviewed changes

nicolas-grekas force-pushed the apcu branch from 0f8598a to 65162f4 Compare December 6, 2016 10:03

staabm reviewed Dec 6, 2016

View reviewed changes

Add --apcu-autoloader option to enable APCu caching of found/not-foun…

6d4e60b

…d classes

nicolas-grekas force-pushed the apcu branch from 65162f4 to 6d4e60b Compare December 6, 2016 10:21

nicolas-grekas mentioned this pull request Dec 6, 2016

[ClassLoader] Deprecate Apc/WinCache/Xcache class loaders symfony/symfony#20777

Merged

nicolas-grekas changed the title ~~Add --apcu option to enable APCu caching of found/not-found classes~~ Add --apcu-autoloader option to enable APCu caching of found/not-found classes Dec 6, 2016

Seldaek mentioned this pull request Dec 6, 2016

Document autoloader optimization procedures #5947

Closed

Seldaek merged commit 6d4e60b into composer:master Dec 6, 2016

nicolas-grekas mentioned this pull request Mar 9, 2017

Inconsistency in Commands when enabling apcu-autoloader #6237

Closed

nicolas-grekas deleted the apcu branch January 4, 2018 16:40

Add --apcu-autoloader option to enable APCu caching of found/not-found classes #5559

Add --apcu-autoloader option to enable APCu caching of found/not-found classes #5559

Conversation

nicolas-grekas commented Jul 28, 2016 • edited Loading

alcohol commented Jul 28, 2016

nicolas-grekas commented Jul 28, 2016

alcohol commented Jul 28, 2016

Choose a reason for hiding this comment

alcohol Jul 28, 2016 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Jul 28, 2016 • edited Loading

Choose a reason for hiding this comment

sbuzonas Nov 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seldaek commented Aug 4, 2016

nicolas-grekas commented Aug 4, 2016

Seldaek commented Aug 4, 2016

nicolas-grekas commented Aug 4, 2016

nicolas-grekas commented Aug 4, 2016

Seldaek commented Aug 4, 2016

nicolas-grekas commented Aug 4, 2016 • edited Loading

alcohol commented Aug 4, 2016

tgalopin commented Aug 4, 2016

alcohol commented Aug 4, 2016

alcohol commented Nov 1, 2016

nicolas-grekas commented Nov 1, 2016 • edited Loading

alcohol commented Nov 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stof commented Nov 2, 2016

nicolas-grekas commented Nov 2, 2016

lisachenko commented Nov 2, 2016 • edited Loading

stof commented Nov 2, 2016

nicolas-grekas commented Nov 2, 2016

stof commented Nov 2, 2016

lisachenko commented Nov 2, 2016

lisachenko commented Nov 2, 2016

nicolas-grekas commented Nov 2, 2016 • edited Loading

nicolas-grekas commented Dec 5, 2016 • edited Loading

Seldaek commented Dec 5, 2016

nicolas-grekas commented Dec 5, 2016 • edited Loading

Seldaek commented Dec 5, 2016

nicolas-grekas commented Dec 6, 2016

JasonChilds commented Dec 6, 2016 via email

alcohol commented Dec 6, 2016 • edited Loading

Choose a reason for hiding this comment

Seldaek commented Dec 6, 2016

nicolas-grekas commented Dec 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seldaek commented Dec 6, 2016

nicolas-grekas commented Jul 28, 2016 •

edited

Loading

alcohol Jul 28, 2016 •

edited

Loading

nicolas-grekas Jul 28, 2016 •

edited

Loading

sbuzonas Nov 8, 2016 •

edited

Loading

nicolas-grekas commented Aug 4, 2016 •

edited

Loading

nicolas-grekas commented Nov 1, 2016 •

edited

Loading

alcohol commented Nov 1, 2016 •

edited

Loading

lisachenko commented Nov 2, 2016 •

edited

Loading

nicolas-grekas commented Nov 2, 2016 •

edited

Loading

nicolas-grekas commented Dec 5, 2016 •

edited

Loading

nicolas-grekas commented Dec 5, 2016 •

edited

Loading

alcohol commented Dec 6, 2016 •

edited

Loading

nicolas-grekas commented Dec 6, 2016 •

edited

Loading