[5.2] [WIP] Fixing dotenv loading

[vlakoff]

Follow-up to #13351. Please note this is very much a WIP and may have important caveats.

Important: we would lose app.env config. But that may be a good thing, things get mixed up because of this setting. Better use app()->environment().

[GrahamCampbell]

We don't want to load anything if w have cached config for performance reasons.

[GrahamCampbell]

Only ever read env variables from inside config files.

[vlakoff]

• I'm pretty sure there are people who define custom env variables, and use them outside from config files. They don't encounter errors, up to the day they activate config caching...
• Do you have more details about config loading performances? I guess dotenv isn't a bottleneck amongst this.
• The issue is still to be resolved. If you can think of a fix, please be my guest :)

[GrahamCampbell]

I'm pretty sure there are people who define custom env variables, and use them outside from config files.

That is not allowed anymore for performance reasons, and it would be a bad practice anyway.

[GrahamCampbell]

Do you have more details about config loading performances? I guess dotenv isn't a bottleneck amongst this.

Yes, it is a bottleneck. We found a 25% throughput increase on the out of the box app when applying these optimizations when prepping for the 5.2 release.

[GrahamCampbell]

The issue is still to be resolved. If you can think of a fix, please be my guest :)

I don't agree that there exists an issue. Sorry.

[vlakoff]

I meant performances of dotenv in particular, amongst the sum of config files + dotenv.

[GrahamCampbell]

I meant performances of dotenv in particular

Yes, config caching with dotenv vs config cahing without dotenv was the 25% different. The change was actually larger than having config caching enabled!

[GrahamCampbell]

Summary: dotenv is REALLY slow because it means we have to do disk IO and lots of CPU time. PHP already has internal optimization for loading compiled php files into memory, so requiring php files is inexpensive, That's also why we made the services.json -> services.php change.

[GrahamCampbell]

Dotenv also has concurrency bugs, and is not suitable for production use. It should only be used to initialize config files like we're using it for in L5.2 really. For that kind of use it's safe.

[vlakoff]

app->environment() tries all of these:

1) --env argument
2) callback
    2.1) app.env config
        2.1.1) APP_ENV
        2.1.2) fallback to "production"

Whereas DetectEnvironment::checkForSpecificEnvironmentFile tries only 2.1.1 from the above (and in a kind of hardcoded way).

As showed in #13351, this is a limitation. Also, this is inconsistent and could lead to other bugs.

[vlakoff]

Thanks for the above explanations concerning dotenv.

[vlakoff]

No plans to improve this? It's complicated because env and config loading depend on each other…