Skip to content

Remove the opcache sapi whitelist #19351

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arnaud-lb
Copy link
Member

Opcache has a whitelist of supported SAPIs. When the SAPI is not in the list, Opcache behaves as if opcache.enable=0 was specified.

@dstogov do you remember why this list was needed? Is this still relevant? Can we remove it?

}

return FAILURE;
return strcmp(sapi_module.name, "cli") == 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

@dstogov
Copy link
Member

dstogov commented Aug 5, 2025

@dstogov do you remember why this list was needed? Is this still relevant? Can we remove it?

Opcache is a successor of closed-source Zend commercial product, that wasn't supported on all available at that time SAPIs. Probably, this is not necessary any more.

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

Successfully merging this pull request may close these issues.

4 participants