feat(settings): simplify OPcache checksenh/opcache-checks

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>

1 files changed, 12 insertions, 14 deletions

diff --git a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php
index 22605012058..ee2e6e57335 100644
--- a/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php
+++ b/apps/settings/lib/SetupChecks/PhpOpcacheSetup.php
@@ -57,7 +57,7 @@ class PhpOpcacheSetup implements ISetupCheck {
 		} elseif ($this->iniGetWrapper->getBool('opcache.file_cache_only')) {
 			$recommendations[] = $this->l10n->t('The shared memory based OPcache is disabled. For better performance, it is recommended to apply "opcache.file_cache_only=0" to your PHP configuration and use the file cache as second level cache only.');
 		} else {
-			// Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks
+			// Check whether opcache_get_status has been explicitly disabled and in case skip usage based checks
 			$disabledFunctions = $this->iniGetWrapper->getString('disable_functions');
 			if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) {
 				return [$level, $recommendations];
@@ -70,21 +70,19 @@ class PhpOpcacheSetup implements ISetupCheck {
 				$level = 'error';
 			}
 
-			// Recommend to raise value, if more than 90% of max value is reached
-			if (
-				empty($status['opcache_statistics']['max_cached_keys']) ||
-				($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9)
-			) {
-				$recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply "opcache.max_accelerated_files" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently')]);
-			}
-
-			if (
-				empty($status['memory_usage']['free_memory']) ||
-				($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9)
-			) {
-				$recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply "opcache.memory_consumption" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]);
+			// Check whether OPcache is full, which can be either the overall OPcache size or limit of cached keys reached.
+			// If the limit of cached keys has been reached, num_cached_keys equals max_cached_keys. The recommendation contains this value instead of opcache.max_accelerated_files, since the effective limit is a next higher prime number: https://www.php.net/manual/en/opcache.configuration.php#ini.opcache.max-accelerated-files
+			// Else, the remaining $status['memory_usage']['free_memory'] was too low to store another script. Aside of used_memory, this can be also due to wasted_memory, remaining cache keys from scripts changed on disk.
+			// Wasted memory is cleared only via opcache_reset(), or if $status['memory_usage']['current_wasted_percentage'] reached opcache.max_wasted_percentage, which triggers an engine restart and hence OPcache reset. Due to this complexity, we check for $status['cache_full'] only.
+			if ($status['cache_full'] === true) {
+				if ($status['opcache_statistics']['num_cached_keys'] === $status['opcache_statistics']['max_cached_keys']) {
+					$recommendations[] = $this->l10n->t('The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be kept in the cache, it is recommended to apply "opcache.max_accelerated_files" to your PHP configuration with a value higher than "%s".', [($status['opcache_statistics']['max_cached_keys'] ?: 'currently')]);
+				} else {
+					$recommendations[] = $this->l10n->t('The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply "opcache.memory_consumption" to your PHP configuration with a value higher than "%s".', [($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently')]);
+				}
 			}
 
+			// Interned strings buffer: recommend to raise size if more than 90% is used
 			$interned_strings_buffer = $this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?? 0;
 			$memory_consumption = $this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?? 0;
 			if (