merged branch dlsniper/loader-exception-improvement (PR #841)

This PR was squashed before being merged into the master branch (closes #841). Commits ------- 08ecb0e Improvements for loader speeds Discussion ---------- Improvements for loader speeds This is something started from #822 . It attempts to improve the speed of the chain loader and rest of the standard loaders by adding a cache level and a ```hasSource()``` method for them in order gain speed. --------------------------------------------------------------------------- by dlsniper at 2012-09-27T14:56:24Z Thanks for the feedback, I'll do the changes tonight or during the coming weekend. As for the utility of the ```::setExtendedErrorHandling()``` function, this saves me about 25% to 40% on the benchkmark Fabien presented in the original ticket. Thanks! --------------------------------------------------------------------------- by dlsniper at 2012-09-29T11:34:59Z I've added some caching for the Chain Loader but it's still a WIP. I'll work more on this in the coming hours. --------------------------------------------------------------------------- by dlsniper at 2012-09-29T23:10:10Z I'm getting better values right now that I had before, standing on: ``` 0.09 0.07 ... x 1 0.07 ... x 1 ``` I've also noticed that if I run the tests from the browser then I'll have an overall slowdown of about 0.10 which I'm not sure from where it comes from. Any pointers before me digging a bit more into Twigs internals. --------------------------------------------------------------------------- by dlsniper at 2012-10-03T21:31:15Z @fabpot what's your opinion about this PR? --------------------------------------------------------------------------- by dlsniper at 2012-10-14T18:26:13Z @Tobion and @fabpot done and done :) I'm not sure why the tests are failing on PHP 5.4 as I don't have it installed anywhere.
twigphp · Oct 19, 2012 · ed05546 · ed05546
2 parents af1bfe9 + 08ecb0e
commit ed05546
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 55 deletions.
diff --git a/lib/Twig/Error.php b/lib/Twig/Error.php
@@ -112,6 +112,8 @@ public function setTemplateLine($lineno)
      * @param array  $arguments The parameters to be passed to the method
      *
      * @return Exception The previous exception or null
+     *
+     * @throws BadMethodCallException
      */
     public function __call($method, $arguments)
     {

diff --git a/lib/Twig/ExtendedLoaderInterface.php b/lib/Twig/ExtendedLoaderInterface.php
@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) 2009 Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+/**
+ * Interface all loaders must implement in order to provide extra functionality
+ * for the Twig core.
+ *
+ * @package    twig
+ * @author     Florin Patan <florinpatan@gmail.com>
+ */
+interface Twig_ExtendedLoaderInterface
+{
+
+    /**
+     * Check if we have the source code of a template, given its name.
+     *
+     * @param string $name The name of the template to check if we can load
+     *
+     * @return boolean If the template source code is handled by this loader or not
+     */
+    public function exists($name);
+
+}
diff --git a/lib/Twig/Loader/Array.php b/lib/Twig/Loader/Array.php
@@ -20,7 +20,7 @@
  * @package    twig
  * @author     Fabien Potencier <fabien@symfony.com>
  */
-class Twig_Loader_Array implements Twig_LoaderInterface
+class Twig_Loader_Array implements Twig_LoaderInterface, Twig_ExtendedLoaderInterface
 {
     protected $templates;
 
@@ -51,11 +51,7 @@ public function setTemplate($name, $template)
     }
 
     /**
-     * Gets the source code of a template, given its name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The template source code
+     * {@inheritdoc}
      */
     public function getSource($name)
     {
@@ -68,11 +64,15 @@ public function getSource($name)
     }
 
     /**
-     * Gets the cache key to use for the cache for a given template name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The cache key
+     * {@inheritdoc}
+     */
+    public function exists($name)
+    {
+        return isset($this->templates[(string) $name]);
+    }
+
+    /**
+     * {@inheritdoc}
      */
     public function getCacheKey($name)
     {
@@ -85,10 +85,7 @@ public function getCacheKey($name)
     }
 
     /**
-     * Returns true if the template is still fresh.
-     *
-     * @param string    $name The template name
-     * @param timestamp $time The last modification time of the cached template
+     * {@inheritdoc}
      */
     public function isFresh($name, $time)
     {

diff --git a/lib/Twig/Loader/Chain.php b/lib/Twig/Loader/Chain.php
@@ -15,8 +15,9 @@
  * @package twig
  * @author  Fabien Potencier <fabien@symfony.com>
  */
-class Twig_Loader_Chain implements Twig_LoaderInterface
+class Twig_Loader_Chain implements Twig_LoaderInterface, Twig_ExtendedLoaderInterface
 {
+    private $hasSourceCache = array();
     protected $loaders;
 
     /**
@@ -40,19 +41,20 @@ public function __construct(array $loaders = array())
     public function addLoader(Twig_LoaderInterface $loader)
     {
         $this->loaders[] = $loader;
+        $this->hasSourceCache = array();
     }
 
     /**
-     * Gets the source code of a template, given its name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The template source code
+     * {@inheritdoc}
      */
     public function getSource($name)
     {
         $exceptions = array();
         foreach ($this->loaders as $loader) {
+            if ($loader instanceof Twig_ExtendedLoaderInterface && !$loader->exists($name)) {
+                continue;
+            }
+
             try {
                 return $loader->getSource($name);
             } catch (Twig_Error_Loader $e) {
@@ -64,16 +66,43 @@ public function getSource($name)
     }
 
     /**
-     * Gets the cache key to use for the cache for a given template name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The cache key
+     * {@inheritdoc}
+     */
+    public function exists($name)
+    {
+        if (isset($this->hasSourceCache[$name])) {
+            return $this->hasSourceCache[$name];
+        }
+
+        foreach ($this->loaders as $loader) {
+            if ($loader instanceof Twig_ExtendedLoaderInterface) {
+                if ($loader->exists($name)) {
+                    return $this->hasSourceCache[$name] = true;
+                }
+            } else {
+                try {
+                    $loader->getSource($name);
+                    return $this->hasSourceCache[$name] = true;
+                } catch (Twig_Error_Loader $e) {
+
+                }
+            }
+        }
+
+        return $this->hasSourceCache[$name] = false;
+    }
+
+    /**
+     * {@inheritdoc}
      */
     public function getCacheKey($name)
     {
         $exceptions = array();
         foreach ($this->loaders as $loader) {
+            if ($loader instanceof Twig_ExtendedLoaderInterface && !$loader->exists($name)) {
+                continue;
+            }
+
             try {
                 return $loader->getCacheKey($name);
             } catch (Twig_Error_Loader $e) {
@@ -85,15 +114,16 @@ public function getCacheKey($name)
     }
 
     /**
-     * Returns true if the template is still fresh.
-     *
-     * @param string    $name The template name
-     * @param timestamp $time The last modification time of the cached template
+     * {@inheritdoc}
      */
     public function isFresh($name, $time)
     {
         $exceptions = array();
         foreach ($this->loaders as $loader) {
+            if ($loader instanceof Twig_ExtendedLoaderInterface && !$loader->exists($name)) {
+                continue;
+            }
+
             try {
                 return $loader->isFresh($name, $time);
             } catch (Twig_Error_Loader $e) {

diff --git a/lib/Twig/Loader/Filesystem.php b/lib/Twig/Loader/Filesystem.php
@@ -15,7 +15,7 @@
  * @package    twig
  * @author     Fabien Potencier <fabien@symfony.com>
  */
-class Twig_Loader_Filesystem implements Twig_LoaderInterface
+class Twig_Loader_Filesystem implements Twig_LoaderInterface, Twig_ExtendedLoaderInterface
 {
     protected $paths;
     protected $cache;
@@ -77,6 +77,8 @@ public function setPaths($paths, $namespace = '__main__')
      *
      * @param string $path      A path where to look for templates
      * @param string $namespace A path name
+     *
+     * @throws Twig_Error_Loader
      */
     public function addPath($path, $namespace = '__main__')
     {
@@ -95,6 +97,8 @@ public function addPath($path, $namespace = '__main__')
      *
      * @param string $path      A path where to look for templates
      * @param string $namespace A path name
+     *
+     * @throws Twig_Error_Loader
      */
     public function prependPath($path, $namespace = '__main__')
     {
@@ -115,34 +119,41 @@ public function prependPath($path, $namespace = '__main__')
     }
 
     /**
-     * Gets the source code of a template, given its name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The template source code
+     * {@inheritdoc}
      */
     public function getSource($name)
     {
         return file_get_contents($this->findTemplate($name));
     }
 
     /**
-     * Gets the cache key to use for the cache for a given template name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The cache key
+     * {@inheritdoc}
      */
     public function getCacheKey($name)
     {
         return $this->findTemplate($name);
     }
 
     /**
-     * Returns true if the template is still fresh.
-     *
-     * @param string    $name The template name
-     * @param timestamp $time The last modification time of the cached template
+     * {@inheritdoc}
+     */
+    public function exists($name)
+    {
+        if (isset($this->cache[$name])) {
+            return true;
+        }
+
+        try {
+            $this->findTemplate($name);
+
+            return true;
+        } catch (Twig_Error_Loader $exception) {
+            return false;
+        }
+    }
+
+    /**
+     * {@inheritdoc}
      */
     public function isFresh($name, $time)
     {

diff --git a/lib/Twig/Loader/String.php b/lib/Twig/Loader/String.php
@@ -24,20 +24,24 @@
  * @package    twig
  * @author     Fabien Potencier <fabien@symfony.com>
  */
-class Twig_Loader_String implements Twig_LoaderInterface
+class Twig_Loader_String implements Twig_LoaderInterface, Twig_ExtendedLoaderInterface
 {
     /**
-     * Gets the source code of a template, given its name.
-     *
-     * @param string $name The name of the template to load
-     *
-     * @return string The template source code
+     * {@inheritdoc}
      */
     public function getSource($name)
     {
         return $name;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function exists($name)
+    {
+        return true;
+    }
+
     /**
      * Gets the cache key to use for the cache for a given template name.
      *
@@ -51,10 +55,7 @@ public function getCacheKey($name)
     }
 
     /**
-     * Returns true if the template is still fresh.
-     *
-     * @param string    $name The template name
-     * @param timestamp $time The last modification time of the cached template
+     * {@inheritdoc}
      */
     public function isFresh($name, $time)
     {