Use Composer's optimized ClassLoader for Core/Component classes

I'm not sure I'm down with an autoloading mechanism that makes contrib a second class citizen. Admittedly with Views in core that eliminates about 2/3 of all classes in contrib right now, but presumably that will change over time. :-)

Improvement for non-APC:

=== core..core-1818628--core compared (508d81486ceb0..508d99752ddc5):

ct  : 37,686|37,686|0|0.0%
wt  : 143,797|143,874|77|0.1%
cpu : 144,009|144,010|1|0.0%
mu  : 9,707,000|9,707,000|0|0.0%
pmu : 9,810,424|9,810,424|0|0.0%

=== core..core-1818628--2 compared (508d81486ceb0..508d998f99fa4):

ct  : 37,686|36,861|-825|-2.2%
wt  : 143,797|140,511|-3,286|-2.3%
cpu : 144,009|140,009|-4,000|-2.8%
mu  : 9,707,000|9,702,936|-4,064|-0.0%
pmu : 9,810,424|9,804,560|-5,864|-0.1%

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage
metric: run1|run2|diff|diff%

Can we instead of using Composers add a ClassMap cache loader in front of APC / Symphony class loader as Decorator?

And then use drupal_save / drupal_load and generate class map once on the fly so it only works for added contrib modules?

Well, could we just implement our own dump command called from inside of Drupal:

http://drupalcode.org/project/composer.git/blob/c7d084dafe1a5a81a6bf68af...

If we set the target_dir to a temp dir (instead of composer) and write the files from /tmp/... via drupal_php_storage('composer')->write('filename') I think we could then load the classmap file directly via drupal_php_storage('composer')->load('classmap.php') and then implement our own Decorator.

That was the idea.

Using a class map including all classes on a Drupal site is going to mean it has a lot of classes in it that will never be used - think of views plugins from contrib and similar - even for enabled modules, let alone disabled ones. It might be quicker than the universal classloader we're using now, but we should check it against Symfony's APC classloader for comparison.

I'd like to look at building the class map on-demand and writing a single entry to APC at the end of the request if new classes are found - similar to how CacheArray works. That means one APC get per request, occasional sets, no compile step and only fetching the classes that are used. We could then use that class entry to write a classmap if we wanted too. I mentioned this to Seldaek at Symfony Live London and he said he'd consider something like that with benchmarks so it probably makes sense to do it in a PR to Composer.

Over in #1971198: [policy] Drupal and PSR-0/PSR-4 Class Loading, I did some basic profiling on our current situation. This is essentially a duplicated post from over there.

In our current ClassLoader.php file, here are the numbers for calls to file_exists():

                                   | file_exists() hits | file_exists() misses 
Typical page load (a normal node)  | 385                | 14
List of content (a view)           | 312                | 10
Cache clear                        | 430                | 55

Using a simple script to test calls to file_exists() benchmarks the importance of file_exists():

$time_pre = microtime(TRUE);
for ($n = 0; $n < 100000; $n++) {
  is_file($n);
}
$time_post = microtime(TRUE);
print $time_post - $time_pre;

For files that do not exist, 100,000 calls takes an average of 0.55 seconds. For files that do exist, 100,000 calls takes an average of 0.35 seconds after the first call. The first call itself if the files exist take the longest of all, 2.5 seconds. Calling 100,000 calls on the same file is essentially free, taking 0.007 seconds. Running this script with APC turned on or off has no significant impact.

So basically, file_exists() has a small amount overhead, and only slightly more if the files are not found, but at the scale we're currently seeing, it's not enough to worry about. I had expected thousands of calls and a lot more missing files. In total, we're only spending 0.00035 seconds per page load in the Symfony autoloader's findFile() method. So it's not slow in the least bit.

---

So in this situation, it appears that the ClassLoader is not at all expensive, even in its current state. The calls to file_exists() aren't free, but they're not particularly expensive either. One caveat in my benchmarking is that this is done on my localhost, which has an SSD, but even OS-level caching would likely mitigate repeated calls to the same files.

Hm, upon reflecting on @Fabianx's numbers above, I'm curious how this patch resulted in what looks like a 2% overall improvement(?) What's actually being benchmarked in #4?

Over in #1744302-17: [meta] Resolve known performance regressions in Drupal 8, I saw a 10% difference (185ms vs. 166ms) between using Symfony\Component\ClassLoader\ClassLoader and Symfony\Component\ClassLoader\ApcClassLoader. That's on a 2010 MacBookPro with a non-SSD drive. According to #1744302-23: [meta] Resolve known performance regressions in Drupal 8, some of that is not due to file_exists(), but I think a good portion still is, at least on Mac 10.6 with non-SSD.

some of that is not due to file_exists(), but I think a good portion still is, at least on Mac 10.6 with non-SSD.

Yeah, it's possible that some of it is merely in the looping that the autoloader goes through. On a normal page load it does over 10,000 strpos() checks. With a straight namespace-to-location map, you can skip all of those.

Quicksketch (#12):

Yeah, it's possible that some of it is merely in the looping that the autoloader goes through.

Exactly.

I did some experiments with http://github.com/Seldaek/autoload-bench.
From #2023325-25: Add interface for classloader so it can be cleanly swapped.

(*) Optimization:
The problem with the Composer and Symfony class loaders is the loop over all registered namespaces.
Recent version of the Composer loader already attempts to mitigate this by using the first character as a predictor. Unfortunately, in Drupal this first character always tends to be "D". So there is still a long list to loop through.
For Drupal it does pay off to have a second predictor index at 7 (first character of the module name) or 9 (third character of the module name, or r/m for Core/Component).

My experiments basically confirmed this observation. It is definitely possible to write a customized version of Composer's loader that is optimized for Drupal, and significantly faster.
(Of course, class map is still ahead of that.)

Every additional predictor character adds a small overhead, esp because we need to support the case where the class name or prefix is too short for the additional predictors. To further optimize this, we really need to do benchmarks. E.g. if you have 400 moudules and themes enabled, then additional predictors will be a blessing. Otherwise, additional predictors will cost more than they help.

The Krautoload loader is also immune to the loop problem, but it is slowed down if class names get too long, and it has an additional level of indirection. If predictor indices are chosen wisely, the predictor technique is more effective than what Krautoload does atm.

(Currently my fork of autoload-bench is a mess, but I am going to improve it and share my results.)

For the class map:
It will clearly be useful, but I think the better solution is to ship Drupal without a classmap by default, and then have a command that can generate a classmap for everything (including modules), not just core.

Here's the benchmark fork:
https://github.com/donquixote/autoload-bench
I am not going to post numbers here, because it would be a lot, and even more to actually explain what they mean...

Just some quick conclusions:
- As expected, classmap is always the fastest. There are different loaders that support classmap, and there is hardly any relevant difference between them.
- findFile() adds a significant overhead. It is better to directly do stuff in loadClass(). Method indirection is expensive.
- Composer (the old one without predictor) and Symfony are generally ok, but get on their knees if there are too many namespaces/prefixes registered.
- A modified composer loader with additional predictor characters clearly beats other loaders in many-namespaces-same-vendor scenarios, and the predictor stuff adds only a tiny overhead in scenarios with fewer namespaces.
- We can add PSR-4 support with very little overhead.

Cool.
Will we keep an option to run it without classmap?
As I understand, we want to commit the generated classmap into the repository, but the developer still has the option to run composer install/update again without --optimize, thus reverting to non-classmap loading (*). Then this developer can add the composer folder to .git/info/exclude, and that's it.
Seems like a decent solution.

And everything else can be done in followups.

(*) The only reason to disable the class map would be if you (as a core developer) remove or rename a class, and then want file_exists() on that class to return false without crashing, without rebuilding the classmap. This could happen if the class is still somewhere in a plugin registry or similar.
It might not be an important case, but it is still good to know that you can opt out of the classmap.

Oh, I only just now realized we are adding all core module namespaces, even for disabled modules!
Will this have any side effects?

One side effect we get, if we don't pay attention, is enabled core module namespaces will be added twice to the the class loader. One time by Composer itself, another time by DrupalKernel.
This might not bother us at all, because it only means that $prefixes['D']['Drupal\system'] will have two entries instead of one. Unless we have 1x $prefixes['D']['Drupal\system'] and 1x $prefixes['D']['Drupal\system\\'].

This will not have an effect as long as no contrib is in play, because the loader will never hit the point where it has to iterate over those namespaces.
Only whenever the loader wants to load a contrib class, it first has to iterate over the core module namespaces, which if we don't pay attention will be twice as long.

Btw, it should be preferable to always do 'Drupal\system\\' *with* trailing slash, to avoid false positives. E.g. 'Drupal\menu' also matches classes beginning with 'Drupal\menu_link', which is not intended.

Otherwise, side effects only occur if some piece of code abuses class_exists() to check whether an extension is installed. Not a good idea in the first place, and I don't think we are currently doing that anywhere in core.

I'd say the approach taken here is ok for now, but we definitely should work on a solution that builds a dynamic class map depending on which modules are enabled.

On https://drupal.org/node/2023325#comment-7858687 RobLoach showed that /core classes (no modules at all) means a classmap of around 288k. Default APC maximum file size is 1mb. So just doing it for /core classes we won't run into issues, but I'd expect us to go over that if we tried to cover core and contrib modules with a classmap (if we were able to figure out how to do that).

THis was discussed by the Performance team and we agree that this is a good idea. If anyone would take it on, that would be welcome.

I assume this just needs a new dump since the "Use composer for everything" issue has been in for a while.

I think for now we just cover /core/lib since that'll be a easy win and there are no problems of scale like catch mentioned.

The follow up I guess would probably be to use Composer\Autoload\ClassLoader::addClassMap(array $classMap) manually like we manually register each namespace to register additional class maps which could exist in separate files including compiled files in secure storage.

Patch in #22 includes /core/vendor in addition to /core/lib. Since this is Composer, that kind of makes sense, but I don't see comments in this issue that explicitly discussed whether that's desirable. Especially for libraries not needed by most requests: like Doctrine, Guzzle, and Zend\Feed, I wonder whether the added memory usage to every request is worthwhile.

Removing the Needs Profiling tag since that was done in #15 and earlier comments.

If adding /core/vendor as #22 does is acceptable, is there anything left to do here before this is RTBC?

I don't think its been discussed because as far as I can tell there isn't really a option to do that sort of thing. We'd have to manually maintain the dump outside of composer which doesn't seem desirable.

https://getcomposer.org/doc/03-cli.md#dump-autoload

You can specify individual directories to add to the auto-generated class map via composer.json in this way:

    "autoload": {
        "classmap": [
            "src/"
        ]
    },

That's how PHPUnit does it. — And btw, because PHPUnit does it, PHPUnit's full/total classmap clutters our runtime classmap in substantial ways...

Needed a re-roll. Interdiff does identify downside to this. We'll need to run this every-so-often as new classes get added.

So we can do something much cooler in addition as discussed today with @donquixote:

We can add a minimal class map to core using core/lib and symfony like done here for all things we need to load the container, which nicely avoids the downside of rebuilding class map again and again ...

and for modules and the rest we can store the classmap in ....

TADA! - the container

The container is loaded from disk anyway, large anyway, does not need to be cached and a rebuild of container rebuilds the class map.

- During container rebuild => persist new class map in container using compiler pass, dump contrib module classes, too as part of that.
- After container loading -> call addClassMap - as simple as that.

If we are really worried about size of the file (opcache has no file limit, its configurable in APC), we could output a warning once container gets too large if APC is used and setting is too low. #fastbydefault

Just need a flag in settings.php to not use class_map so development works, add to example.settings.local.php and we are done.

That really sounds like over-optimization to me, frankly...

I need at least a method to add my class-map, for example using some components from symfony's bundles

@andy: You can add your class map - to the container, too. If we add like an alter hook or event or whatever so that you can react on the class map building,

@Crell: No, its not. It is solving the core vs. contrib problem. The actual implementation detail does not matter that much. (e.g. directly in container vs. in seperate PHP storage files chunked via array_chunk - as discussed with berdir).

It is just an array in memory. Some other structures in core are _way_ larger ... So that is not a real concern, IMHO.

But yes need definitely profile how long the rebuild of container takes before / after.

We would also nicely avoid commit conflicts as our "root" class map only needs to have what is needed to build the container.

In terms of complexity:

20 lines of code I guess plus around 30 to port the broken class map dumper to be only PSR-0 / PSR-4 to where Drupal would find things. Plus tests obviously.

I think there are actual hooks in composer that could be used to get an accurate class map. I've not studied enough to know for sure, but check before you reinvent that wheel.

Eclipse

vs. in separate PHP storage files chunked via array_chunk - as discussed with berdir

Talked to donquixote today about the APC max_file_size issue (only an issue on PHP4, Opcache has no limit apparently) and this seems like a good workaround for that! Didn't think of that this afternoon but solves that blocker at least.

Overall I'm not sure about this issue compared to trying to further optimize the APCu classloader, but if we do this by default and still let people swap in the APCu loader and other variants if they want to, that's completely fine. Would be great to fix the 'fast by default' issue without APCu being a hard requirement for that if we can. Then it's just a question of profiling to ensure it really helps.

Fixing the contrib issue is not an over-optimization, it's fixing a fundamental weakness of the classmap approach which was promised early-on as a perfect solution to the slow default autoloader, despite there being no viable plan to handle modules at all. The default composer autoloader is still one of the worst out of the box performance issues in 8.0.x and it would continue to be bad if we did a half-solution of adding only core classes to the classmap and falling back for everything else.

Even with the optimization of chunking files, there's still the potential for hundreds of unused classes being included in the classmap. See also #2315545-11: Install composer dependencies to D8 when packaging.

If this improves performance, I think this is more of a task than a feature request.

I took another look at that and here is a quick patch that lets us use a classmap until APCClassLoader takes over.

Note for all of core/lib/Core and core/lib/Component and what is in the patch, it is only 162k. I am holding off on that however, because this will mean any rename, will need a composer classmap re-generate, which would be a pain with out default process.

The aim of this minimal initial patch is:

a) Ensure our most used external components are in the class map (could discuss to add routing, too)
b) Ensure Drupal is fast for PageCache - case
c) Ensure everything is class mapped that is needed to install the APCClassLoader fast.

This is the MVP and gives us performance for free, autoload_classmap.php is afterwards 129 kB, which should be okay.

(Ah, yes, it saves about 1-2 ms for the page cache case of 10-12 ms - obviously depending on the performance of file_exists().)

It turns out the APCClassLoader is not optimal though, because apc_fetch() always happens, even though it would not be necessary as its in memory.

The optimal class caching class loader that works in correspondance with composers classmap:

- Gets the class map from composer when constructed
- implements getClassMap and addClassMap calls, so it can update the internal classmap
- Just if it not finds a match in class map reaches out to apc_fetch() / apc_store()

The attached patch makes Drupal faster for both APC classloader and non-APC classloader cases, so a win on both fronts.

BTW.

Even for _all_ of current core/ and vendor/ we only have 591 kB in the classmap file ...

We might consider at least vendor/ (442 kB) for now.

Sleeping over this again, I am very positive, we can use a pure class map based approach in core:

- We have the Symfony class-loader component
- Composers dump-class map is nothing besides a copy of Symfony's dumper
- It is perfectly feasible to dump all modules during container rebuild

In comparison all of core/ and vendor/ takes 1.2s to dump and a peak of 10.03 MB.

That means by just re-building the classmap sometimes in core with 'composer dump-autoload -o' is enough for core and the initial class map. (426 kB as of now)

For all modules to do it during composer dump is simple.

I would suggest we store a list of classmaps in the container:

%classmaps = array('php-storage:
', 'module/mymodule/extra-classmap.php')

This first classmap is auto-generated from all modules (when we do the Yamls parsing, etc.)

Then on container load, we load all the classmaps stored in the %classmaps into the autoloader.

With all of core/ and vendor/ being less than 500 kB, I am positive array_chunk will not be needed for a long time, also having classmaps be in the container makes it as easy as registering a Service Provider to add a new class map to the container (and hence fixing the concerns of contrib authors).

Modules using composer with its own vendor/ can even point to the generated composer classmap and not having to create their own.

Also as we have all standardized on PSR-4, we obviously can optimize by only searching src/, also renames lead to class rename and such are not conflicting with a class map.

We just need to re-generate core's classmap whenever we ship a new (beta) release, which is one command and one git commit and thats it.

It can be _way_ simpler.

The problem with the 1 MB APC limit still remains, but a container parameter is the most clean and easiest solution as it also solves the dumping problem nicely.

Container before is 392k, afterwards 604k, class map generation time is minimal.

In this patch:

- Composers optimize flag used to generate classmap for all known static things
- Classmap generated for all modules src/ automatically
- Can be changed by service providers if they want (e.g. also removed things they don't want)
- Can be disabled via settings.php via $settings['classloader_use_container_classmap'] = FALSE, which is suggested for APC classloader, too.

This shows my original idea of #4 is feasible.

Edit:

- The composer generated optimized classmap does not contain any modules, so the classmap of the container complements the the composer classmap.
- I don't think the problem of unused classes is a big one and its good enough performance for most sites that I would consider it okay to enable by default, but could be disabled for really low memory requirements.

Even with that many exceptions it seems is 2 minutes faster for test bot time! (13 min vs. 15 min).

This approach is pretty much finished, but is missing docs:

In this patch:

- container.classmap is only used for modules wanting to add their own libraries, etc. to the class map
- Explicit functions to dump / load the classmap using the same php storage, but a different class name
- Using a real class with a get() function
- Compressing if zlib extension is supported.

Compression brings the class map down to 43 kB on disk and compressing takes 6 ms, while decompressing takes less than 1 ms, which should be acceptable.

Therefore we should never hit the 1 MB problem and this is solved.

Lets see what testbot says.

Last testbot was 43 min, oops, lets see what different enabled vs. disabled makes.

In this patch:

- Used container.classmap_files[] instead of classmap to avoid big data stored in container
- This is merged into the global classmap
- Used container.use_classmap so that load() / store() are not called needlessly
- Renamed the setting to classloader_dump_modules_classmap, which accurately describes what it does

Lets see what testbot says now.

testbots run times are not very consistent. Nor are they representative of real world metrics so I don't think we can use that as a valid benchmark for anything so lets not focus on that.

#43 true, but 43 min as default configuration is likely a problem just for the test suite, because the container is re-build again and again.

Here are the benchmarks of the latest patch:

This compares core with APC classloader vs. this patch with no APC classloader.

This is on PHP5.5 with opcache and apcu profiled the default frontpage anonymously for 1000 times each.

### FINAL REPORT

=== 8.0.x..8.0.x compared (54ea1a6c05613..54ea1c1ba3ba6):

ct  : 39,541|39,541|0|0.0%
wt  : 70,209|69,880|-329|-0.5%
mu  : 18,738,136|18,738,136|0|0.0%
pmu : 18,858,136|18,858,136|0|0.0%

=== 8.0.x..issue-1818628 compared (54ea1a6c05613..54ea1cce37840):

ct  : 39,541|39,389|-152|-0.4%
wt  : 70,209|70,105|-104|-0.1%
mu  : 18,738,136|19,680,928|942,792|5.0%
pmu : 18,858,136|19,800,920|942,784|5.0%

=== SUM: 8_0_x-summary..issue-1818628-summary compared (54ea1d54cc3cd..54ea1d856d93c):

ct  : 39,543,912|39,391,912|-152,000|-0.4%
wt  : 76,143,300|76,296,247|152,947|0.2%
mu  : 18,738,689,552|19,681,379,272|942,689,720|5.0%
pmu : 18,858,701,920|19,801,378,984|942,677,064|5.0%

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage

### XHPROF-LIB REPORT

+-------------------+------------+------------+------------+------------+------------+
| namespace         |        min |        max |       mean |     median |       95th |
+-------------------+------------+------------+------------+------------+------------+
| Calls             |            |            |            |            |            |
|                   |            |            |            |            |            |
| issue-1818628     |     39,389 |     42,301 |     39,392 |     39,389 |     39,389 |
| 8_0_x             |     39,541 |     42,453 |     39,544 |     39,541 |     39,541 |
|                   |            |            |            |            |            |
| Wall time         |            |            |            |            |            |
|                   |            |            |            |            |            |
| issue-1818628     |     70,105 |    116,880 |     76,296 |     75,905 |     81,354 |
| 8_0_x             |     69,880 |    114,788 |     76,143 |     75,934 |     81,057 |
|                   |            |            |            |            |            |
| Memory usage      |            |            |            |            |            |
|                   |            |            |            |            |            |
| issue-1818628     | 19,680,864 | 19,763,808 | 19,681,379 | 19,680,928 | 19,680,928 |
| 8_0_x             | 18,738,136 | 18,871,584 | 18,738,690 | 18,738,136 | 18,738,136 |
|                   |            |            |            |            |            |
| Peak memory usage |            |            |            |            |            |
|                   |            |            |            |            |            |
| issue-1818628     | 19,800,824 | 19,887,016 | 19,801,379 | 19,800,920 | 19,800,920 |
| 8_0_x             | 18,858,072 | 18,994,752 | 18,858,702 | 18,858,136 | 18,858,136 |
|                   |            |            |            |            |            |
+-------------------+------------+------------+------------+------------+------------+

This means the static classmap is on par with the APC file loader.

What takes time (0.5 ms) is adding the class map to the class loader and loading the file in (0.2 ms), but the APC requests take in sum around 0.5 ms, too.

I disabled compression unless the file got too big now, because it made the class map class loader slower in benchmarks.

I am pretty happy with this now, for around 1 MB of memory the performance of the normal class loader is on par with the APCu Class Loader.

I will open a follow-up issue to enable the APCClassLoader by default when APCu is detected - as there is no reason not to.

With this we would have:

- APC for APCu available.
- ClassMap for all other environments and static seeding wanted.

The only thing I think we could still do (but that might also be over-optimization) is to add those 12 classes needed before APCClassLoader to the default autoload_classmap.php via composer.json AND then put the current contents of the autoload_classmap.php to e.g. core/classmap.php and add that as file in 'container.classmap_files', so we only load one big file and don't load a big classmap for core/lib/* and vendor/* when APCu class loader is active.

This is ready for some serious review, but should give the power to the user.

PS: Again the default is disabled here to have a fast test bot, so dumping is not really tested here ...

I will open a follow-up issue to enable the APCClassLoader by default when
APCu is detected - as there is no reason not to.

There are existing issues, and some reasons to do this. One is that sometimes you can do something that breaks the apc classloader, so that it expects a class to exist in a certain file but it doesn't. And then you need to restart apache to fix it. In those issues, I think it was suggested to clear the apc cache in rebuild.php to solve this.

https://www.drupal.org/node/2296527#comment-8934569 has a really nice unit test to ensure this does not regress for vendor or core/lib.

Still needs reviews.

+1 on #28 and #31.

Trade file_exists() for gobs of memory use as an optimization in a beta phase? Bad idea.

The current core/composer.json file has classmap literals already, ostensibly for this reason:

    "classmap": [
      "lib/Drupal/Component/Utility/Timer.php",
      "lib/Drupal/Component/Utility/Unicode.php",
      "lib/Drupal/Core/Database/Database.php",
      "lib/Drupal/Core/DrupalKernel.php",
      "lib/Drupal/Core/DrupalKernelInterface.php",
      "lib/Drupal/Core/Site/Settings.php",
      "vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Request.php",
      "vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/ParameterBag.php",
      "vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php",
      "vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/ServerBag.php",
      "vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/HeaderBag.php",
      "vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php",
      "vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernelInterface.php",
      "vendor/symfony/http-kernel/Symfony/Component/HttpKernel/TerminableInterface.php"
    ]

So this issue should be: Create a metric for determining which classes should be in this existing classmap array, and then add classes which meet this metric.

That means performing an analysis of class use, rather than speed of execution or memory use. Has such an analysis been done in this issue or another one?

Also, once this classmap is in core/composer.json, it means that the optimization goes away when the user uses DRUPAL_ROOT/composer.json, since the core one would then no longer be the project root. Note that this is all use-cases where anyone uses composer to manage anything at all with Drupal.

One way around this would be to create an autoload map file in a global location that looks a lot like the classmap from core/composer.json. This solution is simlar to #2406681: Add an autoload.php in the repo root to control the autoloader of front controllers which has us chasing our tails because we're doing Composer wrong. But we could, and it would be far desirable to eating up gobs of memory with classmaps that remain unused in most cases.

After that classmap is either in composer.json or in an external classmap file, it becomes a question of which classes should eat memory in favor of speed, and that's a whole other process.

As for modules: If we try to avoid loading classmaps for modules which are disabled then we end up with a worse problem than the original issue: We are dependent on the database to discover which modules are enabled. file_exists() looks attractive by comparison.

#25: That's how PHPUnit does it. — And btw, because PHPUnit does it, PHPUnit's full/total classmap clutters our runtime classmap in substantial ways...

One of many reasons phpunit should be 'require-dev' instead of 'require'.

Please read the actual patch in #44. This is for sites not using the APC class loader and its not that much memory.

The patch in #44 was created before that optimization took place.

The original classmap for core/ and vendor/ has been proposed by fabpot of Symfony fame as well.

That is an interesting observation. We've got 2 testing libraries in the patch and when is updated 5 because mink and goutte libraries where added recently for the mink testing issues. Having those in production classmaps is not ideal obviously. Moving them to require-dev definitely allows us to build a classmap that only contains actual production code but doing so requires a composer update(which updates a lot of libraries no related to this patch) and then removing the dev libraries and dumping the autoloader(composer.phar install -o --no-dev). All of that is going to be a big pain while we insist on tracking /core/vendor in git. It seems like our time would be better spent solving that problem and then agreeing to package with -o --no-dev.

    "phpunit/phpunit": "4.4.*"
    "mikey179/vfsStream": "1.*"
    "behat/mink": "~1.6"
    "behat/mink-goutte-driver": "~1.1"
    "fabpot/goutte": "^2.0.3"

The difference currently is 42 KB if I move phpunit to require-dev.

This is probably something we should just do and its simple to do with just a simple command.

Do we have an issue for that already?

Opened #2444615: Move testing packages to require-dev in composer.json with a patch for that.

Tagging.

Bumping this to major because the performance gain potential is quite large. Of course, we now have #2296009: Use APC Classloader by default (when available), so the additional gain may not be that large. But we should do another round of profiling, figure out whether this is worth it. If it's not, then we can close this. Otherwise, this is another gain.

Was there any motion on this at devdays? It seems like we stalled because of needing some policy thought around #2444615: Move testing packages to require-dev in composer.json

edit: fix typo

Re #60: In contrast to the APC classloader this is something that would greatly affect low-fi / shared hosting / ... environments, right? So definitely seems worth doing although profiling (with + without APC) would of course be great.

#62:

#1818628-44: Use Composer's optimized ClassLoader for Core/Component classes has profiling data against APCu class loader.

It is quite the same overhead and comparable in performance, we neither gain something, but also don't loose much in comparison.

It is quite the same overhead and comparable in performance, we neither gain something, but also don't loose much in comparison.

Well, but this is great for hosted sites, they would otherwise just have a way slower classloader implementation.

Quickly re-rolled the patch.

Not really WSCCI...

We don't need a classmap. It's a memory hog for some classes that might not ever be used.

Instead, we can just register an autoloader that knows where Drupal\-namespaced classes live on the file system. Since we know their position relative to the autoloader, we can just include them without a file_exists(). If this fails, then there are bigger problems than this patch.

It's called DrupalNamespaceLoader::autoload(). It's prepended to the queue of autoloaders, so it gets the first chance at finding classes. We limit this autoloader to Drupal\Core and Drupal\Component namespaces, because those are the only ones we know the location for.

Other namespaces fall through and are handled by Composer's loader or the loaders in the kernel.

The question is where to start it up... I have it being registered in the constructor of DrupalKernel, but it could be the front controller or any of a number of places. We just don't want to register it more than once.

The change is included in the patch, but you really should be in the habit of doing composer dumpautoload because the patch changes autoloading in composer.json.

If someone would like to come up with a benchmark for evaluation, that'd be great. :-)

@Mile23:
I suspect you will be faster with str_replace('\\', '/') instead of explode() + foreach().

I expect that with this solution, core classes will load faster than with the Composer loader, but slower than with a classmap.
However, all other classes will be slowed down thanks to the additional loader in the queue.
And I think we should assume that on an average site, core classes are not the majority.

I doubt that the compiled classmap is as much a memory hog as you describe it.
We can get an estimate by counting the line numbers of the classmap php file, and then comparing that with line numbers of other php files.
Maybe there are different memory pools for opcode and variables? But still I expect that we have much larger arrays than the classmap.

EDIT: But sure if someone wants to benchmark this who could complain :)

And of course there is still the APCu key/value store option, as benchmarked in #44.

@Fabianx (#44):
Just saying, there is the theoretical option of a self-updating classmap cache. This means spending more energy on writing to the cache, compared to a key/value store, until the cache is hot, but have the lookup speed of a classmap. Implementation could use a huge APCu array (instead of single values), or a filesystem IO, or the database..

The approaches here probably could use some benchmarks. I looked at this again a couple weeks ago and saw weird numbers so didn't update the patch. Haven't looked into why but we should review. Already has the `Needs profiling` tag just reinforcing that still applies.

#69:

I expect that with this solution, core classes will load faster than with the Composer loader, but slower than with a classmap.
However, all other classes will be slowed down thanks to the additional loader in the queue.

It would be the same with the classmap. We're adding a classmap which is another lookup that has to occur across a giant array before we fall back to PSR-whatever.

In this case, we're asking if the class name has Drupal\ in it, and then we're done making a decision.

And I think we should assume that on an average site, core classes are not the majority.

Well, if you look at the issue summary, you'll see that someone thinks it's an issue. :-)

Also, attached is a file called classes.txt. It was generated by making an autoloader that writes the name of the class to disk, like this:

spl_autoload_register( function ($class_name) {
  file_put_contents(__DIR__ . '/classes.txt', $class_name . "\n", FILE_APPEND);
  },
  FALSE, TRUE);

I cleared all the caches, added it during index.php, and then loaded the front page.

It's 1191 lines (so 1191 autoloaded classes), and 674 of them are Drupal\Core|\Component namespaced, which is ~57%.

The autoloader I've made here touches 675, which makes me wonder which count is off by one...

Interesting fun fact: We get a request to autoload many classes many times. If you do sort classes.txt | uniq -c you'll see that, for instance, Drupal\Core\Database\StatementInterface gets autoloaded 3 times.

You'll also see this, which is rather disturbing:

   2 Drupal\Core\Annotation\PluginID
   3 Drupal\Core\Annotation\Translation
   5 Drupal\Core\Annotation\code
   3 Drupal\Core\Annotation\end
   3 Drupal\Core\Annotation\end_code
   5 Drupal\Core\Annotation\endcode
  25 Drupal\Core\Annotation\ingroup
   1 Drupal\Core\Annotation\property
  11 Drupal\Core\Annotation\see
   2 Drupal\Core\Annotation\todo

That probably needs a followup to filter known keywords out of the annotation system.

But that's a best case of 25 array lookups for Drupal\Core\Annotation\ingroup, or 25 file_exists() at worst case.

I doubt that the compiled classmap is as much a memory hog as you describe it. [..] But still I expect that we have much larger arrays than the classmap.

Having *no* array (and no file_exists()) for approx 2257 class files in core/lib/Drupal means we have more room for FAPI and whatever else.

PSR-0 is our friend. :-)

Patch uses string manipulation instead of arrays.

Oops. Left the logging stuff in there. Hopefully the test will still pass.

It would be the same with the classmap. We're adding a classmap which is another lookup that has to occur across a giant array before we fall back to PSR-whatever.

No, because PHP needs to call another autoloader callback. Whereas the classmap is handled by the same autoloader method.
Differences like this do matter.
Also, the classmap lookup in Composer's class loader happens even if the classmap is empty.

Well, if you look at the issue summary, you'll see that someone thinks it's an issue. :-)

Yes, but not at the expense of other classes. E.g. if we have 50:50 core vs other, then speeding one up at the expense of the other is not a win.

Adding stuff to the classmap within the composer class loader does not slow down other lookups, because it has to look into this array anyway.
(except maybe if php can do faster lookups in an empty array.. which theoretically should be the case)

It's 1191 lines (so 1191 autoloaded classes), and 674 of them are Drupal\Core|\Component namespaced, which is ~57%.

Exactly. And it is going to be less with more contrib modules added.

Patch uses string manipulation instead of arrays.

Which is exactly what we are trying to avoid to speed things up.. otherwise we can simply use the Composer class loader..

But whatever, you should do some benchmarks and see for yourself :)
The above is just educated assumptions based on previous benchmarks and analysis I did myself.

Note that the patch in #68 was repeatedly timing out at the 60 minute limit on testbot, so I cancelled that test.

@Mile23 (#71)
You have one more serious problem.
If someone calls class_exists('Drupal\Core\NonExistentClass');, it will crash, because you don't call file_exists() before including the file. The classmap solution does not have this problem, because the class would not be in the classmap.

Whatever happened in #68 horribly destroyed both the old bots and the new drupalci ones. That patch kept zombie patching on the old bots, and one of the new bots has been stuck at running out of memory for the last four days. https://dispatcher.drupalci.org/job/default/6591/consoleFull.

Im going to hide it so the old bots stop trying it.

Added: #2557433: Add internal, event, and property to the list of ignored annotations in the plugin annotation system

Is this still necessary with PSR-4?

PSR-4 still does file_exists(). Do we care? :-) Anyone could run composer install -o as part of their deployment.

Also we still have a bunch of classes set up to be autoloaded in the classmap here: http://cgit.drupalcode.org/drupal/tree/core/composer.json#n152

You can verify that the classmap ends up in vendor/composer/autoload_classmap.php, through the wikimedia merge plugin process.

If you look at vendor/composer/autoload_classmap.php you'll see that the lion's share of that classmap is PHPUnit classes, because that's what PHPUnit does. This could be solved for the tarball distribution here: #2745355: Use "composer install --no-dev" to create tagged core packages and put the safety railing on here: #2780093: Have simpletest, run-tests.sh enforce their dependency on PHPUnit

#2557433: Add internal, event, and property to the list of ignored annotations in the plugin annotation system is still NR. :-)

The real question at least related to the proposed solution/title is "is this still necessary now that we don't keep vendor in git?" Its more of a question of package building and documentation instead of a policy and patch.

So this is a really old issue.

It's originally from back in ye olden days when we didn't rely on Composer.

We could move forward here by adding Drupal\Component namespaces to our list of classmap optimizations in Composer::preAutoloadDump(), if we thought it was worth it.

We could also note that our Component composer.json files are broken: #2867960: Merge Component composer.json files to account for them during build

Or we could just say closed (outdated).

Seems even more relevant now that Composer has become mainstream with current workflows than it was back when it was created, so it's hardly outdated, IMHO.

We could move forward here by adding Drupal\Component namespaces to our list of classmap optimizations in Composer::preAutoloadDump(), if we thought it was worth it.

This sounds sensible.

Which classes should we classmap, then?

Data point: For Platform.sh, we've been building all Composer-based sites with --optimize-autoloader for a year or two now. We've never run into an issue with that, and I've never seen a bug report that was in any way related to it.

So concerns about "optimize all the things" seem to not be coming to pass, in our experience.

Looks this issue needs another round of work

1) Symfony deprecated ApcClassLoader https://api.symfony.com/3.4/Symfony/Component/ClassLoader/ApcClassLoader...
2) Composer has own apcu based dumper https://getcomposer.org/doc/articles/autoloader-optimization.md#optimiza...

PS: on one of client sites I see 8k calls to apcu_fetch()

So that means 1.5k classes. I think that sounds feasible to dump.

With all the composer changes around building D8, I am all for just optimizing the class map at build time -- also now taht we have php-dev + using APCu class loader from composer as fallback.

Also with PHP 7 memory usage of arrays has dramatically gone down ...

+1 from me to just implement that.

Regarding the modules installed discussion:

- The class map is just a _cache_, the same as APCu

If the modules installed discussion is a problem, we should just roll our own, decorate composers and check PSR-4 / PSR-0 namespace exists before loading it from either class map or APCu or file system.

Not sure if everyone has seen that but somewhat related: https://wiki.php.net/rfc/preload + https://github.com/composer/composer/issues/7777, so we could preload all the classes that are required on a majority of the requests to always be around and skip quite a bit of IO completely, with the downside of having to restart the server but on many managed environments like platform.sh, that's the case anyway.

My exp still tells that we should use #2704571: Add an APCu classloader with a single entry
The main reason is to prevent apcu call for each class (composer's class loader also query each class with single entry)

Just a heads-up that the plan from the Composer initiative is that drupal/drupal will be a core-dev-only package, and that drupal/drupal-project-legacy and others will be the way people generate their codebase. #2982680: Add composer-ready project templates to Drupal core

So whatever decision is made here for general-purpose optimization might need to be replicated in those other templates.

Then, individual projects could amend their autoloading config as needed, including composer install --optimize-autoloader or --apcu-autoloader in their deploy script.

It seems like one stopgap for the autoload-modules problem would be to add PSR-4 entries in core/composer.json for all the core modules. Then composer install -o at least generates the classmap for all the classes.

Then, in an ideal world, contrib would include their PSR-4 in their composer.json and we'd have the whole thing.

fwiw #2704571: Add an APCu classloader with a single entry is still my preference here as well. That could then function as a source for (optional) class preloading in PHP 7.4, since it would be based on real-life site usage. There are tonnes of classes in core/lib which hardly ever get used and some classes from modules will be used constantly.

A classloader with a single apcu cache entry might be faster once it is fully populated, but I think that getting to that point is going to be quite painful and slow. It's similar to our cache collector pattern, which uses a pretty complex lock pattern to prevent unclean caches and in this case, there would be hundreds of possibilities for clashing read/write cases.

I've been testing things a bit recently and composer has apcu support too, you just have to opt-in there as well, so using optimize for all non-module classes and apcu for module-provided classes worked quite well for me.

On my side it removed 8K calls for each class to apcu_fetch

Did you test it in comparison with the composer classloader with apcu support? (composer install -o --apcu-autoloader)

I would expect the that the difference is much smaller then.

@Berdir I did not after checking https://github.com/composer/composer/blob/master/src/Composer/Autoload/C...

I know that it's separate cache entries, my point is that in combination with the optimized classloader, it's *only* used as a fallback for module-provided classes, anything in vendor/ and core/lib will be fetched from that.

Ah, yep! Makes sense
We still need a drupal classloader as bridge

Reparenting to the Drupal 9 issue tree, since this may need to be done as part of moving to Symfony 4 as per #3020296: Remove Symfony's classloader as it does not exist in Symfony 4.

Can I ask, what is the impact here regarding tarball installations? I do not personally have any sites installed only from tarball, but it seems there's some distribution-management concerns here? AFAIK we do not currently ship any composer artifacts in the tgz download? Or am I missing something very obvious? Answered my own question here... of course we do, I just never use the tarball...

I know there's been a lot of work on the composer integration since this was last active, so perhaps that also changes the perspective a bit.

Re: #111:

The impact on a user of the tarball as of 8.8.0+ is that you can say composer dumpauto -o or --apcu as part of your deploy process, and it will work.

Followup on #100:

As of Drupal 8.8.0, the tarball is constructed by running composer create-project drupal/legacy-project.

You can see the 8.8.0 drupal/legacy-project composer.json file here: https://git.drupalcode.org/project/drupal/blob/8.8.0/composer/Template/L...

It does not use any special autoloader directives, though the generating script might add -o or somesuch. That's doubtful however. @mixologic would be the source of truth about that.

It's also true that as of 8.8.0, the drupal/drupal composer.json file (the one at the root of the repo) is considered a development setup. So if we change the root composer.json file, that will only affect the autoloader generated for core developers doing core development, and not users of the product.

I'm tempted to close this issue as outdated, but I'll leave that to a maintainer.

I think that the resolution on #3020296: Remove Symfony's classloader as it does not exist in Symfony 4 means that this is now outdated. With Drupal 9 it you want an optimised classlaoder in production you can do composer dump-autoload --optimize and away you go. No other changes are necessary.

As to whether we should recommend people do that. Well that feels like a different question. We're still enabling the APCu optimisation automatically and on DrupalCI there doesn't appear to much benefit from doing this - see #3117410: Use an optimised autoloader in testing so we need more perf testing and work to understand what we should recommend.