Quick summary:
Explore Krautoload as a solution for PSR-4 class loading.

About PSR-4 / PSR-X

The PHP-FIG group is discussing a new autoload standard, that will be an alternative to PSR-0 with
- more shallow directory structure.
- no more special handling of the underscore in the class name, and thus, no more backwards support for PEAR.

The working title is "PSR-X", the official title of the spec will probably be "PSR-4".
https://github.com/php-fig/fig-standards/blob/master/proposed/autoloader.md

It was suggested to adopt this for Drupal, even before the official PSR is released.
The main argument was a more DX-friendly directory structure, avoiding subfolder levels that are perceived as redundant.
#1971198: [policy] Drupal and PSR-0/PSR-4 Class Loading

Popular class loading solutions out there (Symfony, Composer) do not yet support the upcoming standard.
One obvious reason is that the standard is not released yet, and it is not entirely clear which name it will have.

The Krautoload library

Krautoload (github.com/donquixote/krautoload) has been suggested by donquixote as a possible solution for PSR-4 class loading in Drupal.

Some facts about the Krautoload library:
- It is a spin-off of the xautoload module, which currently has > 1000 happy users in D7, and rarely any issues.
- It copies or mimicks a lot of stuff from Symfony and Composer. E.g. it can be wired up with a class map and with PSR-0 prefixes, and it has cache decorators for APC, XCache, WinCache. This stuff is equivalent with Symfony/Composer, so we can expect that performance will be mostly identical.
- It has a unique lookup algorithm, with plugins registered for specific namespaces/directories, to distinguish between different patterns (PSR-4, PSR-0, PEAR, other). The lookup is O(1), no matter how many namespaces are registered, and how many different patterns are in use.
- It supports PSR-4.
- It is designed to compete in performance with other popular class loaders. Especially if you have many namespaces.
- It has micro-optimizations for different variations of PSR-0. E.g. if you already know you are not going to use underscores in the class name, Krautoload can use this information to optimize the lookup.
- It is fully aware of all the subtleties of PSR-0, that other loaders silently ignore. It is the user's choice whether to fully support those subtleties, or to take the same (tiny) risk as other loaders.
- It provides class discovery with different filesystem patterns (PSR-0, PSR-4).
- There are convenience methods to do typical composer stuff.

The only "caveat" would be that this is a brand-new thing which is still evolving, and has no significant known user base.
I personally don't see this as a big problem, but it means we apply do some precautions:
We should attempt to reduce the number of places with hard references to Krautoload. Esp, avoid that modules and contrib have to deal directly with Krautoload stuff.
E.g. by subclassing some of the classes and interfaces, so that modules can deal with Drupal-native equivalents.

Integration in Drupal

Changes to module directory structure

New PSR-4 file structure:
class: Drupal\$extension_name\Foo\Bar
old (PSR-0): $extension_dir/lib/Drupal/$extension_name/Foo/Bar.php
new (PSR-4): $extension_dir/src/Foo/Bar.php (src vs lib is still discussed)

Handling of underscores (not happening in Drupal, just for completeness)
class: Drupal\$extension_name\Foo\Bar_Baz
old (PSR-0): $extension_dir/lib/Drupal/$extension_name/Foo/Bar/Baz.php
new (PSR-4): $extension_dir/src/Foo/Bar_Baz.php (src vs lib is still being discussed)

Classes in "tests" subfolder:
class: Drupal\$extension_name\Tests\FooTest
old (PSR-0): $extension_dir/tests/Drupal/$extension_name/Tests/FooTest.php
new (PSR-4): $extension_dir/tests/src/FooTest.php (src vs lib is still being discussed)

Changes to namespace registration

Krautoload provides a rich adapter interface to register "stuff" to the class loader.
Drupal extends this, to add methods that are specific to Drupal.

<?php
// Bootstrap:
$adapter = Krautoload::start($options);
// Register composer classmap and namespaces found in the vendor/composer directory.
// (this includes Drupal core)
$adapter->composerVendorDir(DRUPAL_ROOT . '/core/vendor');
// Register a Drupal extension (currently PSR-0 *and* PSR-4).
$adapter->addDrupalExtension('system', 'core/modules/system');
// Register a Drupal extension 'tests' folder.
$adapter->addDrupalExtensionTests('system', 'core/modules/system');
?>

Changes to AnnotatedClassDiscovery and plugin managers

See #2033611: Port AnnotatedClassDiscovery to PSR-4, and make it agnostic of directory information.
This is important to review!!!

Changes to simpletest discovery

<?php
simpletest_classloader_register
();
[..]
$namespaces = array('Drupal\system\Tests', 'Drupal\views\Tests', ...);
$searchable_namespaces = drupal_classloader()->buildSearchableNamespaces($namespaces);
foreach (
$searchable_namespaces->discoverExistingClasses(TRUE) as $class) {
  ..
}
?>

Plan of execution

We will go smoothly in different steps:

  1. Implement support PSR-4 alongside PSR-0, without actually moving any classes.
    Have a script that can move all classes around to their new location, so people can give it a try.
    This is already working with current patches! (e.g. #32)
  2. Move module classes around to their new location:
    This is as easy as executing that script.
    Give D8 contrib a chance to do the same.
  3. Remove remaining PSR-0 stuff, clean up.
    This should not be too hard either.

Status

Both class loading and plugin discovery have been proven to work fine.
The library on github has mostly stabilized, and semver will protect us from breaking API changes.

Alternatives

It was suggested to use a hand-crafted PSR-4 loader, and do all the rest with Composer's class loader.
The composer loader would have a compiled class map for core classes.

However,
- Krautoload can handle compiled class maps just as good as Composer, and it plays nicely with the composer vendor directory.
- Krautoload can have a cache layer on top of everything, instead of "just" for modules.
- By having two class loaders on the stack, we totally eat up any gain from micro-optimization. Having two loaders means that for one half (or whatever percent) of classes we fire a loader that does nothing.

Besides, Composer does not help us with class discovery.

If this alternative is still being considered, it should be explored in a separate issue!

Performance impact of multiple class loaders on the spl stack

E.g. if the composer loader is first in the chain, only for core and vendor, and autoload fires for a module-provided class, then the following will happen:
- Composer will look in the class map and find nothing.
- Composer will look into registered prefixes, which is exactly what we want to avoid. And again, finds nothing.
- Krautoload will find the class (e.g. in APC cache).

On the other hand, if Krautoload is first, and autoload fires with a core- or vendor-provided class, then
- Krautoload will check the APC cache, and find nothing.
- Krautoload will check the registered prefixes, which is exactly what we want to avoid.
- Composer will do the class loading (e.g. via class map)

Files: 
CommentFileSizeAuthor
#32 D8-krautoload-2033501-32.patch290.46 KBdonquixote
PASSED: [[SimpleTest]]: [MySQL] 57,175 pass(es).
[ View ]
#28 D8-krautoload-psrx-2033501-28.patch266.32 KBdonquixote
PASSED: [[SimpleTest]]: [MySQL] 57,201 pass(es).
[ View ]
#24 D8-krautoload-psrx-2033501-24.patch265.68 KBdonquixote
FAILED: [[SimpleTest]]: [MySQL] 57,323 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#23 D8-krautoload-psrx-2033501-23.patch265.58 KBdonquixote
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch D8-krautoload-psrx-2033501-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#22 D8-krautoload-psrx-2033501-22.patch265.58 KBdonquixote
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch D8-krautoload-psrx-2033501-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#21 D8-krautoload-psrx-2033501-21.patch193.08 KBdonquixote
PASSED: [[SimpleTest]]: [MySQL] 56,768 pass(es).
[ View ]
#20 D8-krautoload-psrx-2033501-20.patch217.5 KBdonquixote
PASSED: [[SimpleTest]]: [MySQL] 56,515 pass(es).
[ View ]
#11 D8-krautoload-psrx-2033501-11.patch171.05 KBdonquixote
PASSED: [[SimpleTest]]: [MySQL] 56,830 pass(es).
[ View ]
#8 D8-krautoload-psrx-2033501-8.patch167.6 KBdonquixote
FAILED: [[SimpleTest]]: [MySQL] 56,840 pass(es), 4 fail(s), and 2 exception(s).
[ View ]
#4 D8-krautoload-psrx-2033501-4.patch165.46 KBdonquixote
FAILED: [[SimpleTest]]: [MySQL] 56,490 pass(es), 14 fail(s), and 3 exception(s).
[ View ]

Comments

donquixote’s picture

Issue summary:View changes

mention tiny overhead of multiple loaders

donquixote’s picture

Issue summary:View changes

mention benchmarks

donquixote’s picture

Issue summary:View changes

mention the alternative that was suggested.

quicksketch’s picture

Thanks @donquixote for making this new issue. My first look over the code, some things appeared to me:

- We don't need the PEAR implementations at all in Drupal core. Is it necessary or desirable to keep that functionality?
- Overall, this seems highly abstracted, perhaps more than necessary. That's probably just fine if the end-result is a faster class loader, but I get the feeling that this is more flexibility than we require for Drupal core. If we can optimize even further by removing abstraction, I'm all for that.
- Let's use {@inheritdoc} in all the places where documentation is copy/pasted from the interfaces.

This sort of thing isn't really an area I'm familiar with, so I'm glad to see you taking it on.

donquixote’s picture

Hi!

We don't need the PEAR implementations at all in Drupal core. Is it necessary or desirable to keep that functionality?

I think for an "external" library that lives in vendor/, it is ok to have some stuff that won't be used in core.
The essential thing is that we don't load/include stuff from that library that we don't need.

Overall, this seems highly abstracted, perhaps more than necessary. That's probably just fine if the end-result is a faster class loader, but I get the feeling that this is more flexibility than we require for Drupal core. If we can optimize even further by removing abstraction, I'm all for that.

I am already killing one level of abstraction for regular loadClass() operations :)
I am going to comment on that when I have a new patch..

Let's use {@inheritdoc} in all the places where documentation is copy/pasted from the interfaces.

Yes!
Documentation needs to be improved throughout..

donquixote’s picture

StatusFileSize
new165.46 KB
FAILED: [[SimpleTest]]: [MySQL] 56,490 pass(es), 14 fail(s), and 3 exception(s).
[ View ]

A big and bad-ass patch.
What does it do?

  1. It introduces the krautoload library (or a very modified version, that will soon go upstream).
    core/vendor/krautoload
    This brings autoloading for PSR-0 and PSR-4, and class discovery for both.
  2. It uses Krautoload for autoloading.
    For each module, we register lib/ as PSR-0, and src/ as PSR-4.
    (we may change this)
  3. It uses Krautoload for AnnotatedClassDiscovery.
  4. It uses Krautoload for simpletest class discovery.
  5. It replaces the container.namespaces ArrayObject service with a "SearchableNamespaces" object.
  6. (EDIT:) It replaces the class_loader service with a Krautoload\\RegistrationHub, which is a wrapper around the class loader and provides a bunch of namespace registration methods. (this is to not bloat the classloader itself with those methods)
  7. (EDIT:) It introduces a new test that would break the existing core classloader, but does not break krautoload. (these are edge cases, so you might not care)

EDIT: Caveat: Comments are horrible.

donquixote’s picture

If you don't feel like studying the patch, here is a snapshot of the class discovery in action in simpletest.

<?php
      $namespaces
= array();
      foreach (
$all_data as $name => $data) {
       
$namespaces[] = "Drupal\\$name\\Tests";
      }

     
// Build a searchable namespace collection.
     
$namespaces = drupal_classloader()->buildSearchableNamespaces($namespaces);

     
// Check that each class has a getInfo() method and store the information
      // in an array keyed with the group specified in the test information.
     
$groups = array();
      foreach (
$namespaces->discoverExistingClasses(TRUE) as $class) {
       
// Test classes need to implement getInfo() to be valid.
       
if (method_exists($class, 'getInfo')) {
          [..]
?>

The goal is to encapsulate the details of the filesystem mappings with PSR-0 or PSR-4, so that the developer can focus on something else.

donquixote’s picture

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, D8-krautoload-psrx-2033501-4.patch, failed testing.

donquixote’s picture

StatusFileSize
new167.6 KB
FAILED: [[SimpleTest]]: [MySQL] 56,840 pass(es), 4 fail(s), and 2 exception(s).
[ View ]

I had to change how the searchable namespaces are registered as a service.
They need to be marked as persistent, just like the ArrayObject.
Then on rebuild we need to update both the namespace names, AND the internal finder object.

donquixote’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, D8-krautoload-psrx-2033501-8.patch, failed testing.

donquixote’s picture

Status:Needs work» Needs review
StatusFileSize
new171.05 KB
PASSED: [[SimpleTest]]: [MySQL] 56,830 pass(es).
[ View ]

Tests adapted for API changes.

donquixote’s picture

Issue summary:View changes

Alternative should be explored in separate issue!

donquixote’s picture

Green, yeah!

Left to do:
- Fine-tune class and method names, and signatures.
- Fix comments. (they are horrible atm)
- Flesh out cache layers, and enable them in Drupal.
- (EDIT:) Write tests in Krautoload.
- Evaluate, benchmark.

And finally:
- Move all class files to the new location according to PSR-4 (src or lib).

donquixote’s picture

We have this a lot in plugin managers:
(example: FieldTypePluginManager)

<?php
-  public function __construct(\Traversable $namespaces, CacheBackendInterface $cache_backend, LanguageManager $language_manager, ModuleHandlerInterface $module_handler) {
+  public function
__construct(SearchableNamespacesInterface $namespaces, CacheBackendInterface $cache_backend, LanguageManager $language_manager, ModuleHandlerInterface $module_handler) {
    
$annotation_namespaces = array(
-     
'Drupal\Core\Entity\Annotation' => DRUPAL_ROOT . '/core/lib',
+     
'Drupal\Core\Entity\Annotation' => TRUE,
     );
    
parent::__construct('field/field_type', $namespaces, $annotation_namespaces, 'Drupal\Core\Entity\Annotation\FieldType');
?>

Maybe this would be nicer for annotation namespaces:

<?php
 
public function __construct(SearchableNamespacesInterface $namespaces, CacheBackendInterface $cache_backend, LanguageManager $language_manager, ModuleHandlerInterface $module_handler) {
   
parent::__construct('field/field_type', $namespaces, 'Drupal\Core\Entity\Annotation\FieldType');
   
$this->addAnnotationNamespace('Drupal\Core\Entity\Annotation');
?>
RobLoach’s picture

+++ b/core/modules/system/lib/Drupal/system/Tests/Plugin/Discovery/AnnotatedClassDiscoveryTest.phpundefined
@@ -57,7 +57,18 @@ public function setUp() {
+
+    // Build namespace finder.
+    $finder = new \Krautoload\NamespaceVisitor_Pluggable();
+    $registrationHub = new \Krautoload\RegistrationHub($finder);
+    $registrationHub->addNamespacePSR0('Drupal\plugin_test', DRUPAL_ROOT . '/core/modules/system/tests/modules/plugin_test/lib');
+    $registrationHub->addNamespacePSR0('Drupal\Component', DRUPAL_ROOT . '/core/lib');
+    $registrationHub->addNamespacePSR0('Drupal\Core', DRUPAL_ROOT . '/core/lib');

Looks like you're defining hard-bindings here? I understand that AnnotatedClassDiscovery is a bit hairy at the moment. Do you know if we have an issue around to help clean it up a bit? This is part of the test suite though, so many we don't need to clean it up.

+++ b/core/vendor/krautoload/README.md
@@ -0,0 +1,88 @@
+Krautoload is a pluggable PHP class autoloader library that makes you fantasize of Kartoffelbrei, Kasseler and Sauerkraut.  ¶
+It has native support for
+- [PSR-0](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md)
+- Variation of PSR-0 which allows shallow directory structures.

Since this is being added in core/vendor, we'd likely want to have Composer handle the third-party dependency. I put together an issue for that in the krautoload issue queue.

Move all class files to the new location according to PSR-4 (src or lib)

Do you know if we have an issue to move from lib to src? I remember it being a long-winded discussion, and it really doesn't matter which one we go with, but it would be good to track that discussion.

donquixote’s picture

Since this is being added in core/vendor, we'd likely want to have Composer handle the third-party dependency. I put together an issue for that in the krautoload issue queue.

I agree with this idea. The question is the timing.
I think it has been useful so far that Krautoload is not a finalized library yet, and we can develop it alongside Drupal.

So, the timing should be:
1. Bikeshed the Krautoload API (class names, method names, signatures)
2. Push upstream to github, and add a composer.json
3. Get it into Drupal in the regular composer fashion, and commit the patch.

donquixote’s picture

Do you know if we have an issue to move from lib to src? I remember it being a long-winded discussion, and it really doesn't matter which one we go with, but it would be good to track that discussion.

#1400748-30: Proposal for unified namespace organization and following posts.

I think it is fair to say that many of the arguments in favour of lib apply to PSR-0 and PSR-4 likewise.
What may have changed is how other projects use either lib or src, so we could do some statistics.

Either way, I would absolutely suggest to keep this discussion out of this issue.
The meta issue might not benefit from this discussion either, but it is probably still a better place than here.
#1971198: [policy] Drupal and PSR-0/PSR-4 Class Loading

And again, it's a timing thing:
1. Make it work with src/, or whatever is easier to work with during development.
(we might not even commit this)
2. Switch back to lib/ (if decided somewhere), and move the module classes.

Btw, as a temporary thing we could also map the module namespace to (module dir)/lib/Drupal/(module) with PSR-4, and totally forget about src/. Then the decision is postponed until we actually move the modules.

Having both lib/ and src/ can make the transition phase more smooth.
But we could even work with an overlapping mapping. Krautoload has plugins for PSR-0 that are safe to use with mapping ambiguity. (and it will be easy to add the same thing for PSR-4)

donquixote’s picture

#14 (RobLoach):

Looks like you're defining hard-bindings here? I understand that AnnotatedClassDiscovery is a bit hairy at the moment. Do you know if we have an issue around to help clean it up a bit? This is part of the test suite though, so many we don't need to clean it up.

What do you mean with "hard bindings" in this context?

I understand that AnnotatedClassDiscovery is a bit hairy at the moment. Do you know if we have an issue around to help clean it up a bit?

So far I only know of #2033611: Port AnnotatedClassDiscovery to PSR-4, and make it agnostic of directory information..

There is a connection with PSR-4:
In the original AnnotatedClassDiscovery and plugin managers, we are passing around arrays where the keys are namespaces, and the values are filesystem locations associated with those namespaces.
With PSR-4, those arrays would get a different meaning. And, creating respective arrays for sub-namespaces would be different, because we would also have to change the mapped directories to the respective subfolders.
It became even uglier when I wanted to support both PSR-0 and PSR-4 in parallel.

I think encapsulating those into SearchableNamespaces (or name it SearchableNamespaceCollection) is a good idea.
This way, AnnotatedClassDiscovery no longer needs to care about directories, only about namespaces.

SearchableNamespaces needs
- one NamespaceVisitor (or name it NamespaceFinder -> bikeshed!), which is a more powerful version of the ClassLoader, and can handle the filesystem mappings during discovery.
- a simple array of namespace names, without any filesystem information.

During a test, for the NamespaceVisitor, we can either use the one from the container, which should already have all module namespace mappings. Or we create our own, and register only the namespace mappings we need. This is what the patch does in AnnotatedClassDiscoveryTest.

Pancho’s picture

Issue tags:+autoload, +PSR-0, +PSR-4, +classloader

Re #14:

Do you know if we have an issue to move from lib to src? I remember it being a long-winded discussion, and it really doesn't matter which one we go with, but it would be good to track that discussion.

In #1971198-151: [policy] Drupal and PSR-0/PSR-4 Class Loading I tried to distill the arguments already discussed in #1400748: Proposal for unified namespace organization.
I think that #1971198: [policy] Drupal and PSR-0/PSR-4 Class Loading would be a good place to finalize naming conventions and more general specifications, while keeping this for the technical implementation.

RobLoach’s picture

I'd love to keep this as simple as possible, and decouple as much as we can so that we don't run into a spaghetti mess.

I think it has been useful so far that Krautoload is not a finalized library yet, and we can develop it alongside Drupal.

I'd actually recommend focusing on making the Krautoload project flawless in your eyes on GitHub first, before bringing it in. It would mean reaching a wider audience, and allow independent testing decoupled from Drupal.

If you're concerned about timing, just follow Semantic Versioning and put out a 0.0.1-alpha.1 and other alpha/beta releases, people would understand that the API is subject to change, but would know that a future is there for the project. That would also allow Drupal to target specific versions based on our needs.

What do you mean with "hard bindings" in this context?

Oh, I meant hard bindings to the class loader. You're interacting directly with the loader instead of depending on the already registered namespaces.

Make it work with src/, or whatever is easier to work with during development.

src vs lib is a completely separate discussion. Not quite sure why that would have to happen here.

Naming of any kind does not matter. As long as we can target a specific version (composer.json and semver), then we can upgrade at our choosing.

donquixote’s picture

StatusFileSize
new217.5 KB
PASSED: [[SimpleTest]]: [MySQL] 56,515 pass(es).
[ View ]

#19:
Krautoload on github is on the way, with composer.json and PHPUnit tests.

Meanwhile, I am trying to refit the plugin manager constructors.

EDIT: Forget this patch!! It is agains the wrong version..

donquixote’s picture

StatusFileSize
new193.08 KB
PASSED: [[SimpleTest]]: [MySQL] 56,768 pass(es).
[ View ]

Better..

donquixote’s picture

StatusFileSize
new265.58 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch D8-krautoload-psrx-2033501-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

New stuff:

  1. Krautoload has evolved. Some of the changes:
    • Refactoring, improved comments, bug fixes, PHPUnit tests added, etc.
    • Full support for class maps generated by Composer:
      ->composerVendorDir(DRUPAL_ROOT . '/core/vendor') simply picks up whatever it finds in the vendor/composer directory.
      This means, you can run composer dump-autoload --optimize and it will have the same effect as if you had the original Composer class loader in place.
    • Cache layers for APC, XCache, WinCache, equivalent to those in Symfony (in fact the logic is mostly copied, but the architecture is different).
    • Bootstrap helper Krautoload::start() now receives an $options array to activate the cache (among other things).
  2. Krautoload is now downloaded via composer, to core/vendor/donquixote/krautoload.
    (See Project name: "krautoload" vs "donquixote/krautoload" vs "krautoload/krautoload". on github)
  3. APC cache activated depending on a setting.
  4. Use php core/scripts/switch-psr4.sh to automatically move all module class files to their new location. (currently that is src/)
    It turns out you can run this without even having to flush caches! Isn't that awesome :)

Important stuff from previous patches:

About the patch:
composer update / composer install did result in a huge changeset, with stuff unrelated to this issue.
This is because some libraries have a new version.
For this patch, I did revert all of those changes except for core/vendor/donquixote/krautoload, to make it easier to review.

donquixote’s picture

StatusFileSize
new265.58 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch D8-krautoload-psrx-2033501-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

It seems for every valid patch I have to upload at least 1x garbage..

donquixote’s picture

StatusFileSize
new265.68 KB
FAILED: [[SimpleTest]]: [MySQL] 57,323 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
donquixote’s picture

We could have a tool that generates a class map for everything: Core and enabled extensions (modules/themes).
This class map would live somewhere in the generated code directory (e.g. sites/default/files/php).

By definition, this file cannot be generated by composer, because composer does not know about enabled modules.
It will need a Drupal bootstrap to know about modules, so it can't be a simple standalone script.
It has to consider all namespaces that are registered in every request, but it has to ignore namespaces that are just temporarily registered - e.g. for simpletest.

Since Krautoload already has some discovery capabilities, it wouldn't be too hard to add one for generating class maps.
The big question is, where to call this within Drupal.

donquixote’s picture

Issue summary:View changes

Mention that it can work with a class map compiled by composer.

donquixote’s picture

I updated the issue summary and the "arguments" section, I hope this is not too opinionated.

Alternatives

It was suggested to use a hand-crafted PSR-4 loader, and do all the rest with Composer's class loader.
The composer loader would have a compiled class map for core classes.

However,
- Krautoload can handle compiled class maps just as good as Composer, and it plays nicely with the composer vendor directory.
- Krautoload can have a cache layer on top of everything, instead of "just" for modules.
- By having two class loaders on the stack, we totally eat up any gain from micro-optimization. Having two loaders means that for one half (or whatever percent) of classes we fire a loader that does nothing.

E.g. if the composer loader is first in the chain, only for core and vendor, and autoload fires for a module-provided class, then the following will happen:
- Composer will look in the class map and find nothing.
- Composer will look into registered prefixes, which is exactly what we want to avoid. And again, finds nothing.
- Krautoload will find the class (e.g. in APC cache).

On the other hand, if Krautoload is first, and autoload fires with a core- or vendor-provided class, then
- Krautoload will check the APC cache, and find nothing.
- Krautoload will check the registered prefixes, which is exactly what we want to avoid. And again, finds nothing.
- Composer will do the class loading (e.g. via class map)

Besides, Composer does not help us with class discovery.

If this alternative is still being considered, it should be explored in a separate issue!

donquixote’s picture

Issue summary:View changes

Updated issue summary: Krautoload has evolved. Removed "performance" section, this is sufficiently covered elsewhere. Add more thoughts to the "alternative".

Status:Needs review» Needs work

The last submitted patch, D8-krautoload-psrx-2033501-24.patch, failed testing.

donquixote’s picture

Status:Needs work» Needs review
StatusFileSize
new266.32 KB
PASSED: [[SimpleTest]]: [MySQL] 57,201 pass(es).
[ View ]

EDIT: Yeah, pass!
For patch notes, see #22.

donquixote’s picture

Left to do:

  1. Reduce hard references to the Krautoload library, and replace them with Drupal-native classes in most places.
    This makes us less vulnerable to changes in Krautoload (it is still "alpha").
  2. (Decide on the signature for plugin managers (see #22).)

The plan:

  1. Drupal-native SearchableNamespaces(+Interface) extending the one from Krautoload, so plugin managers don't need to hard-reference Krautoload.
  2. Drupal-native ClassLoaderAdapter(+Interface) and NamespaceInspectorAdapter(+Interface) extending the RegistrationHub from Krautoload, and adding some Drupal-specific methods.
    Please give some feedback here on github: Find a better name for RegistrationHub, and add an interface.
    Btw, for my taste this thing may also implement the class-loader-adapter thingie by RobLoach.

I played around locally, and got this to work:

<?php
// Adapter for a simple class loader.
$adapter = \Drupal\Core\ClassLoader\ClassLoaderAdapter::start();
// Adapter for a namespace inspector.
$adapter->addDrupalExtension('system', 'core/modules/system');
// Register Drupal-specific stuff.
$adapter->addDrupalExtensionsByFileName($extension_filenames);
$adapter->addDrupalExtensionTests('system', 'core/modules/system');
$adapter->addDrupalCore();
$adapter->addDrupalCoreTests();
// Same as Krautoload, but returns an instance of \Drupal\Core\ClassLoader\SearchableNamespacesInterface.
$searchable_namespaces = $adapter->buildSearchableNamespaces(array('Drupal\system\Plugin\xyz'));
?>

The cool thing: There will be only one class to change, to switch from PSR-0 to PSR-4 for modules.

donquixote’s picture

Issue summary:View changes

Updated issue summary.

Pancho’s picture

Nice. There seems to be quite much progress...
Just tell us when we should really start reviewing concept and details!

donquixote’s picture

@Pancho: I need some feedback over at
#2033611: Port AnnotatedClassDiscovery to PSR-4, and make it agnostic of directory information.
This is the part that is the most intrusive to existing code, and I can't do these decisions alone.

And of course, I would be happy if people could finally settle on the src vs lib.
Maybe organize a vote over in the meta issue?

donquixote’s picture

StatusFileSize
new290.46 KB
PASSED: [[SimpleTest]]: [MySQL] 57,175 pass(es).
[ View ]

More stuff!

  • Classes in tests folders move to PSR-4:
    tests/Drupal/$extension/Tests -> tests/src
  • Update Krautoload to 0.0.1-alpha7.
  • Introduce a Drupal-native ClassLoaderAdapterInterface (and NamespaceInspectorAdapter+Interface) extending their respective Krautoload counterparts, adding Drupal-specific registration methods.
  • Introduce a Drupal-native SearchableNamespacesInterface, to reduce hard references to Krautoload all over the place.
  • Only one file is left that decides whether module namespaces are registered as PSR-0 and PSR-4. So the aftermath will be super easy.

I was going to upload a second patch which actually has all the class files being moved around.
Unfortunately, the size of that is 18.6 MB, so beyond the upload limit. I might find a different solution, e.g. a pull request?

@Pancho: Now is the time for reviewing and feedback!

donquixote’s picture

@Pancho, others:
Anyone who wants to help testing this, please try
php core/scripts/switch-psr4.sh
This should go even without a cache refresh.
(I'm not sure about simpletest, maybe a flush will be useful)

donquixote’s picture

Here we have two branches to explore:
PSR-4 enabled:
https://github.com/donquixote/drupal/tree/8.x-krautoload/core/modules/sy...
Files moved to new location:
https://github.com/donquixote/drupal/tree/8.x-krautoload-moved/core/modu...

Links go directly to system module, which is a nice example.
Have a look at src/ and tests/src/.

EDIT: Attention, the git history is messy! We don't want to pull these branches with all their history.

tstoeckler’s picture

Wow, I like the file structure there. I also like the moving the test classes to /tests/src. Keeps all the test-related stuff in one place. Don't really care about src/ vs. lib/

tstoeckler’s picture

Issue summary:View changes

No longer point to the drupal-vendor branch, which is now outdated.

donquixote’s picture

Issue summary:View changes

Describe how Krautoload will be embedded in Drupal.

donquixote’s picture

There are two problems with this:

  • The algorithm and plugin system in Krautoload is not the best we can get, performance-wise. It is faster than Composer if you have many modules, but we can do much better, if we are already writing our own thing.
  • ClassLoader and NamespaceInspector being the same object may be convenient, but this does make it more difficult to replace the loader with something custom via settings.php, because now if you want to replace the loader you also need to replace the NamespaceInspector.

I am proposing something over at
#2023325-27: Add interface for classloader so it can be cleanly swapped
that should solve these problems. (but so far does nothing about plugin discovery)

The loader might live in Drupal core, or it might be outsourced to github (e.g. as Krautoload v2). The latter would have the benefit that it is easier for others to reuse.

All this being said: I would like to keep a lot of the architectural stuff - e.g. the concept of SearchableNamespaces, NamespaceInspector, etc. So, reviews are still appreciated, especially if they focus on this stuff.

eigentor’s picture

I don't understand no nothing of the technical stuff, but I love the name!

eigentor’s picture

Issue summary:View changes

Updated issue summary.

sun’s picture

Issue summary:View changes
Status:Needs review» Closed (duplicate)

Unless I'm mistaken, this appears to be obsolete and duplicated by #2083547: PSR-4: Putting it all together now?

donquixote’s picture

Yes, this can be closed.