Skip to content

Conversation

@MichaIng
Copy link
Member

@MichaIng MichaIng commented Jun 10, 2025

Summary

For the overall OPcache size check, we currently compare used memory with free memory. However, opcache.memory_consumption is split into used_memory, free_memory and wasted_memory. When cached files change on disk, old entries are not replaced or removed, but remain as wasted memory, until the cache is actually full, and if their percentage is above opcache.max_wasted_percentage, which is 5% by default. When this happens, the engine is restarted, resetting the cache completely, like a opcache_reset() call.

As long as we do not consider wasted cache, recommendations based on free memory can be false. To solve this, we could count wasted memory as free memory, if it is above opcache.max_wasted_percentage, as the engine will be restarted as soon as needed, freeing up this wasted space. On the other hand, wasted memory below the threshold permanently blocks the OPcache, which supports counting it as used memory. Depending on the situation, instead of raising OPcache size, it could be also advised to reduce opcache.max_wasted_percentage. But too frequent cache resets break its purpose as well.

In my opinion, the matter is too complex to consider wasted cache correctly, and do precise recommendations, but we should focus on reducing false positives instead. What we know for sure is: if the cache is full ($status['cache_full'] === true), and the limit for cached keys has not been reached, the OPcache was too small to maintain free space, with wasted memory below the configured threshold, where it consumes memory permanently. Recommending to raise the OPcache size in this case, is hence as accurate as it gets. Even if 5% wasted cache could be freed, 95% used memory is still above the previous threshold for the setup check warning. And if opcache.max_wasted_percentage is above 5%, then the admin must have decided to change the default, deciding that system memory consumption has lower priority than preventing OPcache engine restarts.

cache_full can be true as well if the limit for cached keys has been reached, hence we need to merge both checks. In this case num_cached_keys equals max_cached_keys exactly, hence it is easy to differentiale whether opcache.max_accelerated_files or opcache.memory_consumption needs to be raised to address the cache_full state.

In practice, this change relaxes the checks: the respective limit needs to be reached 100% instead of 90%, to trigger a warning, eliminating also false alarms if a large share of the cache is consumed by wasted memory, which would be automatically freed once cache is 100% full.

Additionally, the recommendation for raising opcache.max_accelerated_files now says "a value higher than max_cached_keys", instead of "higher than opcache.max_accelerated_files". The actual limit, reflected by max_cached_keys from opcache_get_status(), is a next higher value from a set of prime numbers. E.g. if opcache.max_accelerated_files is set to 10,000 (PHP default), the effective limit is 16,229 OPcache keys. Recommending "higher than 10000" could hence lead to a settings change without effect. For an effective change, the new value needs to be "higher than 16229" instead, which is what the setup check will show in this situation, with this change applied.

Checklist

@MichaIng MichaIng added this to the Nextcloud 32 milestone Jun 10, 2025
@MichaIng MichaIng requested a review from a team as a code owner June 10, 2025 17:46
@MichaIng MichaIng requested review from artonge, nfebe and provokateurin and removed request for a team June 10, 2025 17:46
@MichaIng MichaIng force-pushed the enh/opcache-checks branch from d2cb8a7 to 9ecd03f Compare June 10, 2025 18:12
@artonge artonge requested review from come-nc June 17, 2025 14:33
@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

For the overall OPcache size check, we currently compare used memory with free memory. However, `opcache.memory_consumption` is split into `used_memory`, `free_memory` and `wasted_memory`. When cached files change on disk, old entries are not replaced or removed, but remain as wasted memory, until the cache is actually full, and if their percentage is above `opcache.max_wasted_percentage`, which is 5% by default. When this happens, the engine is restarted, resetting the cache completely, like a `opcache_reset()` call.

As long as we do not consider wasted cache, recommendations based on free memory can be false. To solve this, we could count wasted memory as free memory, if it is above `opcache.max_wasted_percentage`, as the engine will be restarted as soon as needed, freeing up this wasted space. On the other hand, wasted memory below the threshold permanently blocks the OPcache, which supports counting it as used memory. Depending on the situation, instead of raising OPcache size, it could be also advised to reduce `opcache.max_wasted_percentage`. But too frequent cache resets break its purpose as well.

In my opinion, the matter is too complex to consider wasted cache correctly, and do precise recommendations, but we should focus on reducing false positives instead. What we know for sure is: if the cache is full (`$status['cache_full'] === true`), and the limit for cached keys has not been reached, the OPcache was too small to maintain free space, with wasted memory below the configured threshold, where it consumes memory permanently. Recommending to raise the OPcache size in this case, is hence as accurate as it gets. Even if 5% wasted cache could be freed, 95% used memory is still above the previous threshold for the setup check warning. And if `opcache.max_wasted_percentage` is above 5%, then the admin must have decided to change the default, deciding that system memory consumption has lower priority than preventing OPcache engine restarts.

`cache_full` can be true as well if the limit for cached keys has been reached, hence we need to merge both checks. In this case `num_cached_keys` equals `max_cached_keys` exactly, hence it is easy to differentiale whether `opcache.max_accelerated_files` or `opcache.memory_consumption` needs to be raised to address the `cache_full` state.

In practice, this change relaxes the checks: the respective limit needs to be reached 100% instead of 90%, to trigger a warning, eliminating also false alarms if a large share of the cache is consumed by wasted memory, which would be automatically freed once cache is 100% full.

Additionally, the recommendation for raising `opcache.max_accelerated_files` now says "a value higher than `max_cached_keys`", instead of "higher than `opcache.max_accelerated_files`". The actual limit, reflected by `max_cached_keys` from `opcache_get_status()`, [is a next higher value from a set of prime numbers](https://www.php.net/manual/en/opcache.configuration.php#ini.opcache.max-accelerated-files). E.g. if `opcache.max_accelerated_files` is set to 10,000 (PHP default), the effective limit is 16,229 OPcache keys. Recommending "higher than 10000" could hence lead to a settings change without effect. For an effective change, the new value needs to be "higher than 16229" instead, which is what the setup check will show in this situation, with this change applied.

Signed-off-by: MichaIng <micha@dietpi.com>
@MichaIng MichaIng force-pushed the enh/opcache-checks branch from 8a03fff to 0d10c4f Compare June 25, 2025 13:12
@susnux susnux merged commit 54f55a1 into master Jul 1, 2025
194 checks passed
@susnux susnux deleted the enh/opcache-checks branch July 1, 2025 12:26
@szaimen
Copy link
Contributor

szaimen commented Jul 3, 2025

@MichaIng thanks a lot for thr PR! :)
Can we backport this?

@MichaIng
Copy link
Member Author

MichaIng commented Jul 3, 2025

Sure! Wasn't sure whether it is suitable for backporting, as it does not really fix a bug, and existing Nextcloud instances will have mostly adjusted the OPcache settings, whether it might be above the now needed value or not.

But I see there are always cases/reports, and if it helps for some, maybe also container appliances until NC 32 release, maybe worth it.

@MichaIng
Copy link
Member Author

MichaIng commented Jul 3, 2025

/backport to stable31

@MichaIng
Copy link
Member Author

MichaIng commented Jul 3, 2025

/backport to stable30

@skjnldsv skjnldsv mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
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.

Consider "wasted memory" for OPcache setup checks

5 participants