(this issue is likely to be closed as won't fix, but I still want to post it for documentation reasons)

I am talking about
https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drup...
https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drup...
https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/_reg...

From the basic idea, this system is equivalent with Composer's classmap.
Just with some unnecessary bogus thrown into it.
Besides being awkward, we can expect this to have a performance impact. Maybe not severe, but measurable.

In this issue I simply want to summarize what is wrong with it, if only for reference.
We might close this as "won't fix", but at least then it is documented.
Or we could implement an "optional" solution, that has to be actively enabled in settings.php, so we don't break any existing sites.
Or we could at least find out why some of this stuff was done the way it was done.

The main problem seems to be that we unnecessarily distinguish between "class" and "interface".
Several other bad ideas have come as a consequence from that:

  1. We register two autoload functions with spl_autoload_register(): drupal_autoload_class(), and drupal_autoload_interface().
    Performance impact: For interfaces, the first function will execute without doing anything useful.
  2. Each of these callbacks calls another function, _registry_check_code().
    Performance impact: Unnecessary function call.
  3. class_exists() and interface_exists() in _registry_check_code(), before getting to the actual lookup.
    One could think this should result in an infinite loop, because the second parameter is not set to FALSE. But apparently, PHP has a built-in check to prevent recursion, if class_exists() is called from within an autoload callback. But we have to assume this is not for free.
    Performance impact: Unnecessary (native) function call + internal internal recursion check.
  4. A number of if/else in _registry_check_code(), to see if the static variable with the classmap is still up to date, and whether the function call was meant to reset the static variable cache.
    Performance impact: if/else is not free.

Some issues are actually caused by the procedural design of the system.

The static variable holding the classmap ("$lookup_cache") is encapsulated inside the _registry_check_code() function, so any contrib module that would attempt to replace this mechanic is forced to load the data from cache again.
Performance impact: Loading the same data twice from cache.

Another flaw is that the registry does not support namespaces - but this is sufficiently discussed elsewhere.
#1399974: Registry does not support namespaced classes

Comments

donquixote’s picture

donquixote CreditAttribution: donquixote commented

I did not profile any of this, and there are some factors that might compensate a bit of the above mentioned flaws.
- Composer registers an object method as the callback, which might be more costly to call.
- The Composer classmap is in an object property, which might have a different lookup cost than a static variable in a function.

All of these performance considerations might actually fall into "micro optimization". They only matter because autoloading is something that should happen a lot of times in a request. But even if there is nothing significant enough to gain, this does not change that the system is kinda awkward.