Skip to content
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

Clean-up OPcache configurations #1661

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

MelvynEzi
Copy link

@MelvynEzi MelvynEzi commented Nov 13, 2022

This PR serves to clean-up the OPcache configurations in opcache.ini.sh.

Removed configurations

These configurations were removed as they are already the defaults in PHP (see PHP OPcache configuration docs):

opcache.enable=1
opcache.max_accelerated_files=10000
opcache.save_comments=1
opcache.memory_consumption=128

These configurations were also removed, with justifications:

opcache.fast_shutdown = 1

opcache.fast_shutdown directive was removed since PHP 7.2.0 (see opcache.fast_shutdown configuration). Similar PR:

opcache.enable_cli=1

opcache.enable_cli=1 was removed from Nextcloud's recommended configurations due to it being bad for performance. See issue:

The recommendation to set opcache.enable_cli=1 is actually bad for performance.


Recommended configurations

These are recommended configurations (see Enabling PHP OPcache - Nextcloud docs):

opcache.save_comments = 1

Nextcloud strictly requires code comments to be preserved in opcode, which is the default.

opcache.save_comments is required by Nextcloud. However, it is also the default value in PHP, therefore removed.

opcache.revalidate_freq = 60

OR

opcache.validate_timestamps = 0

Since Nextcloud handles cache revalidation internally when required, the revalidation frequency can be reduced or completely disabled to enhance performance.

opcache.revalidate_freq is recommended to be set to '60' to revalidate cached scripts every 60 seconds (as opposed to the 2 seconds default in PHP). The revalidation frequency can be disabled by setting opcache.validate_timestamps to '0'.

Other recommended configuration by Nextcloud (undocumented):

opcache.interned_strings_buffer=16

Nextcloud have increased opcache.interned_strings_buffer from '8' to '16' to avoid the following error message:

The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply opcache.interned_strings_buffer to your PHP configuration with a value higher than 8.

See PR:


Personally, I have tested the configuration by modifying /etc/php/8.1/mods-available/opcache.ini and there were no issues after restarting the service.

However, if any issues arises after restarting the service, I would recommend to clear the file cache in /tmp and then restarting the service.

Note that I have tested this on a Docker instance instead of a bare metal installation.

@MelvynEzi MelvynEzi changed the base branch from master to devel January 8, 2023 05:29
@delete-merged-branch delete-merged-branch bot deleted the branch nextcloud:devel January 8, 2023 20:50
@MelvynEzi
Copy link
Author

Hey @theCalcaholic, noticed that OPcache configurations in opcache.ini.sh are being changed in the recent commits. Hope you are willing to check out this PR for some OPcache configuration. 🙏🏻

@theCalcaholic
Copy link
Collaborator

theCalcaholic commented Jan 19, 2023

Hey @DJCrapsody, thank you for the PR!

I'm only changing the bare minimum right now, to a) try to fix the issue with JIT segfaults, that's plaguing us since PHP 8.1 and b) remove warnings in NC 25.

This release is not about opcache specifically and unfortunately I didn't have time to review your PR yet.

Your changes, once approved, will be included in a later release (probably 1.51.1 or 1.51.2).

In the meantime, can you verify if your changes are backwards-compatible to at least PHP 7.4?
Not everyone is on PHP 8 yet (as this is tight to the NC and debian version they're using).

EDIT: Actually, we do need compatibility with PHP 7.3, even, as some people are still on that version.

@delete-merged-branch delete-merged-branch bot deleted the branch nextcloud:devel January 22, 2023 00:56
@delete-merged-branch delete-merged-branch bot deleted the branch nextcloud:devel January 22, 2023 13:10
@delete-merged-branch delete-merged-branch bot deleted the branch nextcloud:devel January 22, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants