Skip to content
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

Documentation enableCompilation incomplete #629

Open
sridesmet opened this issue Sep 18, 2018 · 12 comments
Open

Documentation enableCompilation incomplete #629

sridesmet opened this issue Sep 18, 2018 · 12 comments

Comments

@sridesmet
Copy link

I am puzzled how to use the enableCompilation() function after reading through the documentation. The documentation states you only have to call enableCompilation(), but for optimum speed you don't want to add the definitions after reading in the compilation file?

Isn't it an if-else story? E.g

if (getenv('ENABLECOMPILATION') == 'true') {
        $builder->enableCompilation(__DIR__ . '/tmp/dicompilation');
      } else {
        $builder->addDefinitions([...]);
      }

instead of:

$builder->enableCompilation(__DIR__ . '/tmp/dicompilation');
$builder->addDefinitions([...]);
@mnapoli
Copy link
Member

mnapoli commented Sep 18, 2018

The compilation will prevent to evaluate the definitions when fetching entries.

So you still need to add the definitions every time (the container is actually compiled when ->build() is called). Those definitions will be added but not processed when the container is already compiled.

If you want to save even more time, you can store those definitions in separate files and call ->addDefinitions('file-name.php'), that way the file will not even be read if the definitions are all compiled.

I'll keep this issue open, it might be good to improve the documentation.

@sridesmet
Copy link
Author

sridesmet commented Sep 19, 2018

So you still need to add the definitions every time

Hmm, I'm still puzzled why my code works. At build phase I create the compiled di-file. At run-time I check with an environment variable if I add the compilation file or if I add the definitions. This setup doesn't break any code.

Even better, the compilationfile includes all my environment variables (whom I load through dotenv). Which is why I can eliminate the reading of my .env files in production.

Note: I use the php-di/slim bridge.

@mnapoli
Copy link
Member

mnapoli commented Sep 19, 2018

the compilationfile includes all my environment variables (whom I load through dotenv). Which is why I can eliminate the reading of my .env files in production.

Note that environment variables are still read with the compiled container. That allows to change environment variables after the container has been compiled and those changes will be picked up.

At run-time I check with an environment variable if I add the compilation file or if I add the definitions. This setup doesn't break any code.

Yes in your setup that might work because everything is compiled. If you use Container::make() or non-compilable definitions (e.g. wildcards) those will break. This is why I recommend to always add the definitions.

@sridesmet
Copy link
Author

sridesmet commented Sep 19, 2018

Note that environment variables are still read with the compiled container. That allows to change environment variables after the container has been compiled and those changes will be picked up.

So if I precompile the following DI-entry, the environment variables can still be changed after the compilation step?

ApiService::class => \DI\create()->constructor(
    \DI\get(Helperclass::class),
    getenv('API_KEY')
  )

Because in the precompiled file I can see the environment variables hardcoded in the constructor. Which is not a problem, but helpful to know in CI-environments.

Yes in your setup that might work because everything is compiled. If you use Container::make() or non-compilable definitions (e.g. wildcards) those will break. This is why I recommend to always add the definitions.

That clarifies alot! Thank you.

@mnapoli
Copy link
Member

mnapoli commented Sep 19, 2018

In the compiled file you should still see getenv() calls, the value should not be inlined. For information here is the code that is generated: https://github.com/PHP-DI/PHP-DI/blob/master/src/Compiler/Compiler.php#L178

As you can see getenv() is still there in the compiled code.

@sridesmet
Copy link
Author

sridesmet commented Sep 19, 2018

Hmm, that's not what I see:
CompiledContainer.php:

const METHOD_MAPPING = array (
  'App\\Helpers\\PDOHelper' => 'get5ba25da2f3900571311313',
...
protected function get5ba25da2f3900571311313()
    {
        $object = new App\Helpers\PDOHelper('mysql:host=localhost;dbname=mydbname;charset=utf8', 'testuser', '', 'utf8');
        return $object;
    }

And in the definition file:

PDOHelper::class => \DI\create()->constructor(
    getenv('DATABASE_DSN'),
    getenv('DATABASE_USER'),
    getenv('DATABASE_PASSWORD'),
    getenv('DATABASE_CHARACSET')
  ),

A search in the CompiledContainer.php for getenv has no results.

@mnapoli
Copy link
Member

mnapoli commented Sep 19, 2018

OK I see, you are using getenv() to get environment values. This function is the native php function, it is executed immediately when the config is loaded. Instead you should use DI\env() (see http://php-di.org/doc/php-definitions.html#environment-variables)

@sridesmet
Copy link
Author

My bad, I did not know the DI\env() function. It all makes sense now. 👍

@deftbrain
Copy link

deftbrain commented Sep 28, 2018

If you want to save even more time, you can store those definitions in separate files and call ->addDefinitions('file-name.php'), that way the file will not even be read if the definitions are all compiled.

Could you clarify the moment related to reading the file, please?
I have the following:

$this->container = (new ContainerBuilder())
    ->enableCompilation(static::cacheDir())
    ->addDefinitions(static::configDir() . '/definitions.php')
    ->build();

I use the container for creating objects (using autowiring) that should not be stored inside the container, but that have dependencies defined in definitions.php:
$module = $this->container->make($moduleClassName, ['id' => $id, 'name' => $moduleName]);
I've checked the content of the compiled container (CompiledContainer.php) and see compiled definitions for all dependencies declared in definitions.php. But when the make() mentioned above is called, I see reading of the file definitions.php in the following stacktrace:

definitions.php:5, require()
DefinitionFile.php:59, DI\Definition\Source\DefinitionFile->initialize()
DefinitionFile.php:38, DI\Definition\Source\DefinitionFile->getDefinition()
SourceChain.php:54, DI\Definition\Source\SourceChain->getDefinition()
Container.php:153, CompiledContainer->getDefinition()
Container.php:186, CompiledContainer->make()

Why does it read the file?
I use php-di/php-di: 6.0.5

Thanks in advance!

@mnapoli
Copy link
Member

mnapoli commented Sep 28, 2018

@deftbrain ah right, if you use make or any entry that isn't compiled (your autowired entries here) then PHP-DI will check in the definition files if it can find them. So that's why the file is read.

@deftbrain
Copy link

@mnapoli, could you explain in more detail, please, what is the reason to read definition files again if they are already transformed into a compiled container? Is there a case when some definition from a file will not be passed into the compiled container?

@mnapoli
Copy link
Member

mnapoli commented Oct 4, 2018

@deftbrain you can read more about it here (http://php-di.org/doc/performances.html#optimizing-for-compilation) but basically some definitions cannot be compiled. For example wildcard definitions, or autowired definitions that are not declared in config files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants