feat(settings): simplify OPcache checks

For the overall OPcache size check, we currently compare used memory with free memory only. However, `opcache.memory_consumption` is split into `used_memory`, `free_memory` and `wasted_memory`. When cached files change on disk, old entires are not replaced or removed, but remain as wasted memory, until their percentage reach `opcache.max_wasted_percentage`, and if the cache is actually full. 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. The probably most accurate check would be to count wasted memory as free nemory, if it is above `opcache.max_wasted_percentage`, as the engine will be restarted as soon as needed, freeing up this space as minimum. On the other hand, wasted memory below the threshold can permanently block the OPcache, which supports counting it as used memory. Depending on the situation, it could be also advised to reduce `opcache.max_wasted_percentage` instead. But too frequent cache resets break its purpose as well.

The matter is IMO too complex to consider everything in detail. But 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 threshold, where it consumes memory permanently. Recommending to raise the OPcache size in this case, is hence as accurate as it gets.

`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 differenciate whether `opcache.max_accelerated_files` or `opcache.memory_consumption` needs to be raised.

In practice, the 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 when cache is 100% full.

Additionally, the recommendation for raising `opcache.max_accelerated_files` now says "a value higher than `max_cached_keys`", instead of `opcache.max_accelerated_files`. The actual limit, reflected by `max_cached_keys` from 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. Recommending "higher than 10000" could hence lead to a settings change without effect. To be sure the effective limit is really reached, it needs to be "higher than 16229" instead.

Signed-off-by: MichaIng <[email protected]>

Author: MichaIng

apps/settings/lib/SetupChecks/PhpOpcacheSetup.php

@@ -57,7 +57,7 @@ protected function getOpcacheSetupRecommendations(): array {
 		} 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 @@ protected function getOpcacheSetupRecommendations(): array {
 				$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 (