[DI] generate preload.php file for PHP 7.4 in cache folder #32032

nicolas-grekas · 2019-06-13T17:22:16Z

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 preloading enabled.

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php

nicolas-grekas · 2019-06-14T14:26:41Z

For reference, here is some previous work by @BenMorel that shows that loading only the hot classes (vs everything) is what provides the best boost. Identifying these hot classes is what I'm trying to achieve here.

teohhanhui · 2019-06-14T17:11:40Z

Is it really worth it to preload only hot classes? Doesn't that increase the complexity a lot, vs just preloading everything?

BenMorel · 2019-06-14T17:38:09Z

There's a slight performance improvement vs preloading everything when preloading hot classes that were selected using actual runtime data, as reported by opcache after a few runs of the application:

Benchmark	Requests per second	Diff
No preloading	631 rq/s	-
Preload hot classes	738 rq/s	+17%
Preload everything	712 rq/s	+13%

(original benchmark in this thread)

Any other pre-computed list would need to be benchmarked again, as the results may be very different (and could even be slower than preloading everything, as the list may be missing classes that will need to be linked at runtime).

All in all, the difference between a carefully optimized preloader and an eager preloader is small, I'd personally be in favour of preloading everything by default, and let users create their own preloading script if they need the extra few % of performance.

What could be interesting however, is shipping Symfony with a tool (like the one I used in this post) to generate the preloading file from opcache, together with some instructions on how to:

restart the server
navigate the website / use as many features of the app as possible
then dump the preloading script

nicolas-grekas · 2019-06-14T20:52:12Z

Identifying the hot classes using runtime data from opcache has a big drawback: rebooting the fpm pool on prod servers. Not sure it's calling for good ops practices.
Without having working numbers, it's way too early to draw any conclusions or close any doors.
You may be right though, but please allow me to do the experiment :)

BenMorel · 2019-06-14T21:29:44Z

I did not have in mind to gather runtime data from the prod environment, but from the dev one!

No worries, I just wanted to give a bit of warning here, I'm looking forward to your numbers!

k00ni · 2019-06-18T11:09:52Z

I am not sure, if this is the right place to ask.

In the related RFC they say:

The traded-in flexibility would include the inability to update these files once the server has been started (updating these files on the filesystem will not do anything; A server restart will be required to apply the changes); And also, this approach will not be compatible with servers that host multiple applications, or multiple versions of applications [...]

How do you plan to manage updates in the vendor folder, if you have to restart the server to apply changes? If i am on a shared hosting environment, would that affect me when using Symfony in the future?

nicolas-grekas · 2019-06-18T11:26:31Z

Preloading is fundamentally incompatible with shared hosting yes, so you wouldn't use this on your provider. The added file is totally opt-in so one won't use it if one isn't able to restart the FPM server when deploying new code, that's correct.

BenMorel · 2019-06-18T11:26:59Z

If am on a shared hosting environment, would that affect me when using Symfony in the future?

Nope, preloading requires setup at server level (opcache.preload), which will never be enabled on shared hosting environments.

Most likely any preloading script shipped with Symfony or Composer will be opt-in, and the choice to include it or not will be left to the server administrator.

teohhanhui · 2019-06-18T11:57:34Z

It's 2019, so hopefully the population of developers who unfortunately still have to deal with shared hosting is shrinking rapidly...

…grekas)" (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- Revert "minor #32054 Prepare for PHP 7.4 preload (nicolas-grekas)" | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #32032 | License | MIT | Doc PR | - This reverts commit a0aa941, reversing changes made to 8496003. This change is incompatible with Composer's class map generation. Commits ------- 8a1813a Revert "minor #32054 Prepare for PHP 7.4 preload (nicolas-grekas)"

tsantos84 · 2019-06-26T10:19:31Z

Hi @nicolas-grekas, if we have this feature in composer we could configure the classes directly on composer.json file and it could work for vendor/package level as well. Makes sense?

HttpFoundation composer.json file:

{
    "preload": [
         "Symfony\\HttpFoundation\\{Request,Response}",
         "Symfony\\EntireNamespace\\*",
         "Symfony\\{NS1,NS2}\\*"
    ]
}

tsantos84 · 2019-06-26T13:22:56Z

I've just read the RFC at composer/composer#7777 and seems that is not an easy decision to put such feature on composer.

nicolas-grekas · 2019-09-08T15:52:30Z

I rebased this PR and I think it's time to merge it.

Preloading doesn't work yet, but that's on PHP's side.
The patch also fines tune the generated container even when preloading is not in use.

Status: needs review

fabpot · 2019-09-08T17:24:53Z

Thank you @nicolas-grekas.

…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 preloading enabled. Commits ------- c4dad0d [DI] generate preload.php file for PHP 7.4 in cache folder

stof · 2019-09-09T09:36:12Z

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

@@ -522,13 +553,17 @@ private function addServiceInclude(string $cId, Definition $definition): string
            }

            foreach (array_diff_key(array_flip($lineage), $this->inlinedRequires) as $file => $class) {
+                $file = preg_replace('#^\\$this->targetDirs\[(\d++)\]#', sprintf('\dirname(__DIR__, %d + $1)', $this->asFiles), $file);


why generating code using $this->targetDirs first, to replace it later with dirname(__DIR__) ? We would generate the right code directly.

I prefer not changing this when it's not needed: the code starts with
$this->targetDirs[0] = \dirname($containerDir): target dirs are dynamic. This is used when calling cache clear and is stable, no reason to change.
Using dirname() is really useful when referencing files in the src/ or in the vendor/ folder, which is where it is used.
Lowest risk strategy :)

Second look, I did something, please check #33529 :)

stof · 2019-09-09T09:36:52Z

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

@@ -1285,7 +1320,7 @@ protected function {$methodNameAlias}()
        return $code;
    }

-    private function addInlineRequires(): string
+    private function addInlineRequires(?array &$preload): string


should null be allowed ?

to allow calling the method with an uninitialized variable

stof · 2019-09-09T09:40:47Z

src/Symfony/Component/DependencyInjection/Dumper/Preloader.php

+                        $c = $p->getDefaultValueConstantName();
+
+                        if ($i = strpos($c, '::')) {
+                            self::doPreload(substr($c, 0, $i));


if you reuse the same class multiple types in different parameters, wouldn't this preload it multiple times ? Also, it would not register it in $preloaded in the main function, and so it will be processed once again by the main function.

Indeed, fixed in #33525

steevanb · 2019-11-18T00:25:38Z

@nicolas-grekas As soon as PHP 7.4 will be available, I will benchmark with and without preloading and add results on phpbenchmarks.com ;)

bradjones1 · 2020-01-26T00:36:48Z

@steevanb Is there a benchmark on preloading per your note above? I looked on the site and didn't find anything specific, but maybe I just missed it.

steevanb · 2020-01-29T10:58:56Z

@bradjones1 for now PHP 7.4 is not added on phpbenchmarks.com, cause i'm on a new version of benchmarks and site who will be released in a few weeks.

nicolas-grekas added this to the next milestone Jun 13, 2019

carsonbot added Status: Needs Review DependencyInjection Feature labels Jun 13, 2019

nicolas-grekas mentioned this pull request Jun 13, 2019

Preload RFC and Symfony #29105

Closed

frankdejonge reviewed Jun 13, 2019

View reviewed changes

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php Show resolved Hide resolved

stof reviewed Jun 13, 2019

View reviewed changes

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php Outdated Show resolved Hide resolved

digilist reviewed Jun 13, 2019

View reviewed changes

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php Show resolved Hide resolved

nicolas-grekas force-pushed the preload branch 2 times, most recently from 85e1fec to e2b7dce Compare June 14, 2019 14:18

nicolas-grekas force-pushed the preload branch 5 times, most recently from aa39efe to 8aa4aca Compare June 17, 2019 12:54

nicolas-grekas added Status: Needs Work and removed Status: Needs Review labels Jun 19, 2019

nicolas-grekas mentioned this pull request Jun 20, 2019

Revert "minor #32054 Prepare for PHP 7.4 preload (nicolas-grekas)" #32110

Merged

nicolas-grekas force-pushed the preload branch 5 times, most recently from 517bd3e to 5d119d6 Compare September 8, 2019 15:45

[DI] generate preload.php file for PHP 7.4 in cache folder

c4dad0d

nicolas-grekas force-pushed the preload branch from 5d119d6 to c4dad0d Compare September 8, 2019 15:48

carsonbot added Status: Needs Review and removed Status: Needs Work labels Sep 8, 2019

fabpot approved these changes Sep 8, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Sep 8, 2019

fabpot mentioned this pull request Sep 8, 2019

[DI] generate preload.php file for PHP 7.4 in cache folder symfony/symfony-docs#12290

Closed

fabpot merged commit c4dad0d into symfony:4.4 Sep 8, 2019

stof reviewed Sep 9, 2019

View reviewed changes

nicolas-grekas deleted the preload branch September 10, 2019 16:18

nicolas-grekas mentioned this pull request Sep 24, 2019

[HttpKernel][DI] allow bundles to declare classes that should be preloaded #33689

Closed

Flyingmana mentioned this pull request Sep 26, 2019

add php 7.4 Opcache Preloading capability OpenMage/magento-lts#816

Closed

4 tasks

nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019

This was referenced Nov 12, 2019

Release v4.4.0-BETA1 #34338

Merged

Release v5.0.0-BETA1 #34339

Merged

murphpdx mentioned this pull request Nov 26, 2019

Preloading file is missing from the website-skeleton app #34611

Closed

brusch mentioned this pull request Dec 5, 2019

Opcache Preloading Integration pimcore/pimcore#5387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DI] generate preload.php file for PHP 7.4 in cache folder #32032

[DI] generate preload.php file for PHP 7.4 in cache folder #32032

nicolas-grekas commented Jun 13, 2019 •

edited

Loading

nicolas-grekas commented Jun 14, 2019

teohhanhui commented Jun 14, 2019

BenMorel commented Jun 14, 2019

nicolas-grekas commented Jun 14, 2019

BenMorel commented Jun 14, 2019

k00ni commented Jun 18, 2019 •

edited

Loading

nicolas-grekas commented Jun 18, 2019

BenMorel commented Jun 18, 2019 •

edited

Loading

teohhanhui commented Jun 18, 2019 •

edited

Loading

tsantos84 commented Jun 26, 2019 •

edited

Loading

tsantos84 commented Jun 26, 2019

nicolas-grekas commented Sep 8, 2019

fabpot commented Sep 8, 2019

stof Sep 9, 2019

nicolas-grekas Sep 10, 2019 •

edited

Loading

nicolas-grekas Sep 10, 2019

stof Sep 9, 2019

nicolas-grekas Sep 10, 2019

stof Sep 9, 2019

nicolas-grekas Sep 10, 2019

steevanb commented Nov 18, 2019

bradjones1 commented Jan 26, 2020

steevanb commented Jan 29, 2020

[DI] generate preload.php file for PHP 7.4 in cache folder #32032

[DI] generate preload.php file for PHP 7.4 in cache folder #32032

Conversation

nicolas-grekas commented Jun 13, 2019 • edited Loading

nicolas-grekas commented Jun 14, 2019

teohhanhui commented Jun 14, 2019

BenMorel commented Jun 14, 2019

nicolas-grekas commented Jun 14, 2019

BenMorel commented Jun 14, 2019

k00ni commented Jun 18, 2019 • edited Loading

nicolas-grekas commented Jun 18, 2019

BenMorel commented Jun 18, 2019 • edited Loading

teohhanhui commented Jun 18, 2019 • edited Loading

tsantos84 commented Jun 26, 2019 • edited Loading

tsantos84 commented Jun 26, 2019

nicolas-grekas commented Sep 8, 2019

fabpot commented Sep 8, 2019

stof Sep 9, 2019

Choose a reason for hiding this comment

nicolas-grekas Sep 10, 2019 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Sep 10, 2019

Choose a reason for hiding this comment

stof Sep 9, 2019

Choose a reason for hiding this comment

nicolas-grekas Sep 10, 2019

Choose a reason for hiding this comment

stof Sep 9, 2019

Choose a reason for hiding this comment

nicolas-grekas Sep 10, 2019

Choose a reason for hiding this comment

steevanb commented Nov 18, 2019

bradjones1 commented Jan 26, 2020

steevanb commented Jan 29, 2020

nicolas-grekas commented Jun 13, 2019 •

edited

Loading

k00ni commented Jun 18, 2019 •

edited

Loading

BenMorel commented Jun 18, 2019 •

edited

Loading

teohhanhui commented Jun 18, 2019 •

edited

Loading

tsantos84 commented Jun 26, 2019 •

edited

Loading

nicolas-grekas Sep 10, 2019 •

edited

Loading