Merge pull request #27403 from nextcloud/enh/opcache-usage-based-reco…

…mmendations Enhance and complement OPcache setup checks
nextcloud · Dec 20, 2021 · e56d1dd · e56d1dd
2 parents d98e31a + 82c1bea
commit e56d1dd
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 138 deletions.
diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
@@ -25,6 +25,7 @@
  * @author timm2k <timm2k@gmx.de>
  * @author Timo Förster <tfoerster@webfoersterei.de>
  * @author Valdnet <47037905+Valdnet@users.noreply.github.com>
+ * @author MichaIng <micha@dietpi.com>
  *
  * @license AGPL-3.0
  *
@@ -238,7 +239,7 @@ protected function getCurlVersion() {
 	}
 
 	/**
-	 * Check if the used  SSL lib is outdated. Older OpenSSL and NSS versions do
+	 * Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
 	 * have multiple bugs which likely lead to problems in combination with
 	 * functionality required by ownCloud such as SNI.
 	 *
@@ -450,31 +451,61 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
 	}
 
 	/**
-	 * Checks whether a PHP opcache is properly set up
-	 * @return bool
+	 * Checks whether a PHP OPcache is properly set up
+	 * @return string[] The list of OPcache setup recommendations
 	 */
-	protected function isOpcacheProperlySetup() {
-		if (!$this->iniGetWrapper->getBool('opcache.enable')) {
-			return false;
+	protected function getOpcacheSetupRecommendations(): array {
+		// If the module is not loaded, return directly to skip inapplicable checks
+		if (!extension_loaded('Zend OPcache')) {
+			return ['The PHP OPcache module is not loaded. <a target="_blank" rel="noreferrer noopener" class="external" href="' . $this->urlGenerator->linkToDocs('admin-php-opcache') . '">For better performance it is recommended</a> to load it into your PHP installation.'];
 		}
 
-		if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
-			return false;
-		}
+		$recommendations = [];
 
-		if ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') < 10000) {
-			return false;
+		// Check whether Nextcloud is allowed to use the OPcache API
+		$isPermitted = true;
+		$permittedPath = $this->iniGetWrapper->getString('opcache.restrict_api');
+		if (isset($permittedPath) && $permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, $permittedPath)) {
+			$isPermitted = false;
 		}
 
-		if ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') < 128) {
-			return false;
-		}
+		if (!$this->iniGetWrapper->getBool('opcache.enable')) {
+			$recommendations[] = 'OPcache is disabled. For better performance, it is recommended to apply <code>opcache.enable=1</code> to your PHP configuration.';
 
-		if ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < 8) {
-			return false;
+			// Check for saved comments only when OPcache is currently disabled. If it was enabled, opcache.save_comments=0 would break Nextcloud in the first place.
+			if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
+				$recommendations[] = 'OPcache is configured to remove code comments. With OPcache enabled, <code>opcache.save_comments=1</code> must be set for Nextcloud to function.';
+			}
+
+			if (!$isPermitted) {
+				$recommendations[] = 'Nextcloud is not allowed to use the OPcache API. With OPcache enabled, it is highly recommended to include all Nextcloud directories with <code>opcache.restrict_api</code> or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.';
+			}
+		} elseif (!$isPermitted) {
+			$recommendations[] = 'Nextcloud is not allowed to use the OPcache API. It is highly recommended to include all Nextcloud directories with <code>opcache.restrict_api</code> or unset this setting to disable OPcache API restrictions, to prevent errors during Nextcloud core or app upgrades.';
+		} else {
+			// Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks
+			$disabledFunctions = $this->iniGetWrapper->getString('disable_functions');
+			if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) {
+				return [];
+			}
+
+			$status = opcache_get_status(false);
+
+			// Recommend to raise value, if more than 90% of max value is reached
+			if ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) {
+				$recommendations[] = 'The maximum number of OPcache keys is nearly exceeded. To assure that all scripts can be hold in cache, it is recommended to apply <code>opcache.max_accelerated_files</code> to your PHP configuration with a value higher than <code>' . ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') ?: 'currently') . '</code>.';
+			}
+
+			if ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) {
+				$recommendations[] = 'The OPcache buffer is nearly full. To assure that all scripts can be hold in cache, it is recommended to apply <code>opcache.memory_consumption</code> to your PHP configuration with a value higher than <code>' . ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') ?: 'currently') . '</code>.';
+			}
+
+			if ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) {
+				$recommendations[] = 'The OPcache interned strings buffer is nearly full. To assure that repeating strings can be effectively cached, it is recommended to apply <code>opcache.interned_strings_buffer</code> to your PHP configuration with a value higher than <code>' . ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') ?: 'currently') . '</code>.';
+			}
 		}
 
-		return true;
+		return $recommendations;
 	}
 
 	/**
@@ -574,10 +605,6 @@ protected function getCronErrors() {
 		return [];
 	}
 
-	protected function hasOpcacheLoaded(): bool {
-		return extension_loaded('Zend OPcache');
-	}
-
 	private function isTemporaryDirectoryWritable(): bool {
 		try {
 			if (!empty($this->tempManager->getTempBaseDir())) {
@@ -791,9 +818,7 @@ public function check() {
 				'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(),
 				'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
 				'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
-				'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(),
-				'hasOpcacheLoaded' => $this->hasOpcacheLoaded(),
-				'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'),
+				'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(),
 				'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
 				'hasFreeTypeSupport' => $this->hasFreeTypeSupport(),
 				'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(),

diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php
@@ -179,13 +179,12 @@ protected function setUp(): void {
 				'getSuggestedOverwriteCliURL',
 				'getCurlVersion',
 				'isPhpOutdated',
-				'isOpcacheProperlySetup',
+				'getOpcacheSetupRecommendations',
 				'hasFreeTypeSupport',
 				'hasMissingIndexes',
 				'hasMissingPrimaryKeys',
 				'isSqliteUsed',
 				'isPHPMailerUsed',
-				'hasOpcacheLoaded',
 				'getAppDirsWithDifferentOwner',
 				'hasRecommendedPHPModules',
 				'hasBigIntConversionPendingColumns',
@@ -479,8 +478,8 @@ public function testCheck() {
 			->willReturn(true);
 		$this->checkSetupController
 			->expects($this->once())
-			->method('isOpcacheProperlySetup')
-			->willReturn(false);
+			->method('getOpcacheSetupRecommendations')
+			->willReturn(['recommendation1', 'recommendation2']);
 		$this->checkSetupController
 			->method('hasFreeTypeSupport')
 			->willReturn(false);
@@ -505,10 +504,6 @@ public function testCheck() {
 			->expects($this->once())
 			->method('hasFileinfoInstalled')
 			->willReturn(true);
-		$this->checkSetupController
-			->expects($this->once())
-			->method('hasOpcacheLoaded')
-			->willReturn(true);
 		$this->checkSetupController
 			->expects($this->once())
 			->method('hasWorkingFileLocking')
@@ -624,9 +619,7 @@ public function testCheck() {
 				'isCorrectMemcachedPHPModuleInstalled' => true,
 				'hasPassedCodeIntegrityCheck' => true,
 				'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity',
-				'isOpcacheProperlySetup' => false,
-				'hasOpcacheLoaded' => true,
-				'phpOpcacheDocumentation' => 'http://docs.example.org/server/go.php?to=admin-php-opcache',
+				'OpcacheSetupRecommendations' => ['recommendation1', 'recommendation2'],
 				'isSettimelimitAvailable' => true,
 				'hasFreeTypeSupport' => false,
 				'isSqliteUsed' => false,

diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
@@ -328,18 +328,16 @@
 							type: OC.SetupChecks.MESSAGE_TYPE_ERROR
 						});
 					}
-					if(!data.hasOpcacheLoaded) {
-						messages.push({
-							msg: t('core', 'The PHP OPcache module is not loaded. {linkstart}For better performance it is recommended ↗{linkend} to load it into your PHP installation.')
-								.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
-								.replace('{linkend}', '</a>'),
-							type: OC.SetupChecks.MESSAGE_TYPE_INFO
+					if(data.OpcacheSetupRecommendations.length > 0) {
+						var listOfOPcacheRecommendations = "";
+						data.OpcacheSetupRecommendations.forEach(function(element){
+							listOfOPcacheRecommendations += "<li>" + element + "</li>";
 						});
-					} else if(!data.isOpcacheProperlySetup) {
 						messages.push({
-							msg: t('core', 'The PHP OPcache module is not properly configured. {linkstart}For better performance it is recommended ↗{linkend} to use the following settings in the <code>php.ini</code>:')
-								.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
-								.replace('{linkend}', '</a>') + "<pre><code>opcache.enable=1\nopcache.interned_strings_buffer=8\nopcache.max_accelerated_files=10000\nopcache.memory_consumption=128\nopcache.save_comments=1\nopcache.revalidate_freq=1</code></pre>",
+							msg: t(
+								'core',
+								'The PHP OPcache module is not properly configured:'
+							) + "<ul>" + listOfOPcacheRecommendations + "</ul>",
 							type: OC.SetupChecks.MESSAGE_TYPE_INFO
 						});
 					}