-
-
Notifications
You must be signed in to change notification settings - Fork 197
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
The cached configuration is ignored #1297
Comments
FYI, this has come up before, but looks like it was auto-archived. I left some notes on why it is not working here: #885 (comment) |
@webbati have you ever noticed any performance improvements from caching the config? Fixing support for it in Winter while retaining support for per-environment configuration overrides is somewhat complicated, so I'd like to make sure it's actually worth doing before we go to the effort of fixing it. |
@LukeTowers, I thought it was obvious, but I just tested a 10-users simulation with Locust, and the performance seems better without cached configuration. Could I have done something wrong? Is there a better way to test this? with cache (avg 496ms) without cache (avg 357ms) |
@webbati Could be issues with our current implementation. Could you test on latest fresh install of just Laravel itself the performance of config:cache on and off? If there's a performance benefit there then potentially it could make sense to investigate in Winter, if not then it's probably overzealous premature optimization and we won't bother fixing it, we'll just remove it. |
@LukeTowers I did it, these are the results on a fresh install of Laravel 9 and 11 LARAVEL 9 without cache (avg 32.61) LARAVEL 11 without cache (avg 36.79) |
If I could hazard a guess, the reason Laravel changed their config functionality to load everything on initialization was precisely because they wanted to implement caching capabilities, so those benchmarks above don't surprise me. As mentioned with my comment in the other issue, the way our config is set up makes caching not workable, because we support multiple environments, and we don't know ahead of time all the config keys that will be used since configs can be required by plugins that may be enabled or disabled, and it's possible to define configuration at runtime (such as in a plugin's Arguably, we could cache the config as it's read in our current system, but all that would really do is save us a file read if the install is not using file cache. It'll likely be a micro-optimization at best. I feel our current system is probably the best for what we need it to accomplish, but happy to be proven wrong if someone can come up with a better system. |
Winter CMS Build
dev-develop
PHP Version
Other (please specify below)
Database engine
MySQL/MariaDB
Plugins installed
Winter.User, Winter.Blog,Winter.Builder, Winter.Translate
Issue description
Creating a cache file with
config:cache
command i expect that the new values will be cached and (of course) used by the system.If i open
config.php
cached file instorage/framework
i can see the correct values but if i don't fill fallback values in theenv
helper function i'm getting missing values error (for Encription key or database credentials for example)Running
config:clear
all come back to works well also if i don't fill the fallback values.Same behaviour with PHP 8.1.31 and PHP 8.2.16
Steps to replicate
Fill
.env
file with encription key and database values => All wok's fineRun
config.cache
artisan command => You get the errorsFill fallback values in the
env
helper function forkey
value in theapp.php
file and fordatabase
,password
andusername
keys inconnections->mysql
key in databsse.php file => All work's fineRemove the falback values that you have just added => You get the errors
Remove cache with
config:clear
artisan command => All work's fineWorkaround
Fill the fallback values of the env function in the interested files or clear the config cache and don't use it.
The text was updated successfully, but these errors were encountered: