-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Preload RFC and Symfony #29105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The preloading RFC is about preloading class and function definitions, it can't preload variables. We could go back to something similar to the class cache we had at some point but IMO preloading is more something that should be resolved at the composer level (composer could provide a script that preload all classes of a project) not at the Symfony level. |
Yes, I thought about class cache to reduce overhead per request. About composer, Jordi has some ideas: https://externals.io/message/103333#103335 I wanted to say we can do some benchmarks and think about possible improvements on symfony side. I asked about existing benchmarks on symfony slack today, and got a response from Marco - I don't know technical aspects of preloading well, but it looks like some changes can be discussed. |
Maybe preloading the compiled container can provide some benefit? |
Sure it would; see php/php-src#3538 (comment) for some numbers using Symfony. I'm not sure yet what we should do in core. Having support for preloading at composer stage looks like the way to go to me also. |
The only could be done IMO in Symfony (but I'm not even sure it would bring benefits) is give the possibility to revert to a single file container to allow having the full container code preloaded since included files would not be preloaded. |
@jvasseur the extra container files are not defining any class or function, and so would not benefit from preloading. |
hmm, maybe the bypass of the autoloader by using |
I tried the php-preload branch yesterday with symfony 3.4 The preload script was generated by calling an endpoint, and extracting the loaded classes, from php, so not all the classes was preloaded from vendor After the extraction, i removed the following classes from preloading to make it work currently
There can be problems, where the code, is using if/else constructs for different class declarations or class_exists/interface_exists checks ... |
Well, for swiftmailer, it relies on performing some special initializing when loading the library, so it indeed cannot be preloaded. The cache should indeed not be preloaded in debug mode (you could maybe preload it in prod, where you can afford restarting PHP-FPM when you rebuild the cache (and it may still be complex to rebuild the cache in such case, so I would not advice doing it). But a preload script generated For Twig, maybe preloading does not handle |
@stof what I meant is by including extra files in the container we have to hit the opcache to get the opcodes necessary to create the service, if the code was included in the container class, the opcodes would already be preloaded with the container class. |
|
@jvasseur for services not part of the hot path (or for all services when disabling the |
About |
@stof this has nothing about defining new classes. We split the container into multiple files because it removes the cost of copying opcodes from opcache to the symbol table. With preloading this cost is removed so we could go back to the old version to remove the cost of including another file. This is probably a really small optimization but I could be useful to do a benchmark to see if it could bring some benefits. |
maybe if i just preload vendor/twig/twig/lib/Twig/RuntimeLoaderInterface.php which contains an additional class_alias: var_dump(interface_exists('\Twig_RuntimeLoaderInterface')); bool(true) I will ask dstogov, on this |
Here is the issue to implement it in composer: composer/composer#7777 |
Ah sorry, I missed this! |
I think his reply means the class_alias call will not create a peristent, immutable class alias during preload |
the last comment says it should now work |
Preloading is not about avoiding the cost of copying opcodes. It is about avoiding the cost of copying class and function definitions (plus the resolving of parents, which is done at runtime when they live in separate files). So AFAIU, preloading won't change things for files not defining classes or functions. |
that's an interesting idea @nicolas-grekas |
I think class_alias calls can remain in the original files,, if include is used instead of opcache_compile_file in preload file |
Please check #32032, feedback and benchs appreciated. |
…lder (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [DI] generate preload.php file for PHP 7.4 in cache folder | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29105 | License | MIT | Doc PR | - This PR makes the PhpDumper generate a preloading file suited for PHP 7.4. On a skeleton app, the generated file is `var/cache/dev/srcApp_KernelDevDebugContainer.preload.php` (of course, this varies by env name + kernel class) One missing thing is listing some classes that are always needed but are not related to services. Typically: `Request` and `Response`. We might need a new mechanism to make this list extensible. I did not measure the benefit of this on PHP 7.4. I would really appreciate if someone could give it a try on PHP 7.4 with preloadin 7071 g enabled. Commits ------- c4dad0d [DI] generate preload.php file for PHP 7.4 in cache folder
Hi,
Preload RFC is now in voting phase, so I thought this is a good time to find out how it works with Symfony and what changes can be made to improve performance even further.
Maybe someone already did that?
The text was updated successfully, but these errors were encountered: