merged branch fabpot/classloader-optim (PR symfony#4729)

Commits ------- 3f9e8ff [ClassLoader] made ClassCollectionLoader::load() automatically include class dependencies 6f4d281 [ClassLoader] added missing support for PHP 5.4 traits Discussion ---------- Classloader optimization The first commit fixes support for PHP 5.4 trait. The second one does several things: * it optimizes the recent merge so that the reflection class instance is only loaded once; * we use the fact that we now get all class dependencies to automatically add all class dependencies to the map. --------------------------------------------------------------------------- by fabpot at 2012-07-03T17:26:46Z I've updated to take into accounts traits. --------------------------------------------------------------------------- by bamarni at 2012-07-04T11:58:57Z great job :+1: I can't see it in the diff as this part hasn't changed, but somewhere in the autoReload block there is : ``` if ($meta[1] != $classes) { $reload = true; } ``` It should be array_unique($classes), otherwise the file would be perpetually regenerated in autoReload mode when the input contains duplicate, because they're implicitely removed when dumping the files. --------------------------------------------------------------------------- by fabpot at 2012-07-04T13:20:04Z @bamarni I've added an `array_unique` call at the top (this bug existed before by the way).

Author: fabpot

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -116,17 +116,12 @@ public function load(array $configs, ContainerBuilder $container)
 
  'Symfony\\Component\\Config\\FileLocator',
 
- 'Symfony\\Component\\EventDispatcher\\EventDispatcherInterface',
- 'Symfony\\Component\\EventDispatcher\\EventDispatcher',
  'Symfony\\Component\\EventDispatcher\\Event',
- 'Symfony\\Component\\EventDispatcher\\EventSubscriberInterface',
  'Symfony\\Component\\EventDispatcher\\ContainerAwareEventDispatcher',
 
- 'Symfony\\Component\\HttpKernel\\HttpKernel',
  'Symfony\\Component\\HttpKernel\\EventListener\\ResponseListener',
  'Symfony\\Component\\HttpKernel\\EventListener\\RouterListener',
  'Symfony\\Component\\HttpKernel\\Controller\\ControllerResolver',
- 'Symfony\\Component\\HttpKernel\\Controller\\ControllerResolverInterface',
  'Symfony\\Component\\HttpKernel\\Event\\KernelEvent',
  'Symfony\\Component\\HttpKernel\\Event\\FilterControllerEvent',
  'Symfony\\Component\\HttpKernel\\Event\\FilterResponseEvent',
@@ -140,7 +135,6 @@ public function load(array $configs, ContainerBuilder $container)
  'Symfony\\Bundle\\FrameworkBundle\\Controller\\ControllerResolver',
  // Cannot be included because annotations will parse the big compiled class file
  // 'Symfony\\Bundle\\FrameworkBundle\\Controller\\Controller',
- 'Symfony\\Bundle\\FrameworkBundle\\HttpKernel',
  ));
  }
 
@@ -263,13 +257,8 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
  $container->setParameter('request_listener.https_port', $config['https_port']);
 
  $this->addClassesToCompile(array(
- 'Symfony\\Component\\Routing\\Matcher\\UrlMatcherInterface',
- 'Symfony\\Component\\Routing\\Generator\\UrlGeneratorInterface',
- 'Symfony\\Component\\Routing\\RouterInterface',
  'Symfony\\Component\\Routing\\Matcher\\UrlMatcher',
  'Symfony\\Component\\Routing\\Generator\\UrlGenerator',
- 'Symfony\\Component\\Routing\\Matcher\\RedirectableUrlMatcherInterface',
- 'Symfony\\Component\\Routing\\RequestContextAwareInterface',
  'Symfony\\Component\\Routing\\RequestContext',
  'Symfony\\Component\\Routing\\Router',
  'Symfony\\Bundle\\FrameworkBundle\\Routing\\RedirectableUrlMatcher',
@@ -313,7 +302,6 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
 
  $this->addClassesToCompile(array(
  'Symfony\\Bundle\\FrameworkBundle\\EventListener\\SessionListener',
- 'Symfony\\Component\\HttpFoundation\\Session\\Storage\\SessionStorageInterface',
  'Symfony\\Component\\HttpFoundation\\Session\\Storage\\NativeSessionStorage',
  'Symfony\\Component\\HttpFoundation\\Session\\Storage\\Handler\\FileSessionHandler',
  'Symfony\\Component\\HttpFoundation\\Session\\Storage\\Proxy\\AbstractProxy',
@@ -405,24 +393,13 @@ function($v, Reference $ref) use ($container) {
 
  $this->addClassesToCompile(array(
  'Symfony\\Bundle\\FrameworkBundle\\Templating\\GlobalVariables',
- 'Symfony\\Bundle\\FrameworkBundle\\Templating\\EngineInterface',
- 'Symfony\\Component\\Templating\\StreamingEngineInterface',
- 'Symfony\\Component\\Templating\\TemplateNameParserInterface',
- 'Symfony\\Component\\Templating\\TemplateNameParser',
- 'Symfony\\Component\\Templating\\EngineInterface',
- 'Symfony\\Component\\Config\\FileLocatorInterface',
- 'Symfony\\Component\\Templating\\TemplateReferenceInterface',
- 'Symfony\\Component\\Templating\\TemplateReference',
  'Symfony\\Bundle\\FrameworkBundle\\Templating\\TemplateReference',
  'Symfony\\Bundle\\FrameworkBundle\\Templating\\TemplateNameParser',
  $container->findDefinition('templating.locator')->getClass(),
  ));
 
  if (in_array('php', $config['engines'], true)) {
  $this->addClassesToCompile(array(
- 'Symfony\\Component\\Templating\\PhpEngine',
- 'Symfony\\Component\\Templating\\Loader\\LoaderInterface',
- 'Symfony\\Component\\Templating\\Storage\\Storage',
  'Symfony\\Component\\Templating\\Storage\\FileStorage',
  'Symfony\\Bundle\\FrameworkBundle\\Templating\\PhpEngine',
  'Symfony\\Bundle\\FrameworkBundle\\Templating\\Loader\\FilesystemLoader',

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

@@ -91,21 +91,14 @@ public function load(array $configs, ContainerBuilder $container)
  // add some required classes for compilation
  $this->addClassesToCompile(array(
  'Symfony\\Component\\Security\\Http\\Firewall',
- 'Symfony\\Component\\Security\\Http\\FirewallMapInterface',
  'Symfony\\Component\\Security\\Core\\SecurityContext',
- 'Symfony\\Component\\Security\\Core\\SecurityContextInterface',
  'Symfony\\Component\\Security\\Core\\User\\UserProviderInterface',
  'Symfony\\Component\\Security\\Core\\Authentication\\AuthenticationProviderManager',
- 'Symfony\\Component\\Security\\Core\\Authentication\\AuthenticationManagerInterface',
  'Symfony\\Component\\Security\\Core\\Authorization\\AccessDecisionManager',
- 'Symfony\\Component\\Security\\Core\\Authorization\\AccessDecisionManagerInterface',
  'Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface',
-
  'Symfony\\Bundle\\SecurityBundle\\Security\\FirewallMap',
  'Symfony\\Bundle\\SecurityBundle\\Security\\FirewallContext',
-
  'Symfony\\Component\\HttpFoundation\\RequestMatcher',
- 'Symfony\\Component\\HttpFoundation\\RequestMatcherInterface',
  ));
  }
 
@@ -181,7 +174,6 @@ private function createAuthorization($config, ContainerBuilder $container)
  }
 
  $this->addClassesToCompile(array(
- 'Symfony\\Component\\Security\\Http\\AccessMapInterface',
  'Symfony\\Component\\Security\\Http\\AccessMap',
  ));
 

src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php

@@ -92,14 +92,12 @@ public function load(array $configs, ContainerBuilder $container)
 
  $this->addClassesToCompile(array(
  'Twig_Environment',
- 'Twig_ExtensionInterface',
  'Twig_Extension',
  'Twig_Extension_Core',
  'Twig_Extension_Escaper',
  'Twig_Extension_Optimizer',
  'Twig_LoaderInterface',
  'Twig_Markup',
- 'Twig_TemplateInterface',
  'Twig_Template',
  ));
  }

src/Symfony/Component/ClassLoader/ClassCollectionLoader.php

@@ -19,7 +19,7 @@
 class ClassCollectionLoader
 {
  static private $loaded;
- static private $baseClassesCountMap;
+ static private $seen;
 
  /**
  * Loads a list of classes and caches them in one big file.
@@ -42,14 +42,21 @@ static public function load($classes, $cacheDir, $name, $autoReload, $adaptive =
 
  self::$loaded[$name] = true;
 
+ $declared = array_merge(get_declared_classes(), get_declared_interfaces());
+ if (function_exists('get_declared_traits')) {
+ $declared = array_merge($declared, get_declared_traits());
+ }
+
  if ($adaptive) {
  // don't include already declared classes
- $classes = array_diff($classes, get_declared_classes(), get_declared_interfaces());
+ $classes = array_diff($classes, $declared);
 
  // the cache is different depending on which classes are already declared
  $name = $name.'-'.substr(md5(implode('|', $classes)), 0, 5);
  }
 
+ $classes = array_unique($classes);
+
  $cache = $cacheDir.'/'.$name.$extension;
 
  // auto-reload
@@ -85,23 +92,19 @@ static public function load($classes, $cacheDir, $name, $autoReload, $adaptive =
  return;
  }
 
- // order classes to avoid redeclaration at runtime (class declared before its parent)
- self::orderClasses($classes);
-
  $files = array();
  $content = '';
- foreach ($classes as $class) {
- if (!class_exists($class) && !interface_exists($class) && (!function_exists('trait_exists') || !trait_exists($class))) {
- throw new \InvalidArgumentException(sprintf('Unable to load class "%s"', $class));
+ foreach (self::getOrderedClasses($classes) as $class) {
+ if (in_array($class->getName(), $declared)) {
+ continue;
  }
 
- $r = new \ReflectionClass($class);
- $files[] = $r->getFileName();
+ $files[] = $class->getFileName();
 
- $c = preg_replace(array('/^\s*<\?php/', '/\?>\s*$/'), '', file_get_contents($r->getFileName()));
+ $c = preg_replace(array('/^\s*<\?php/', '/\?>\s*$/'), '', file_get_contents($class->getFileName()));
 
  // add namespace declaration for global code
- if (!$r->inNamespace()) {
+ if (!$class->inNamespace()) {
  $c = "\nnamespace\n{\n".self::stripComments($c)."\n}\n";
  } else {
  $c = self::fixNamespaceDeclarations('<?php '.$c);
@@ -229,47 +232,80 @@ static private function stripComments($source)
  }
 
  /**
- * Orders a set of classes according to their number of parents.
+ * Gets an ordered array of passed classes including all their dependencies.
  *
  * @param array $classes
  *
+ * @return array An array of sorted \ReflectionClass instances (dependencies added if needed)
+ *
  * @throws \InvalidArgumentException When a class can't be loaded
  */
- static private function orderClasses(array &$classes)
+ static private function getOrderedClasses(array $classes)
  {
+ $map = array();
+ self::$seen = array();
  foreach ($classes as $class) {
- if (isset(self::$baseClassesCountMap[$class])) {
- continue;
- }
-
  try {
  $reflectionClass = new \ReflectionClass($class);
  } catch (\ReflectionException $e) {
  throw new \InvalidArgumentException(sprintf('Unable to load class "%s"', $class));
  }
 
- // The counter is cached to avoid reflection if the same class is asked again later
- self::$baseClassesCountMap[$class] = self::countParentClasses($reflectionClass) + count($reflectionClass->getInterfaces());
+ $map = array_merge($map, self::getClassHierarchy($reflectionClass));
+ }
+
+ return $map;
+ }
+
+ static private function getClassHierarchy(\ReflectionClass $class)
+ {
+ if (isset(self::$seen[$class->getName()])) {
+ return array();
  }
 
- asort(self::$baseClassesCountMap);
+ self::$seen[$class->getName()] = true;
+
+ $classes = array($class);
+ $parent = $class;
+ while (($parent = $parent->getParentClass()) && $parent->isUserDefined() && !isset(self::$seen[$parent->getName()])) {
+ self::$seen[$parent->getName()] = true;
 
- $classes = array_intersect(array_keys(self::$baseClassesCountMap), $classes);
+ array_unshift($classes, $parent);
+ }
+
+ if (function_exists('get_declared_traits')) {
+ foreach ($classes as $c) {
+ foreach (self::getTraits($c) as $trait) {
+ self::$seen[$trait->getName()] = true;
+
+ array_unshift($classes, $trait);
+ }
+ }
+ }
+
+ foreach ($class->getInterfaces() as $interface) {
+ if ($interface->isUserDefined() && !isset(self::$seen[$interface->getName()])) {
+ self::$seen[$interface->getName()] = true;
+
+ array_unshift($classes, $interface);
+ }
+ }
+
+ return $classes;
  }
 
- /**
- * Counts the number of parent classes in userland.
- *
- * @param \ReflectionClass $class
- * @param integer $count If exists, the current counter
- * @return integer
- */
- static private function countParentClasses(\ReflectionClass $class, $count = 0)
+ static private function getTraits(\ReflectionClass $class)
  {
- if (($parent = $class->getParentClass()) && $parent->isUserDefined()) {
- $count = self::countParentClasses($parent, ++$count);
+ $traits = $class->getTraits();
+ $classes = array();
+ while ($trait = array_pop($traits)) {
+ if ($trait->isUserDefined() && !isset(self::$seen[$trait->getName()])) {
+ $classes[] = $trait;
+
+ $traits = array_merge($traits, $trait->getTraits());
+ }
  }
 
- return $count;
+ return $classes;
  }
 }

src/Symfony/Component/ClassLoader/Tests/ClassCollectionLoaderTest.php

@@ -12,7 +12,6 @@
 namespace Symfony\Component\ClassLoader\Tests;
 
 use Symfony\Component\ClassLoader\ClassCollectionLoader;
-use Symfony\Component\ClassLoader\UniversalClassLoader;
 
 require_once __DIR__.'/Fixtures/ClassesWithParents/CInterface.php';
 require_once __DIR__.'/Fixtures/ClassesWithParents/B.php';
@@ -25,38 +24,19 @@ class ClassCollectionLoaderTest extends \PHPUnit_Framework_TestCase
  */
  public function testClassReordering(array $classes)
  {
- $expected = <<<EOF
-<?php 
-
-namespace ClassesWithParents
-{
-
-interface CInterface {}
-}
- 
-
-namespace ClassesWithParents
-{
-
-class B implements CInterface {}
-}
- 
-
-namespace ClassesWithParents
-{
-
-class A extends B {}
-}
-
-EOF;
+ $expected = array(
+ 'ClassesWithParents\\CInterface',
+ 'ClassesWithParents\\B',
+ 'ClassesWithParents\\A',
+ );
 
- $dir = sys_get_temp_dir();
- $fileName = uniqid('symfony_');
+ $r = new \ReflectionClass('Symfony\Component\ClassLoader\ClassCollectionLoader');
+ $m = $r->getMethod('getOrderedClasses');
+ $m->setAccessible(true);
 
- ClassCollectionLoader::load($classes, $dir, $fileName, true);
- $cachedContent = @file_get_contents($dir.'/'.$fileName.'.php');
+ $ordered = $m->invoke('Symfony\Component\ClassLoader\ClassCollectionLoader', $classes);
 
- $this->assertEquals($expected, $cachedContent);
+ $this->assertEquals($expected, array_map(function ($class) { return $class->getName(); }, $ordered));
  }
 
  public function getDifferentOrders()
@@ -77,6 +57,59 @@ public function getDifferentOrders()
  'ClassesWithParents\\B',
  'ClassesWithParents\\A',
  )),
+ array(array(
+ 'ClassesWithParents\\A',
+ )),
+ );
+ }
+
+ /**
+ * @dataProvider getDifferentOrdersForTraits
+ */
+ public function testClassWithTraitsReordering(array $classes)
+ {
+ if (version_compare(phpversion(), '5.4.0', '<')) {
+ $this->markTestSkipped('Requires PHP > 5.4.0.');
+
+ return;
+ }
+
+ require_once __DIR__.'/Fixtures/ClassesWithParents/ATrait.php';
+ require_once __DIR__.'/Fixtures/ClassesWithParents/BTrait.php';
+ require_once __DIR__.'/Fixtures/ClassesWithParents/CTrait.php';
+ require_once __DIR__.'/Fixtures/ClassesWithParents/D.php';
+ require_once __DIR__.'/Fixtures/ClassesWithParents/E.php';
+
+ $expected = array(
+ 'ClassesWithParents\\CInterface',
+ 'ClassesWithParents\\CTrait',
+ 'ClassesWithParents\\ATrait',
+ 'ClassesWithParents\\BTrait',
+ 'ClassesWithParents\\B',
+ 'ClassesWithParents\\A',
+ 'ClassesWithParents\\D',
+ 'ClassesWithParents\\E',
+ );
+
+ $r = new \ReflectionClass('Symfony\Component\ClassLoader\ClassCollectionLoader');
+ $m = $r->getMethod('getOrderedClasses');
+ $m->setAccessible(true);
+
+ $ordered = $m->invoke('Symfony\Component\ClassLoader\ClassCollectionLoader', $classes);
+
+ $this->assertEquals($expected, array_map(function ($class) { return $class->getName(); }, $ordered));
+ }
+
+ public function getDifferentOrdersForTraits()
+ {
+ return array(
+ array(array(
+ 'ClassesWithParents\\E',
+ 'ClassesWithParents\\ATrait',
+ )),
+ array(array(
+ 'ClassesWithParents\\E',
+ )),
  );
  }