Add an APCu classloader with a single entry

Let's try that out. This is a really naive implementation

Especially the boolean logic is broken :)

Alright, here is a more working implementation :P

Probably worth doing a get -> merge -> set here?

This is really a strong point, especially when it comes down to plugin/controller classes.

This just fixes the last failure.

+++ b/core/lib/Drupal/Component/ClassLoader/ApcClassLoader.php
@@ -0,0 +1,138 @@
+namespace Drupal\Component\ClassLoader;

Maybe better to move this to `Drupal\Core` namespace? otherwise this new component needs composer.json

Patch with fixed comments for #14 based on reroll for 8.7.x

+++ b/core/lib/Drupal/Component/ClassLoader/ApcClassLoader.php
@@ -0,0 +1,138 @@
+    if (!empty($this->classmap[$class])) {
...
+      $this->classmap[$class] = $this->decorated->findFile($class);
...
+    return $this->classmap[$class];

decorated could return FALSE or NULL when file not found so probably better to normalize it

For example \Composer\Autoload\ClassLoader::findFile() returns FALSE

This feels like a patch which ideally someone actually battle-tests in a live environment.

@dawehner yep, that's why I rerolled to get real numbers on project - at least it removes 8k calls to apcu_fetch() but I need to get the RAM effect

+++ b/core/lib/Drupal/Component/ClassLoader/ApcClassLoader.php
@@ -0,0 +1,144 @@
+      // A different request might have loaded different classes, so let's merge
+      // the data to improve efficiency.
+      if ($previous_classmap = apcu_fetch($this->prefix)) {
+        $class_map = array_merge($this->classmap, $previous_classmap);
+        // Using non-strict comparison because order of keys does not matter.

I am concerned about initialization. If the key is missing, how can this initialize without triggering warnings? On first call we will end with array_merge(array, false). The negative case should be handled with a sane default for $previous_classmap - an empty array.

Adding 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.

• Added composer.json and license to the component
• Expanded the documentation

All looks good so setting back to Needs Review! Tests failure is not related (see #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest).

Thanks for the clarification and sorry for the noise. I'm trying to digest these in context of wrapping my head around an option for preloading in #3108687: Compatibility with/take advantage of code preloading in PHP 7.4 so doing some cleanup on some older tickets as I go. I'll try to help with these since it would be great not to have a delay on 9.0.

Re-rolled for a small bit of fuzz against 9.0.x.

The failure is an assertion after module uninstallation of help_topics... the test was added in October, so after the last pass. I'll have to manually test to see if this is a fluke or related somehow.

+    if (!empty($this->classmap[$class])) {
+      return $this->classmap[$class];
+    }
+    elseif (!isset($this->classmap) && $data = apcu_fetch($this->prefix)) {
+      $this->classmap = $data;
+    }
+    else {
+      $this->classmap[$class] = $this->decorated->findFile($class);
+      $this->writeClassMap = TRUE;
+    }
+
+    return $this->classmap[$class];

The logic here doesn't look right - my understanding is that if the classmap is not already initialized, and the requested class does not already exist in the cached data, then it will skip the call to the decorated class

+      if ($previous_classmap = apcu_fetch($this->prefix)) {
+        $class_map = array_merge($this->classmap, $previous_classmap);
+        // Using non-strict comparison because order of keys does not matter.
+        if ($class_map != $previous_classmap) {
+          apcu_store($this->prefix, $this->classmap);
+        }
+      }

@ndobromirov raised concern that if the classmap wasn't previously cached this would raise a warning due to calling array_merge(array, false), but I think the conditional will prevent the data from ever being cached, since fetch will return FALSE.

Also, I think since the classmap is initialized to an empty array

  /**
   * A map between classname and filename containing this class.
   *
   * @var string[]
   */
  protected $classmap = [];

Then the isset check will always return TRUE

    elseif (!isset($this->classmap) && $data = apcu_fetch($this->prefix)) {
      $this->classmap = $data;
    }

and data will never be fetched from APCu.

Fixes for my previous comments, plus a test to check that a repeated path lookup uses cached data.

An important consideration is that if composer's caching is enabled, then it will cache each class lookup individually as well as this decorator's single cache entry.

- This decorator could be only enabled in DrupalKernel::initializeSettings() if composer's caching is not

  if (function_exists('apcu_fetch')) {
    if (method_exists($this->classLoader, 'getApcuPrefix') && $this->classLoader->getApcuPrefix() == NULL) {
      $loader = new ApcClassLoader($prefix, $this->classLoader);
    }
  }

- or Composer's caching could be disabled in favour of Drupal's

  if (function_exists('apcu_fetch')) {
    if (method_exists($this->classLoader, 'setApcuPrefix')) {
      $this->classLoader->setApcuPrefix(NULL);
    }
    $loader = new ApcClassLoader($prefix, $this->classLoader);
  }

I'm getting a little off-track but when I saw various xdebug profiles runs for a stock D9 site, the acpu thing jump at me and I've been thinking about the idea of bundling PHP classes that are dependent/related in a single file, like we'd bundle js code.

This would be more of a composer-level optimisation (like in the dump-autoload command). Had the idea seeing the symfony code where there are many class_exists all over the place to "optimize" loading, for example the HttpKernel.php has:

// Help opcache.preload discover always-needed symbols
class_exists(LegacyEventDispatcherProxy::class);
class_exists(ControllerArgumentsEvent::class);
class_exists(ControllerEvent::class);
class_exists(ExceptionEvent::class);
class_exists(FinishRequestEvent::class);
class_exists(RequestEvent::class);
class_exists(ResponseEvent::class);
class_exists(TerminateEvent::class);
class_exists(ViewEvent::class);
class_exists(KernelEvents::class);

A whole dependency graph would be needed but since it's done at composer time that might be acceptable in term of time/memory consumption. The file above would bundle everything only used by this class (using php multiple namespace feature):

namespace Symfony\Component\EventDispatcher {
  use Psr\EventDispatcher\StoppableEventInterface;
  use Symfony\Contracts\EventDispatcher\Event as ContractsEvent;
  use Symfony\Contracts\EventDispatcher\EventDispatcherInterface as ContractsEventDispatcherInterface;
  final class LegacyEventDispatcherProxy implements EventDispatcherInterface {}
}

namespace Symfony\Component\HttpKernel {
  // ...
  class HttpKernel implements HttpKernelInterface, TerminableInterface
  // ...
}

This should reduce the number of separate file needed (don't have a measure of how much it would help for now, didn't spend too much time on this). I mean if it looks good, we might not need to do much on the acpu front. There were a couple of strange things going on, I think the "use" are global within the file so if we use different class with the same name in different namespace it kinda breaks but it should be possible to find a solution.

Sounds like you're looking for preloading? See #3108687: Compatibility with/take advantage of code preloading in PHP 7.4

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

With #3020296: Remove Symfony's classloader as it does not exist in Symfony 4 done, is this issue still relevant?