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

Compile the container #494

Merged
merged 61 commits into from Jun 4, 2017

Conversation

Projects
None yet
3 participants
@mnapoli
Member

mnapoli commented May 28, 2017

3rd try at compiling the container (after #133 and #149).

This will bring massive performance improvements in production. To learn more I have included below the TODO list the updated documentation for the "Performances" section.

TODO :

  • compile most definitions:
    • values
    • string expressions
    • environment variables
    • aliases
    • arrays
    • array extensions
    • objects
    • lazy objects
    • sub-definitions
  • cover with tests definitions that are not compiled yet:
    • factories
    • decorators
    • wildcards
  • get()
    • basic implementation
    • don't skip singleton entries (already resolved)
    • circular dependencies
  • has()
  • make()
  • injectOn()
  • call()
  • delegate containers
  • all integration tests should cover the compiled container too
  • test the compiler
  • handle set() of already compiled entries
  • define the file name to store the compiled container (will allow to use several containers in the same process)
  • remove the cache completely (will be unnecessary)
  • benchmarks
  • documentation
  • changelog

Compiling the container

PHP-DI performs two tasks that can be expensive:

In order to avoid those two tasks, the container can be compiled into PHP code optimized especially for your configuration and your classes.

Setup

Compiling the container is as easy as calling the compile() method on the container builder:

$containerBuilder = new \DI\ContainerBuilder();
$containerBuilder->compile(__DIR__ . '/var/cache/CompiledContainer.php');

// […]

$container = $containerBuilder->build();

The compile() method takes a single argument: the name of a file in which to store the container.

Please note that the file name will also be the name of the generated PHP class. Because of that the filename you specify must also be a valid class name. For example:

  • var/cache/CompiledContainer.php: valid
  • var/cache/compiled-container.php: invalid since compiled-container is not a valid PHP class name

Deployment in production

When a container is configured to be compiled, it will be compiled once and never be regenerated again. That allows for maximum performances in production.

When you deploy new versions of your code to production you must delete the generated file to ensure that the container is re-compiled.

If your production handles a lot of traffic you may also want to generate the compiled container before the new version of your code goes live. That phase is known as the "warmup" phase. To do this, simply create the container (call $containerBuilder->build()) during your deployment step and the compiled container will be created.

Development environment

Do not compile the container in a development environment, else all the changes you make to the definitions (annotations, configuration files, etc.) will not be taken into account. Here is an example of what you can do:

$containerBuilder = new \DI\ContainerBuilder();
if (/* is production */) {
    $containerBuilder->compile(__DIR__ . '/var/cache/CompiledContainer.php');
}

As a side note, do not confuse "development environment" with your automated tests. You are encouraged to run your automated tests (PHPUnit, Behat, etc.) on a system as close to your production setup (which means with the container compiled).

Optimizing for compilation

As you can read in the "How it works" section, PHP-DI will take all the definitions it can find and compile them. That means that definitions like autowired classes that are not listed in the configuration cannot be compiled since PHP-DI doesn't know about them.

If you want to optimize performances to a maximum in exchange for more verbosity, you can let PHP-DI know about all the autowired classes by listing them in definition files:

return [
    // ... (your definitions)

    UserController::class => autowire(),
    BlogController::class => autowire(),
    ProductController::class => autowire(),
    // ...
];

You do not need to configure them (autowiring will still take care of that) but at least now PHP-DI will know about those classes and will compile their definitions.

Currently PHP-DI does not traverse directories to find autowired or annotated classes automatically. It also does not resolve wildcard definitions when it is compiled.

Please note that the following definitions are not compiled (yet):

Those definitions will still work perfectly, they will simply not get a performance boost when using a compiled container.

How it works

PHP-DI will read definitions from your configuration. When the container is compiled, PHP code will be generated based on those definitions.

For example let's take the definition for creating an object:

return [
    'Logger' => DI\create()
        ->constructor('/tmp/app.log')
        ->method('setLevel', 'warning'),
];

This definition will be compiled to PHP code similar to this:

$object = new Logger('/tmp/app.log');
$object->setLevel('warning');
return $object;

All the compiled definitions will be dumped into a PHP class (the compiled container) which will be written to a file (for example CompiledContainer.php).

At runtime, the container builder will see that the file CompiledContainer.php exists and will load it (instead of loading the definition files). That PHP file may contain a lot of code but PHP's opcode cache will cache that class in memory (remember to use opcache in production). When a definition needs to be resolved, PHP-DI will simply execute the compiled code and return the created instance.

@mnapoli mnapoli added the enhancement label May 28, 2017

@mnapoli mnapoli added this to the 6.0 milestone May 28, 2017

@mnapoli mnapoli self-assigned this May 28, 2017

mnapoli added some commits May 29, 2017

Do not require the covers annotation anymore in PHPUnit
It doesn't make sense. Either a feature is covered, either it isn't.
Compile to a specific file instead of a directory
That will allow to use several containers in the same process.

@mnapoli mnapoli changed the title from [WIP] Compile the container to Compile the container Jun 4, 2017

Forbid setting definitions at runtime on a compiled container
It needs to be forbidden because that would mean get() must go through the definitions every time, which kinds of defeats the performance gains of the compiled container.

@mnapoli mnapoli merged commit ab24212 into master Jun 4, 2017

6 checks passed

Scrutinizer 2 new issues, 31 updated code elements
Details
continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/styleci/push The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+13.5%) to 94.605%
Details

@mnapoli mnapoli deleted the compiler branch Jun 4, 2017

@sagikazarmark

This comment has been minimized.

Contributor

sagikazarmark commented Jun 4, 2017

Nice!

When does the actual compilation happen? When you call build or compile?

I am asking because I want to create Docker images with compiled containers and I don't want to compile them runtime.

@mnapoli

This comment has been minimized.

Member

mnapoli commented Jun 4, 2017

@sagikazarmark when ->build() is called

@mnapoli

This comment has been minimized.

Member

mnapoli commented Jun 4, 2017

@sagikazarmark By the way I'll probably be tagging an alpha release today, testing this new version (which comes with a big refactoring) would be most welcome :)

@sagikazarmark

This comment has been minimized.

Contributor

sagikazarmark commented Jun 4, 2017

Will do here: https://github.com/nofw/nofw

@sagikazarmark

This comment has been minimized.

Contributor

sagikazarmark commented Jun 4, 2017

BTW this is how I built the cache so far, I guess compilation will be less hacky: https://github.com/nofw/nofw/blob/master/bin/cache#L9

@mnapoli

This comment has been minimized.

Member

mnapoli commented Jun 4, 2017

Yes I hope this will be useful for you! I've tagged a new alpha, you can test it right away (feel free to open issues if necessary): https://github.com/PHP-DI/PHP-DI/releases/tag/6.0.0-alpha2

The documentation: https://github.com/PHP-DI/PHP-DI/blob/master/doc/performances.md

mnapoli added a commit that referenced this pull request Jun 4, 2017

Fix #498 and #488
In PHP-DI 5, definitions could be nested in some places (e.g. use a get() in an object definition, etc.). However it did not behave everywhere the same, for example it didn't work for sub-definitions in arrays.

Now in PHP-DI 6 all nested definitions will all be recognized and resolved correctly everywhere. Since #494 (compiled container) performance will not be affected so we can implement a more robust behavior.

mnapoli added a commit that referenced this pull request Jun 4, 2017

Fix #498 and #488 Standardize resolution of nested definitions everyw…
…here

In PHP-DI 5, definitions could be nested in some places (e.g. use a get() in an object definition, etc.). However it did not behave everywhere the same, for example it didn't work for sub-definitions in arrays.

Now in PHP-DI 6 all nested definitions will all be recognized and resolved correctly everywhere. Since #494 (compiled container) performance will not be affected so we can implement a more robust behavior.
@juliangut

This comment has been minimized.

Contributor

juliangut commented Jun 5, 2017

@mnapoli I've an issue with the nomenclature of compile. IMHO the name misleads as compile is an action (verb) while the method does not really compile anything but sets the compiled file path (and name). Thus I'd suggest to rename it to setCompilationPath instead (more on this below)

Another problem I see as per documentation:

Please note that the file name will also be the name of the generated PHP class. Because of that the filename you specify must also be a valid class name

Wouldn't it be better just to use a single file name (CompiledContainer.php is just perfect) and let the user set the path only? (this plays better with my previous suggestion of setCompilationPath)

@sagikazarmark

This comment has been minimized.

Contributor

sagikazarmark commented Jun 5, 2017

Hm, I kinda agree with the compile name thing.

Not sure about the path one.

@mnapoli

This comment has been minimized.

Member

mnapoli commented Jun 10, 2017

@juliangut & @sagikazarmark thanks I've opened #506

Sorry for not answering sooner, I was at a conference these last days.

mnapoli added a commit that referenced this pull request Jun 11, 2017

Compile factories
Factory definitions are now dumped into the compiled container!

This follows #494 where all definitions but factories, decorators and wildcards were compiled. Decorators and wildcards are still not compiled.

Compiling factories means also compiling closures. That was actually much more doable than I thought, I've used [Roave/BetterReflection](https://github.com/Roave/BetterReflection) (thanks asgrim and ocramius!)

The main downside of the whole pull request is the number of dependencies that BetterReflection brings:

  - Installing symfony/polyfill-util (v1.4.0) Loading from cache
  - Installing symfony/polyfill-php56 (v1.4.0) Loading from cache
  - Installing nikic/php-parser (v2.1.1) Loading from cache
  - Installing jeremeamia/superclosure (2.3.0) Loading from cache
  - Installing zendframework/zend-eventmanager (3.1.0) Loading from cache
  - Installing zendframework/zend-code (3.1.0) Loading from cache
  - Installing phpdocumentor/reflection-common (1.0) Loading from cache
  - Installing phpdocumentor/type-resolver (0.2.1) Loading from cache
  - Installing phpdocumentor/reflection-docblock (2.0.5) Loading from cache
  - Installing roave/better-reflection (1.2.0) Loading from cache

Maybe we could get on with superclosure only. Or else make compiling closures optional (only compile them if BetterReflection is installed…). Also I've seen that there is a v2.0 of BetterReflection in development [but it requires 7.1](https://github.com/Roave/BetterReflection#changes-in-br-20), I have no idea for how long the v1 will be supported (e.g. will it support PHP 7.2). PHP-DI could upgrade to 7.1 at the end of this year but that's not for sure yet.

Performance improvements: -20% on the factory.php benchmark, [profile](https://blackfire.io/profiles/compare/6692ca3e-521a-49df-96c6-1bf9833c8209/graph), which is very good for a micro-benchmark IMO. I'll test all that in larger scenarios for the complete 6.0 release but it's safe to say this PR is an improvement.

There are also a lot of micro/not-so-micro optimisations possible here in the future, especially because there are a lot of parameter-guessing done at runtime (you can inject dependencies into the factories and PHP-DI figures it out with type-hints). https://github.com/PHP-DI/Invoker could be compiled in the future (optionally), and simpler scenarios (classic closures) could be much more optimized than that. Work for later! :)

Because of closures, some scenarios are explicitly not supported:

- you should not use `$this` inside closures
- you should not import variables inside the closure using the `use` keyword, like in `function () use ($foo) { ...`
- you should not define multiple closures on the same line

But those scenarios don't make sense in container factories written in array config so that's good!

mnapoli added a commit that referenced this pull request Jun 11, 2017

Compile factories
Factory definitions are now dumped into the compiled container!

This follows #494 where all definitions but factories, decorators and wildcards were compiled. Decorators and wildcards are still not compiled.

Compiling factories means also compiling closures. That was actually much more doable than I thought, I've used [Roave/BetterReflection](https://github.com/Roave/BetterReflection) (thanks asgrim and ocramius!)

The main downside of the whole pull request is the number of dependencies that BetterReflection brings:

  - Installing symfony/polyfill-util (v1.4.0) Loading from cache
  - Installing symfony/polyfill-php56 (v1.4.0) Loading from cache
  - Installing nikic/php-parser (v2.1.1) Loading from cache
  - Installing jeremeamia/superclosure (2.3.0) Loading from cache
  - Installing zendframework/zend-eventmanager (3.1.0) Loading from cache
  - Installing zendframework/zend-code (3.1.0) Loading from cache
  - Installing phpdocumentor/reflection-common (1.0) Loading from cache
  - Installing phpdocumentor/type-resolver (0.2.1) Loading from cache
  - Installing phpdocumentor/reflection-docblock (2.0.5) Loading from cache
  - Installing roave/better-reflection (1.2.0) Loading from cache

Maybe we could get on with superclosure only. Or else make compiling closures optional (only compile them if BetterReflection is installed…). Also I've seen that there is a v2.0 of BetterReflection in development [but it requires 7.1](https://github.com/Roave/BetterReflection#changes-in-br-20), I have no idea for how long the v1 will be supported (e.g. will it support PHP 7.2). PHP-DI could upgrade to 7.1 at the end of this year but that's not for sure yet.

Performance improvements: -20% on the factory.php benchmark, [profile](https://blackfire.io/profiles/compare/6692ca3e-521a-49df-96c6-1bf9833c8209/graph), which is very good for a micro-benchmark IMO. I'll test all that in larger scenarios for the complete 6.0 release but it's safe to say this PR is an improvement.

There are also a lot of micro/not-so-micro optimisations possible here in the future, especially because there are a lot of parameter-guessing done at runtime (you can inject dependencies into the factories and PHP-DI figures it out with type-hints). https://github.com/PHP-DI/Invoker could be compiled in the future (optionally), and simpler scenarios (classic closures) could be much more optimized than that. Work for later! :)

Because of closures, some scenarios are explicitly not supported:

- you should not use `$this` inside closures
- you should not import variables inside the closure using the `use` keyword, like in `function () use ($foo) { ...`
- you should not define multiple closures on the same line

But those scenarios don't make sense in container factories written in array config so that's good!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment