merged branch fabpot/classloader-optim (PR #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).
symfony · Jul 4, 2012 · 5d8ded6 · 5d8ded6
2 parents 0942a7b + 685dd12
commit 5d8ded6
Show file tree

Hide file tree

Showing 7 changed files with 170 additions and 63 deletions.
diff --git a/ClassCollectionLoader.php b/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;
     }
 }
diff --git a/Tests/ClassCollectionLoaderTest.php b/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',
+            )),
         );
     }
 

diff --git a/Tests/Fixtures/ClassesWithParents/ATrait.php b/Tests/Fixtures/ClassesWithParents/ATrait.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace ClassesWithParents;
+
+trait ATrait
+{
+}
diff --git a/Tests/Fixtures/ClassesWithParents/BTrait.php b/Tests/Fixtures/ClassesWithParents/BTrait.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace ClassesWithParents;
+
+trait BTrait
+{
+    use ATrait;
+}
diff --git a/Tests/Fixtures/ClassesWithParents/CTrait.php b/Tests/Fixtures/ClassesWithParents/CTrait.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace ClassesWithParents;
+
+trait CTrait
+{
+}
diff --git a/Tests/Fixtures/ClassesWithParents/D.php b/Tests/Fixtures/ClassesWithParents/D.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace ClassesWithParents;
+
+class D extends A
+{
+    use BTrait;
+}
diff --git a/Tests/Fixtures/ClassesWithParents/E.php b/Tests/Fixtures/ClassesWithParents/E.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace ClassesWithParents;
+
+class E extends D
+{
+    use CTrait;
+}