Add interface for classloader so it can be cleanly swapped

I think it'd be sensible to adopt whatever interface composer is currently using for Composer\Autoload\ClassLoader. Although I dont actually see them using an interface there which seems odd…

Another bigger issue with all this is AnnotatedClassDiscovery::getDefinitions(), which implement the converse of the autoloader and hardcodes (partial) PSR-0 assumptions.

In order to make the class loader swappable, we'd need an interface to allow interoperability between the different loaders' APIs. Class Loader Adapter can swap between Symfony and Composer's, more could be added.

Another bigger issue with all this is AnnotatedClassDiscovery::getDefinitions(), which implement the converse of the autoloader and hardcodes (partial) PSR-0 assumptions.

I believe that issue is now resolved, I could be wrong though. There might be some left over clean up needed.

@Rob - definitely an interface, that's in the issue summary already. I'll need to review ClassLoader adapter.

Excellent, let me know if you run into anything. It's still quite young, so I'd love your thoughts on additional functionality it could take on. Added it to the OP, and will be adding Travis testing, and possibly a Krautoload interface to it.

Thanks for checking it out. Transferring between loaders currently isn't covered directly by the API. Currently, to do that, you'd do something along these lines:

// Retrieve all old loader definitions.
$map = $loader->getClassMap();
$prefixes = $loader->getPrefixes();
$loader->unregister();

// Create the new loader and add the definitions.
$loader = new RobLoach\ClassLoaderAdapter\Symfony\ClassLoader();
$loader->addClassMap($map);
$loader->addPrefixes($prefixes);
$loader->register();

I think having this covered directly within the interface would be nice though. Perhaps something like this:

$loader = new RobLoach\ClassLoaderAdapter\Symfony\ClassLoader($old_loader);

Any thoughts? Essentially, the classes are swappable, since they all use the same interface. You can call all the same functions across all the loaders. Swapping the class loader objects, however, is currently up to you. That could could be covered by the API as above. Do you think that's something it should cover?

I think a "universal" class loader adapter interface needs to restrict itself to the bare minimum, and avoid anything that could be implementation-specific.

The "getPrefixes()" is one such a thing.
E.g. Symfony's UniversalClassLoader (last time I checked) makes an artificial distinction of prefixes vs namespaces.
If implemented correctly (*), this can be a legitimate implementation choice, and the loader can still behave equivalent to other PSR-0 loaders.
As a consequence, UniversalClassLoader does not store a list of prefixes in the same way as Composer class loader, or Symfony's ClassLoader. So the getPrefixes() cannot work.
On the other hand, addPrefix() / addPrefixes() is very well possible to mimick with a class loader that distinguishes between namespaces and prefixes: If nothing else helps, you register both.

Therefore, I would suggest to remove getPrefixes().
I have rarely seen a legitimate use case, that could not be implemented in a more elegant way..

(*) There are some cases where you can not 100% mimick the composer prefix maps with UniversalClassLoader.
However, Krautoload (attention, work in progress) does make the same distinction (for performance reasons), and it can be made 100% equivalent if we want to. There is just one stupid aspect of composer prefix maps, that I am not sure if I want to implement that. But there is no technical reason why not to.

findFile($class) is another candidate I am not sure about.

If you interpret PSR-0 correctly, you can never be sure whether a file does actually define the class you are looking for, or if the class defined in it has some namespace separators replaced by underscores.
If the class is not in that file, you need to continue with other registered prefixes/namespaces.

Most popular class loaders (Symfony, Composer) ignore this subtlety, and just include the first file they can find.
This is showcased here:
https://drupal.org/node/1971198#comment-7601195 (first patch)
https://qa.drupal.org/pifr/test/565933
The patch (psr-0-scrunity) introduces a test which fails due to this behavior.

Krautoload can be wired up to behave like Symfony/Composer, but it can also be wired up to avoid this problem with a class_exists(*, FALSE) check and include_once instead of include.
In this case, findFile() would make no sense, unless we allow findFile() to include the file. But even then we are not sure because it could be that the class is already defined.. so we would have to parse the file to be sure - ouch.

Although we could agree that it should always return the first one, even if that might be wrong in some case.
Then it would be the same as Symfony and Composer.

Btw, this subtlety depends more on how class_exists() is used, than on the library contents.

Symfony ClassLoader (and Composer, afaik) have another subtlety I am unsure about.
Again, it is showcased here: https://qa.drupal.org/pifr/test/565933
"A PSR-0 class loader will not look in the lib folder of another module."
And explained here: #1971198-131: [policy] Drupal and PSR-0/PSR-4 Class Loading

There is something weird and possibly unexpected with Symfony2 ClassLoader prefix registration:
If you register a prefix "MyVendor\Gira", then it will trigger on anything that begins with this prefix, e.g.
class_exists('MyVendor\Gira\Foo')
class_exists('MyVendor\Gira_Foo')
class_exists('MyVendor\Giraffe\Foo')
class_exists('MyVendor\Giraffe_Foo')
It would be possible for sure to reproduce this behavior with a Krautoload plugin. But is this even desirable?

In other words, are class loaders expected to behave exactly the same way as composer/symfony, if they are wrapped in the adapter?

And finally, what do we do with PSR-4 ?
All that we know from Composer until today is aimed at PSR-0. So, is Composer a good starting point in the first place?
Or should we explicitly distinguish PSR-0 and PSR-X ?

Maybe something like this:

// Same as Symfony ClassLoader addPrefix(), with all the oddities of the previous post.
addPrefixPSR0($prefix, $dir)
addPrefixesPSR0($prefixMap)

// Explicitly treat $namespace as a namespace.
// (easy to achieve by appending '\\' to the namespace, but some loaders might have shortcuts for it)
addNamespacePSR0($namespace, $dir)
addNamespacesPSR0($namespaceMap)

// Explicitly treat $prefix as a PEAR prefix, not allowing any namespaced classes.
// (some loaders might not explicitly support that)
addPrefixPEAR($prefix, $dir)
addPrefixesPEAR($prefixMap)

// PSR-4 support
addNamespacePSR4($namespace, $dir)
addNamespacesPSR4($namespaceMap)

We might go with a two-level approach:
1. Stuff that we expect to be supported by every serious class loader on the planet.
2. Stuff that is only supported by specific class loaders. Whenever we swap out the class loader, this stuff may break.

@Damien, that used to call into the class loader directly but was factored out not to. I think it's OK now.

No, it's not. We are hardcoding assumptions about how to transform a fully qualified class name into a file name that is not even implementing PSR-0 properly (in AnnotatedClassDiscovery::getDefinitions()):

$dir .= DIRECTORY_SEPARATOR . str_replace('\\', DIRECTORY_SEPARATOR, $namespace);

Obviously this *should* be in the class-loader, as the class-loader is the only one that knows how to do this transformation.

Thinking about all of this, I feel it would be simpler to just implement a stream-wrapper that exposes the PHP namespace hierarchy, and have *that* be swappable. This would give us full introspection *and* would also give us the best performance when an op-code cache is available (because we would hit the cache once, instead of twice right now with the APC decorator).

Clarifying what I mean, I suggest we make URLs like this work:

namespace://Drupal/Component/Plugin/Discovery/AnnotatedClassDiscovery.php

The autoloader code itself would be something stupid like:

function autoload($name) {
  @include_once "namespace://" . str_replace("\\", "/") . ".php";
}

And you could easily introspect the namespaces:

foreach (glob("namespace://Drupal/Component/Plugin/Discovery/*.php") as $class) {
  // Do something.
}

... or use something like DirectoryIterator or RecursiveDirectoryIterator.

@Damien (#15)
Can APC cache stream wrappers? I mean, can it cache the file *as if* it was at the location passed to the stream wrapper?
Will it skip the invocation of stream wrapper methods on each class lookup?

@donquixote All that we know from Composer until today is aimed at PSR-0. So, is Composer a good starting point in the first place?

The Composer class loader currently supports three different loading methodologies:

1. files to include individual files
2. psr-0 to support PSR-0. It also loads PEAR-like paths just fine (both "namespace" and "prefix" is pretty much covered by this)
3. classmap to generate an associative array of class-path of PHP files

Composer will get the PSR-X autoload methodology when it's accepted. It's looking pretty good so far, I'd love to push up a PR for that just to keep the discussion going. If you have any other issue surrounding it, we should likely split the discussion into individual topics.

@Damien I feel it would be simpler to just implement a stream-wrapper

Sounds like a suggestion for an entirely new class loader, which is somewhat unrelated to having the loader be swappable. Seems like a neat idea, would love to hear more about how you think it would work with other systems. Should open up a new issue to track that discussion. This is just about allowing the class loader to switch out.

@donquixote Are class loaders expected to behave exactly the same way as composer/symfony, if they are wrapped in the adapter?

The Class Loader Adapter tries its best to make the APIs work the same between the different class loaders (see Adapter pattern). Of course, depending on which class loader you're using, the details may differ a bit, but the interface is there to allow you to choose which one and use the same interface between them all.

@donquixote: yes, APC can cache stream-wrapper URIs.

Sounds like a suggestion for an entirely new class loader, which is somewhat unrelated to having the loader be swappable.

@RobLoach: yes, and no. Making the class-loader swappable is a two-sided problem: you have to have an uniform API for defining class prefixes (in different PSR flavor), which is what this issue has been focusing on, but you *also* have the issue of consumers of the class-loader, and in our case mainly the ability to introspect namespaces (that PHP itself doesn't have).

@donquixote: yes, APC can cache stream-wrapper URIs.

Nice..
I am not convinced yet, but it is fun enough to talk about!

There still needs to be the actual implementation of the stream wrapper, that will behave similar to a regular autoloader.
So, the stram wrapper thingie could be seen as a cache layer on top of class loaders.

So..

// This thing implements the PSR-0 or PSR-4 or whatever mappings.
$internalLoader = new MyPersonalClassLoader();

if (!$useStreamWrappers) {
  // Register the loadClass() to the spl autoload stack.
  spl_autoload_register(array($internalLoader, 'loadClass'));
}
else {
  // Wrap in a stream wrapper.
  // Unfortunately, we cannot instantiate.
  StreamWrapperClassLoaderWrapper::setClassFinder($internalLoader);
  stream_wrapper_register('namespace', 'StreamWrapperClassLoaderWrapper');
  spl_autoload_register(function($class) {
    @include_once "namespace://" . str_replace("\\", "/", $class) . ".php";
  });
}

One problem is, this takes us one step back towards "global state".

One problem is, this takes us one step back towards "global state".

Not more then any other autoloader. As long as you can load/unload something when the context changes, having a global state is fine. The kernel supports boot/terminate methods and we should probably register/unregister the autoloader in those anyway.

Closed #1983114: Make the autoloader swappable as duplicate.

There are two levels of being "swappable".

- add an interface for the class loader
- adapter class if the specific one we're using doesn't match it.

The Krautoload patch does the following:

• Krautoload itself has an adapter layer for all the registration stuff, and a SearchableNamespaces abstraction for class discovery.
• Drupal inherits from both of that, and adds Drupal-specific methods, such as addDrupalExtension($name, $dir).
• Drupal components that need a class loader or searchable namespaces, work with these Drupal-native classes, and have no hard reference to Krautoload.

I think this should make it easy enough for core to replace the class loader with another one.

- allow the classloader to be changed in settings.php via $settings and load whichever is there.

The Krautoload patch still has the class loader hardcoded in bootstrap.php, in drupal_classloader(), where it says Krautoload::start($options);. The options (APC or not) are based on the same settings that we used to have for the Symfony loader.

We could make the entire class loader swappable based on a setting in settings.php, in a follow-up.
However, I wonder if that is actually desirable.

• Class loading is a concern for early-bootstrap.
  We might even get to a point in the future where we want it to be available before settings.php - e.g. if we objectify the way that settings.php is loaded.
  Maybe this could be solved with a temporary loader for early bootstrap, that is replaced once we have the settings from settings.php available.
• Performance is top priority.
  Abstraction and swappability is nice, but we don't want to lose performance to added abstraction layers and indirections.
  In a world where we compare APC-cache vs classmap vs front-loading, every level of indirection makes a measurable difference.
  All these micro optimizations may become useless if we throw a clunky abstraction layer in between.
• It may be more tedious than we think.
  Krautoload covers both class loading and class discovery. For convenience, we simply use one object for both, so that the namespace directory mappings are already registered (but we still filter by a distinct set of namespaces, so we don't have to worry about junk in the class loader).
  If someone wanted to replace this with e.g. the Composer loader (e.g. once Composer supports PSR-4), this person would have to hand-craft all the discovery stuff. Or use Krautoload for discovery, and Composer for class loading.
  This means, we'd need a better distinction between class loader and namespace inspector. This is all technically possible, but the question is, do we want it, and is it worth the trouble?
• Class loading is at the heart of the framework.
  So it might be one of the things that can be justified to be hardcoded.

Imo we should get the Krautoload thing in, enjoy PSR-4, and then we still have time to discuss class loader swappability.
The Krautoload patch does not make this any less possible.

Surprising change of mind:
Krautoload is indeed faster than Composer or Symfony for Drupal with many modules enabled.
BUT, it is by far not the best we can do.

I played around with Seldaek's autoload-bench, and found how the Composer loader can be optimized for Drupal, and how it can be modified to support PSR-4.

I therefore propose:

• A Drupal-native ClassLoaderInterface, with
  • Most of the methods from Composer's class loader.
  • An added method addDrupalExtension(), that will do our PSR-4 stuff for modules.
    
  • If some day in the future Composer should support PSR-4, we can add these methods as well.
  • I am not sure about findFile(), see below (*).
• A Drupal-native ClassLoader, with
  • Most stuff copied from Composer. (we could inherit, but we'd gain nothing)
  • Modifications to support PSR-4.
  • Optimizations for the Drupal use case (= many package namespaces in same vendor namespace).
  • I am not sure about findFile(), see below (*).
• An additional parameter in in drupal_classloader(), that allows to set a custom classloader from settings.php.
  (In the future we may want to kill drupal_classloader() and move it to e.g. Drupal::getClassLoader() or whatever, but this is out of scope for now I would say.)
• Class discovery will be done in other ways, out of scope for now.

Someone who wants to replace the class loader needs to define a class implementing the ClassLoaderInterface.
This means, a basic Symfony or Composer loader will not do. You would have to subclass it and make it implement the interface, and make it support PSR-4.

(*) Benchmarks showed that the findFile() indirection adds a measurable/significant overhead.
It is better to put all the logic into loadClass().
For compatibility with decorators, we can either try to mimick findFile() with loadClass() or copy it, or we can make our own decorators that don't need findFile().

I am going to try this stuff, and also publish a fork of autoload-bench on github, and post some results.

The patch introduces
- a ClassLoaderInterface including Drupal-specific stuff.
- a ClassLoader implementation optimized for Drupal (*), with PSR-4 support for modules.
- A $custom_loader option in drupal_classloader(), to be called from settings.php.
- Dedicated methods to add Drupal extensions.
- Dedicated method to add the composer directory (autoload_classes.php, autoload_namespaces.php, other stuff)
- It should still work with Symfony's APC decorator

Benchmarks with autoload-bench will follow, and may result in changes to the algorithm.
We also need more tests.

(*) 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).
The tricky thing is that the prefix or the class name may be shorter than that, so we also need to try with the one-character predictor.
Further performance discussion should be done on github, where we can fork autoload-bench and play with different implementations. Here we should focus on the interface.

A bit of cleanup in drupal_classloader().

The PSR-4 stuff is only in one class (ClassLoader). I am ok to upload a new patch with PSR-4 ripped out.
Do you agree with the rest?

Patch:
Same as #25/#27, but with PSR-4 and performance optimization ripped out.
To recap from #25:
- a ClassLoaderInterface based on Composer and class-loader-adapter, but including Drupal-specific stuff.
- a default ClassLoader implementation based on Composer, but split into AbstractClassLoader vs ClassLoader, to allow for easier subclassing with custom implementations.
- A $custom_loader option in drupal_classloader(), to be called from settings.php.
- Dedicated methods to add Drupal extensions.
- Dedicated method to add the composer directory (autoload_classes.php, autoload_namespaces.php, other stuff)
- It should still work with Symfony's APC decorator.

PHPUnit classes no longer need to be added manually in simpletest_classloader_register().
Instead, they are added with drupal_classloader() in the usual composer fashion.

forgot the interdiff.

It is probably wise to separate the addDrupalExtension() stuff out of ClassLoaderInterface.
See #2033611-20: Port AnnotatedClassDiscovery to PSR-4, and make it agnostic of directory information.

Having a swappable ClassLoader seems like a flawed design, and I outlined that in the class-loader-adapter documentation . You usually want to just initialize and use one class loader for your application, and then use a wrapper that takes advantage of caching methodologies available on the system (APC, Xcache, etc).

Not having the common ClassLoader interface was an issue when we were switching from UniversalClassLoader to ClassLoader, and Plugins were using the ClassLoader as a registry, but now that's much less of an issue. What I'm trying to say is that having a swappable class loader doesn't actually gain us anything. Use a class loader wrapper instead to take advantage of caching.

So far we know very little about the use case of having a swappable class loader.

Someone might just want to swap the decorator.

Someone else might want to swap everything, but still want the autoload_namespaces.php and the autoload_classes.php from Composer.

Someelse else might want to swap everything AND skip Composer's autoload_namespaces.php and autoload_classes.php, e.g. to use an enhanced copy of these files that exists somewhere else (e.g in the sites/sitename/ folder). Or to postpone this step until the first cache miss.

From this perspective, it would be reasonable to have two swapping mechanics: One to specify a decorator, and another to manually set the class loader object.

@Rob, having more than one class loader active at the same time is very bad for performance, changing which classloader you use is not the same thing and I don't see how that's 'flawed'.

I'm not suggesting having more than one active class loader, I'm suggesting allow the decorator to be switched so that we can use a different caching method than just APC: #2060425: Improve ClassLoader decorator support

With that in place, you could completely switch the behaviour of how classes are loaded. Build on top of what Composer provides, change the way it functions. Use APC, WinCache, add PSR-4 capabilities to it, etc.

Similar to the container, if we actually created a classmap including all module classes on a large site, I could see that exceeding the default APC file size maximum.

One way to find out for just Drupal Core and all its vendors:

$ composer install -o
$ ls -sh core/vendor/composer/autoload_classmap.php
288K core/vendor/composer/autoload_classmap.php

The default APC max is 1M, correct? That discussion belongs over at #1818628: Use Composer's optimized ClassLoader for Core/Component classes, however.

it looks like your current patch on the Composer issue allows any class to be specified as long as it matches Symfony's classloaders - which unfortunately don't have an interface..

I've mentioned class-loader-adapter before, which provides that interface. There isn't really a reason for it though, since all the classes are just meant to have minimal interaction with your application. No real need for that interface, it just has the findFile() function and you're good to go.

Updated issue summary.

31: D8-classloader-swappable-2023325-31.patch queued for re-testing.

Adding some related issues. Don't have concrete thoughts/plans for this issue ATM, but will watch this issue. :-)

Autoloader is now generated by Composer, so in theory, the Symfony one can be swapped with another PSR-4 compatible one. Issue can be closed?

Quis custodiet ipsos custodes? If the autoloader implements an interface, what loads that interface?