Skip to content

Loading…

DDC-717: Do not use files when using proxy autogeneration #5230

Closed
doctrinebot opened this Issue · 13 comments

2 participants

@doctrinebot

Jira issue originally created by user jakajancar:

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)?

@doctrinebot

Comment created by jakajancar:

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

--- library/Doctrine/ORM/Proxy/ProxyFactory.php (revision 2)
<ins></ins><ins> library/Doctrine/ORM/Proxy/ProxyFactory.php    (working copy)
@@ -78,9 </ins>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;
<ins>            $file = $this->*generateProxyClass($this->_em->getClassMetadata($className), $proxyClassName, null, self::$*proxyClassTemplate);
</ins>            eval('?>'.$file);
         }

         if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
@@ -144,6 <ins>143,9 @@

         $file = str_replace($placeholders, $replacements, $file);

</ins>        if ($fileName === null)
<ins>            return $file;
</ins>
         file*put*contents($fileName, $file);
     }

@doctrinebot

Comment created by @beberlei:

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.

@doctrinebot

Comment created by @beberlei:

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.

@doctrinebot

Comment created by jakajancar:

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)
<ins></ins><ins> library/Doctrine/ORM/Proxy/ProxyFactory.php    (working copy)
@@ -78,9 </ins>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;
<ins>            $code = $this->*generateProxyClass($this->*em->getClassMetadata($className), $proxyClassName);
</ins>            eval($code);
         }

         if ( ! $this->_em->getMetadataFactory()->hasMetadataFor($fqn)) {
@@ -107,19 <ins>106,19 @@
         foreach ($classes as $class) {
             $proxyClassName = str_replace('\\', '', $class->name) . 'Proxy';
             $proxyFileName = $proxyDir . $proxyClassName . '.php';
-            $this->*generateProxyClass($class, $proxyClassName, $proxyFileName, self::$*proxyClassTemplate);
</ins>            $code = $this->_generateProxyClass($class, $proxyClassName);
<ins>            file*put*contents($proxyFileName, "<?php\n" . $code);
         }
     }

     /****
      * Generates a proxy class file.
      *
-     * @param $class
-     * @param $originalClassName
</ins>     * @param ClassMetadata $class
      * @param $proxyClassName
-     * @param $file The path of the file to write to.
<ins>     * @return string The code of the generated methods.
      */
-    private function _generateProxyClass($class, $proxyClassName, $fileName, $file)
</ins>    private function _generateProxyClass(ClassMetadata $class, $proxyClassName)
     {
         $methods = $this->_generateMethods($class);
         $sleepImpl = $this->_generateSleep($class);
@@ -142,9 <ins>141,9 @@
             $methods, $sleepImpl
         );

-        $file = str_replace($placeholders, $replacements, $file);
</ins>        $file = str*replace($placeholders, $replacements, self::$*proxyClassTemplate);

-        file*put*contents($fileName, $file);
<ins>        return $file;
     }

     /****
@@ -244,8 </ins>243,7 @@

     /*** Proxy class code template **/
     private static $_proxyClassTemplate =
-'<?php
-
+'
 namespace <namespace>;

 /****
@doctrinebot

Comment created by @beberlei:

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

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

Comment created by jakajancar:

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.

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Comment created by jakajancar:

It's no slower than current autogeneration (fileputcontents+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.

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Comment created by romanb:

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.

@doctrinebot

Comment created by k-fish:

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.

@doctrinebot

Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.4 milestone
@doctrinebot doctrinebot closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.