Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-717

Do not use files when using proxy autogeneration

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      Proxy classes are generated in less than 1ms for me. I prefer to not have a "build" step to reducing loading time by a milisecond, so I use autogenerate.

      For users like me, wouldn't it be nicer if we wouldn't even have to configure a proxy dir and those files were never written (since they're not read more than once anyway)?

        Issue Links

          Activity

          Hide
          Jaka Jancar added a comment -

          This very minimal patch removes the use of these temporary files:

          --- library/Doctrine/ORM/Proxy/ProxyFactory.php	(revision 2)
          +++ library/Doctrine/ORM/Proxy/ProxyFactory.php	(working copy)
          @@ -78,9 +78,8 @@
                   $fqn = $this->_proxyNamespace . '\\' . $proxyClassName;
           
                   if ($this->_autoGenerate && ! class_exists($fqn, false)) {
          -            $fileName = $this->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php';
          -            $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate);
          -            require $fileName;
          +            $file = $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, null, self::$_proxyClassTemplate);
          +            eval('?>'.$file);
                   }
           
                   if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
          @@ -144,6 +143,9 @@
           
                   $file = str_replace($placeholders, $replacements, $file);
           
          +        if ($fileName === null)
          +            return $file;
          +
                   file_put_contents($fileName, $file);
               }
           
          
          Show
          Jaka Jancar added a comment - This very minimal patch removes the use of these temporary files: --- library/Doctrine/ORM/Proxy/ProxyFactory.php (revision 2) +++ library/Doctrine/ORM/Proxy/ProxyFactory.php (working copy) @@ -78,9 +78,8 @@ $fqn = $ this ->_proxyNamespace . '\\' . $proxyClassName; if ($ this ->_autoGenerate && ! class_exists($fqn, false )) { - $fileName = $ this ->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php'; - $ this ->_generateProxyClass($ this ->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate); - require $fileName; + $file = $ this ->_generateProxyClass($ this ->_em->getClassMetadata($className), $proxyClassName, null , self::$_proxyClassTemplate); + eval('?>'.$file); } if ( ! $ this ->_em->getMetadataFactory()->hasMetadataFor($fqn)) { @@ -144,6 +143,9 @@ $file = str_replace($placeholders, $replacements, $file); + if ($fileName === null ) + return $file; + file_put_contents($fileName, $file); }
          Hide
          Benjamin Eberlei added a comment -

          The proxy dir is used for the "doctrine orm:generate-proxies" command in the case of "autogenerate= false", so you need to define it anyways.

          You have to use proxies, the option is not for Proxy yes/no. If you have autogenerate=false and doctrine requires a proxy for a use case but can't find it you will get a fatal error.

          Show
          Benjamin Eberlei added a comment - The proxy dir is used for the "doctrine orm:generate-proxies" command in the case of "autogenerate= false", so you need to define it anyways. You have to use proxies, the option is not for Proxy yes/no. If you have autogenerate=false and doctrine requires a proxy for a use case but can't find it you will get a fatal error.
          Hide
          Benjamin Eberlei added a comment -

          I just saw the eval() keyword, ieeks It could maybe be a convenience option for those that don't want to use proxy direcotiries, however i am not sure.

          Show
          Benjamin Eberlei added a comment - I just saw the eval() keyword, ieeks It could maybe be a convenience option for those that don't want to use proxy direcotiries, however i am not sure.
          Hide
          Jaka Jancar added a comment - - edited

          eval() is no different than writing code to a file and using require().

          When using runtime-generated proxies, there are no benefits (that I know of) from writing them to a file. The disadvantages are:

          • slower because of write disk access
          • has problems with high concurrency, unless special care is taken (DDC-716)
          • potentially has permission problems if code is executed by different users (e.g. nobody for a daemon and www-data for http)
          • requires setup of a writable directory

          This is a nicer patch, which makes _generateProxyClass() return a string, just like other _generate* methods:

          --- library/Doctrine/ORM/Proxy/ProxyFactory.php	(revision 2)
          +++ library/Doctrine/ORM/Proxy/ProxyFactory.php	(working copy)
          @@ -78,9 +78,8 @@
                   $fqn = $this->_proxyNamespace . '\\' . $proxyClassName;
           
                   if ($this->_autoGenerate && ! class_exists($fqn, false)) {
          -            $fileName = $this->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php';
          -            $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate);
          -            require $fileName;
          +            $code = $this->_generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName);
          +            eval($code);
                   }
           
                   if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
          @@ -107,19 +106,19 @@
                   foreach ($classes as $class) {
                       $proxyClassName = str_replace('\\', '', $class->name) . 'Proxy';
                       $proxyFileName = $proxyDir . $proxyClassName . '.php';
          -            $this->_generateProxyClass($class, $proxyClassName, $proxyFileName, self::$_proxyClassTemplate);
          +            $code = $this->_generateProxyClass($class, $proxyClassName);
          +            file_put_contents($proxyFileName, "<?php\n" . $code);
                   }
               }
           
               /**
                * Generates a proxy class file.
                *
          -     * @param $class
          -     * @param $originalClassName
          +     * @param ClassMetadata $class
                * @param $proxyClassName
          -     * @param $file The path of the file to write to.
          +     * @return string The code of the generated methods.
                */
          -    private function _generateProxyClass($class, $proxyClassName, $fileName, $file)
          +    private function _generateProxyClass(ClassMetadata $class, $proxyClassName)
               {
                   $methods = $this->_generateMethods($class);
                   $sleepImpl = $this->_generateSleep($class);
          @@ -142,9 +141,9 @@
                       $methods, $sleepImpl
                   );
           
          -        $file = str_replace($placeholders, $replacements, $file);
          +        $file = str_replace($placeholders, $replacements, self::$_proxyClassTemplate);
           
          -        file_put_contents($fileName, $file);
          +        return $file;
               }
           
               /**
          @@ -244,8 +243,7 @@
           
               /** Proxy class code template */
               private static $_proxyClassTemplate =
          -'<?php
          -
          +'
           namespace <namespace>;
           
           /**
          
          Show
          Jaka Jancar added a comment - - edited eval() is no different than writing code to a file and using require(). When using runtime-generated proxies, there are no benefits (that I know of) from writing them to a file. The disadvantages are: slower because of write disk access has problems with high concurrency, unless special care is taken ( DDC-716 ) potentially has permission problems if code is executed by different users (e.g. nobody for a daemon and www-data for http) requires setup of a writable directory This is a nicer patch, which makes _generateProxyClass() return a string, just like other _generate* methods: --- library/Doctrine/ORM/Proxy/ProxyFactory.php (revision 2) +++ library/Doctrine/ORM/Proxy/ProxyFactory.php (working copy) @@ -78,9 +78,8 @@ $fqn = $ this ->_proxyNamespace . '\\' . $proxyClassName; if ($ this ->_autoGenerate && ! class_exists($fqn, false )) { - $fileName = $ this ->_proxyDir . DIRECTORY_SEPARATOR . $proxyClassName . '.php'; - $ this ->_generateProxyClass($ this ->_em->getClassMetadata($className), $proxyClassName, $fileName, self::$_proxyClassTemplate); - require $fileName; + $code = $ this ->_generateProxyClass($ this ->_em->getClassMetadata($className), $proxyClassName); + eval($code); } if ( ! $ this ->_em->getMetadataFactory()->hasMetadataFor($fqn)) { @@ -107,19 +106,19 @@ foreach ($classes as $class) { $proxyClassName = str_replace('\\', '', $class->name) . 'Proxy'; $proxyFileName = $proxyDir . $proxyClassName . '.php'; - $ this ->_generateProxyClass($class, $proxyClassName, $proxyFileName, self::$_proxyClassTemplate); + $code = $ this ->_generateProxyClass($class, $proxyClassName); + file_put_contents($proxyFileName, "<?php\n" . $code); } } /** * Generates a proxy class file. * - * @param $class - * @param $originalClassName + * @param ClassMetadata $class * @param $proxyClassName - * @param $file The path of the file to write to. + * @ return string The code of the generated methods. */ - private function _generateProxyClass($class, $proxyClassName, $fileName, $file) + private function _generateProxyClass(ClassMetadata $class, $proxyClassName) { $methods = $ this ->_generateMethods($class); $sleepImpl = $ this ->_generateSleep($class); @@ -142,9 +141,9 @@ $methods, $sleepImpl ); - $file = str_replace($placeholders, $replacements, $file); + $file = str_replace($placeholders, $replacements, self::$_proxyClassTemplate); - file_put_contents($fileName, $file); + return $file; } /** @@ -244,8 +243,7 @@ /** Proxy class code template */ private static $_proxyClassTemplate = -'<?php - +' namespace <namespace>; /**
          Hide
          Benjamin Eberlei added a comment -

          Scheduled usage of eval() for 2.1, if the following conditions exist:

          1. Autogenerate is set to TRUE
          2. No Proxy Directory is configured.

          Show
          Benjamin Eberlei added a comment - Scheduled usage of eval() for 2.1, if the following conditions exist: 1. Autogenerate is set to TRUE 2. No Proxy Directory is configured.
          Hide
          Jaka Jancar added a comment -

          Great, this is even better. This way you can have 1) autogenerated in ram-only, 2) autogenerated in files and 3) pregenerated.

          And the minimal amount of config needed to get up and running is reduced, which is always nice.

          Show
          Jaka Jancar added a comment - Great, this is even better. This way you can have 1) autogenerated in ram-only, 2) autogenerated in files and 3) pregenerated. And the minimal amount of config needed to get up and running is reduced, which is always nice.
          Hide
          Benjamin Eberlei added a comment -

          you should know though, eval is dead slow. It generates the necessary proxies on EACH request and that cannot be cached in APC.

          Show
          Benjamin Eberlei added a comment - you should know though, eval is dead slow. It generates the necessary proxies on EACH request and that cannot be cached in APC.
          Hide
          Jaka Jancar added a comment -

          It's no slower than current autogeneration (file_put_contents+require). TBH, I don't know why anyone would want to use that over eval(), but I don't mind it being there.

          Pre-generation is, of course, a different thing. Seems like a valid tradeoff to offer: build/a bit of config/better perfomance vs. no build/no config/potentially slower.

          Show
          Jaka Jancar added a comment - It's no slower than current autogeneration (file_put_contents+require). TBH, I don't know why anyone would want to use that over eval(), but I don't mind it being there. Pre-generation is, of course, a different thing. Seems like a valid tradeoff to offer: build/a bit of config/better perfomance vs. no build/no config/potentially slower.
          Hide
          Benjamin Eberlei added a comment -

          Yes, that is because file_put_contents + require is a development only strategy. The manual clearly states that autogenerate has to be false in production.

          Show
          Benjamin Eberlei added a comment - Yes, that is because file_put_contents + require is a development only strategy. The manual clearly states that autogenerate has to be false in production.
          Hide
          Roman S. Borschel added a comment -

          Using eval() instead of producing and requiring the file in the case of enabled auto-generation of proxy classes sounds like a good improvement for 2.1 to make proxies more transparent during deveopment and for anyone for whom performance is no issue.

          I'm increasing the priority as I think it is easy to implement for 2.1 and a good enhancement.

          Show
          Roman S. Borschel added a comment - Using eval() instead of producing and requiring the file in the case of enabled auto-generation of proxy classes sounds like a good improvement for 2.1 to make proxies more transparent during deveopment and for anyone for whom performance is no issue. I'm increasing the priority as I think it is easy to implement for 2.1 and a good enhancement.
          Hide
          Karsten Dambekalns added a comment -

          A note on why having the proxies written to a file can be useful even with autogenerate being on: it makes it really easy to check the proxy code being generated. I use that a lot currently.

          The solution suggested, giving three possibilities is cool, though.

          Show
          Karsten Dambekalns added a comment - A note on why having the proxies written to a file can be useful even with autogenerate being on: it makes it really easy to check the proxy code being generated. I use that a lot currently. The solution suggested, giving three possibilities is cool, though.

            People

            • Assignee:
              Benjamin Eberlei
              Reporter:
              Jaka Jancar
            • Votes:
              2 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: