Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Opcache should be disabled by default #129

Closed
angrybrad opened this issue May 10, 2020 · 19 comments
Closed

Opcache should be disabled by default #129

angrybrad opened this issue May 10, 2020 · 19 comments

Comments

@angrybrad
Copy link
Member

Usually ends up being a pain for local development

@bencroker
Copy link
Contributor

So that explains why file changes often don't take effect immediately, +1 for this!

@jasonmccallister
Copy link
Member

jasonmccallister commented May 11, 2020

For my own curiosity, before we just blanket remove OPCache, can we tweak a setting which would tell OPCache to use the files timestamp for validation and see if we are still seeing the need for a multiple refreshes to see changes?

@bencroker can you ssh into nitro (nitro ssh) and edit the file:

sudo vi /etc/php/7.4/fpm/conf.d/10-opcache.ini

Add the the following into the file:

opcache.validate_timestamps=1

Finally restart php-fpm using:

sudo service php7.4-fpm restart

If you are logged into your control panel, under Utilities > PHP Info you should see the following:

opcache.validate_timestamps      On

@bencroker
Copy link
Contributor

@jasonmccallister Without doing this, I see that opcache.validate_timestamps is already set to On.

Screenshot 2020-05-11 at 13 28 08

OPCache does appear to be validating timestamps, as the changes are detected, however the changes are sometimes detected immediately and sometimes takes up to a few seconds to take effect, which is rather frustrating during development.

@jasonmccallister
Copy link
Member

@bencroker you are right, can you instead add this setting:

opcache.revalidate_freq=0

and then restart php-fpm?

@bencroker
Copy link
Contributor

bencroker commented May 11, 2020

Changed opcache.revalidate_freq from 2 to 0 and restarted php-fpm. This doesn't appear to have helped though. Neither does opcache.enable=0 so I don't think OPCache is the culprit here.

Could it be that when I edit a file, it takes some time for Nitro to detect and mount the modified file into Multipass?

@bencroker
Copy link
Contributor

bencroker commented May 11, 2020

Tested with a PHP file and changes take effect immediately.

So, this appears to have to do with compiled twig templates being cached. I've submitted craftcms/cms#6046 to Craft core which should help resolve this.

As per my tests, with the PR above, OPCache can remain enabled as it is.

@khalwat
Copy link

khalwat commented May 11, 2020

What would the benefit be of having opcache available in a local dev environment?

@jasonmccallister
Copy link
Member

It is faster and when @bencroker and I were troubleshooting, there was still a delay in the twig template rendering. His PR will resolve that when using devMode.

@khalwat
Copy link

khalwat commented May 11, 2020

Right, I know opcache is faster but for anything caching, I'd want to opt-in for local development, I'd think?

Re: the PR @bencroker my question would be... why does this only show up when doing local dev with Nitro?

@bencroker
Copy link
Contributor

@khalwat Possibly because of the way Nitro mounts the directory, but that's more of a question for @jasonmccallister.

@jasonmccallister
Copy link
Member

@khalwat I think it surfaced in Nitro but I have seem the exact same behavior when running Craft in Docker.

@bencroker
Copy link
Contributor

craftcms/cms#6046 was rejected so this issue remains unresolved.

@brandonkelly
Copy link
Member

brandonkelly commented May 11, 2020

Twig determines whether a template needs to be recompiled by comparing the template file’s modify time with the compiled PHP file’s modify time. But with OPcache enabled, the new template file modify time might not be immediately available, as OPcache will return the cached modify time instead.

When you disable Twig “caching”, you’re disabling its ability to save the compiled templates as PHP files for future requests, which does technically solve the OPcache conflict (no precompiled files to compare against in the first place), but you’d be throwing the baby out with the bathwater; having Twig save compiled templates as PHP files is a huge timesaver, versus recompiling templates on every request.

If we have to choose one, OPcache is the one that should be disabled, not Twig caching.

@jasonmccallister
Copy link
Member

Hmm, would be interesting to add a custom loader that does not use a filesystem: https://twig.symfony.com/doc/3.x/api.html#create-your-own-loader

@jasonmccallister
Copy link
Member

Either way, will make those changes for the next beta release.

@brandonkelly
Copy link
Member

We do use our own loader, but that wouldn’t solve this problem. That’s just for loading the actual template files, which are always going to be file-based, so will suffer from this problem as long as OPcache is enabled.

@jasonmccallister
Copy link
Member

This will be released with beta 3.

@jskrivanek
Copy link

@jasonmccallister Was this released with beta 3?

@jasonmccallister
Copy link
Member

Hi @jskrivanek this was released with a beta of Nitro v1. We are actively working on Nitro V2 that is based on Docker instead of Multipass. If you are referring to v2 can you please create a new issue and describe any issues you are having?

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

No branches or pull requests

6 participants