Remove the opcache sapi whitelist #19351
Conversation
| } | ||
|
|
||
| return FAILURE; | ||
| return strcmp(sapi_module.name, "cli") == 0 |
There was a problem hiding this comment.
Not that it's terribly relevant to have opcache active for php -i or other commands that don't compile php files, but is there a reason that it must be disabled? It should never even get to a opcache check on that path.
Just a question, trying to read into the source. cli-server is e.g. php some_cli_script.php, right?
There was a problem hiding this comment.
cli-server is the builtin web server: https://www.php.net/manual/en/features.commandline.webserver.php
cli is php some_cli_script.php.
Opcache is disabled by default on cli because populating a cache in this context will just consume more memory for no benefits (as the cache lives only for the duration of a single "request"), unless the file_cache is used. It is possible to enable opcache in cli by setting opcache.enable_cli=1. This can be beneficial on long running scripts due to opcode optimizations, or when the file_cache is enabled.
There was a problem hiding this comment.
I see, the if statement will have to be changed then, though:
if (!ZCG(accel_directives).enable_cli && accel_sapi_is_cli()) {Would enable it for phpdbg when enable_cli is enabled. Or is that intended?
There was a problem hiding this comment.
This is intended :) Opcache is disabled by default in phpdbg for the same reasons.
| @@ -2841,39 +2841,10 @@ static void zps_startup_failure(const char *reason, const char *api_reason, int | |||
| zend_llist_del_element(&zend_extensions, NULL, (int (*)(void *, void *))cb); | |||
| } | |||
|
|
|||
| static inline zend_result accel_find_sapi(void) | |||
| static inline bool accel_sapi_is_cli(void) | |||
There was a problem hiding this comment.
| static inline bool accel_sapi_is_cli(void) | |
| static inline bool disable_accel_for_sapi(void) |
or something like that, considering that it's not only cli sapi but also phpdbg.
Opcache has a whitelist of supported SAPIs. When the SAPI is not in the list, Opcache behaves as if
opcache.enable=0was specified.@dstogov do you remember why this list was needed? Is this still relevant? Can we remove it?