-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Disable opcache if no SHM backend is available #19350
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
zps_startup_failure("No available SHM backend", NULL, accelerator_remove_cb); | ||
/* Do not abort PHP startup */ | ||
return SUCCESS; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Whitespace that isn't consistent with rest of switch cases. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get. You compile opcache into PHP just to say it can't work on this platform...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dstogov This comes after https://wiki.php.net/rfc/make_opcache_required. The aim is to consistently provide opcache without having to install it separately, and without having to load the extension, but keep PHP compatible with rare systems that don't support any of Opcache's backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, for these systems we provide a feature that can't work and just waste resources.
Note that compilation of opcache for these systems may be a problem as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory we could at least make the file cache work, at least when opcache.file_cache
is specified. In the long term, always building opcache will help reduce complexity and some inconsistencies, even if it ends up being disabled as in this PR, or by opcache.enable(_cli)=0.
I've measured the overhead of building opcache: It is about 110KiB of .text
in a -Os
build. The total size of the .text
section in a minimal PHP build is about 6MiB.
Note that compilation of opcache for these systems may be a problem as well.
I'm happy making changes to address these problems.
Currently,
./configure
fails when no SHM backend is available (we support a wide range of systems with mmap(MAP_ANONYMOUS), shmget(), shm_open(), but some non-standard systems may not have any of these). Additionally, even after bypassing the configure check, Opcache emits a fatal error if no SHM backend is available.Changes in this PR:
cc @krakjoe @derickr