Skip to content

Commit 17e6f70

Browse files
committed
Enhance and complement OPcache setup checks
The current OPcache recommendations match the PHP defaults, but the values are much higher than required to run Nextcloud, even with a high number of installed apps. On the other hand, when other applications use the same OPcache instance, the recommended values might not be sufficient. Accurate recommendations need to take into account actual OPcache usage. With this commit, recommendations are shown to raise the config value if more than 90% of max cache size or number of keys is used. The checks whether the module is loaded and whether the OPcache is properly configured have been merged into a single function. This allowed to reduce the overhead of OPcache configuration checks when the module is not loaded. A check has been added whether Nextcloud is permitted to use the OPcache API. Without this, inconsistencies during core or app upgrades may cause errors and OPcache usage cannot be determined for the new usage based checks. OPcache usage based checks are skipped when Nextcloud is not permitted to use the API. Signed-off-by: MichaIng <[email protected]>
1 parent ad6e975 commit 17e6f70

File tree

4 files changed

+80
-138
lines changed

4 files changed

+80
-138
lines changed

apps/settings/lib/Controller/CheckSetupController.php

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
* @author timm2k <[email protected]>
2626
* @author Timo Förster <[email protected]>
2727
* @author Valdnet <[email protected]>
28+
* @author MichaIng <[email protected]>
2829
*
2930
* @license AGPL-3.0
3031
*
@@ -238,7 +239,7 @@ protected function getCurlVersion() {
238239
}
239240

240241
/**
241-
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
242+
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
242243
* have multiple bugs which likely lead to problems in combination with
243244
* functionality required by ownCloud such as SNI.
244245
*
@@ -450,31 +451,61 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
450451
}
451452

452453
/**
453-
* Checks whether a PHP opcache is properly set up
454-
* @return bool
454+
* Checks whether a PHP OPcache is properly set up
455+
* @return string[] The list of OPcache setup recommendations
455456
*/
456-
protected function isOpcacheProperlySetup() {
457-
if (!$this->iniGetWrapper->getBool('opcache.enable')) {
458-
return false;
457+
protected function getOpcacheSetupRecommendations(): array {
458+
// If the module is not loaded, return directly to skip inapplicable checks
459+
if (!extension_loaded('Zend OPcache')) {
460+
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.'];
459461
}
460462

461-
if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
462-
return false;
463-
}
463+
$recommendations = [];
464464

465-
if ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') < 10000) {
466-
return false;
465+
// Check whether Nextcloud is allowed to use the OPcache API
466+
$isPermitted = true;
467+
$permittedPath = $this->iniGetWrapper->getString('opcache.restrict_api');
468+
if (isset($permittedPath) && $permittedPath !== '' && !str_starts_with(\OC::$SERVERROOT, $permittedPath)) {
469+
$isPermitted = false;
467470
}
468471

469-
if ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') < 128) {
470-
return false;
471-
}
472+
if (!$this->iniGetWrapper->getBool('opcache.enable')) {
473+
$recommendations[] = 'OPcache is disabled. For better performance, it is recommended to apply <code>opcache.enable=1</code> to your PHP configuration.';
472474

473-
if ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < 8) {
474-
return false;
475+
// 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.
476+
if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
477+
$recommendations[] = 'OPcache is configured to remove code comments. With OPcache enabled, <code>opcache.save_comments=1</code> must be set for Nextcloud to function.';
478+
}
479+
480+
if (!$isPermitted) {
481+
$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.';
482+
}
483+
} elseif (!$isPermitted) {
484+
$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.';
485+
} else {
486+
// Check whether opcache_get_status has been explicitly disabled an in case skip usage based checks
487+
$disabledFunctions = $this->iniGetWrapper->getString('disable_functions');
488+
if (isset($disabledFunctions) && str_contains($disabledFunctions, 'opcache_get_status')) {
489+
return [];
490+
}
491+
492+
$status = opcache_get_status(false);
493+
494+
// Recommend to raise value, if more than 90% of max value is reached
495+
if ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) {
496+
$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>.';
497+
}
498+
499+
if ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) {
500+
$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>.';
501+
}
502+
503+
if ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) {
504+
$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>.';
505+
}
475506
}
476507

477-
return true;
508+
return $recommendations;
478509
}
479510

480511
/**
@@ -574,10 +605,6 @@ protected function getCronErrors() {
574605
return [];
575606
}
576607

577-
protected function hasOpcacheLoaded(): bool {
578-
return extension_loaded('Zend OPcache');
579-
}
580-
581608
private function isTemporaryDirectoryWritable(): bool {
582609
try {
583610
if (!empty($this->tempManager->getTempBaseDir())) {
@@ -791,9 +818,7 @@ public function check() {
791818
'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(),
792819
'hasPassedCodeIntegrityCheck' => $this->checker->hasPassedCheck(),
793820
'codeIntegrityCheckerDocumentation' => $this->urlGenerator->linkToDocs('admin-code-integrity'),
794-
'isOpcacheProperlySetup' => $this->isOpcacheProperlySetup(),
795-
'hasOpcacheLoaded' => $this->hasOpcacheLoaded(),
796-
'phpOpcacheDocumentation' => $this->urlGenerator->linkToDocs('admin-php-opcache'),
821+
'OpcacheSetupRecommendations' => $this->getOpcacheSetupRecommendations(),
797822
'isSettimelimitAvailable' => $this->isSettimelimitAvailable(),
798823
'hasFreeTypeSupport' => $this->hasFreeTypeSupport(),
799824
'missingPrimaryKeys' => $this->hasMissingPrimaryKeys(),

apps/settings/tests/Controller/CheckSetupControllerTest.php

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,12 @@ protected function setUp(): void {
179179
'getSuggestedOverwriteCliURL',
180180
'getCurlVersion',
181181
'isPhpOutdated',
182-
'isOpcacheProperlySetup',
182+
'getOpcacheSetupRecommendations',
183183
'hasFreeTypeSupport',
184184
'hasMissingIndexes',
185185
'hasMissingPrimaryKeys',
186186
'isSqliteUsed',
187187
'isPHPMailerUsed',
188-
'hasOpcacheLoaded',
189188
'getAppDirsWithDifferentOwner',
190189
'hasRecommendedPHPModules',
191190
'hasBigIntConversionPendingColumns',
@@ -479,8 +478,8 @@ public function testCheck() {
479478
->willReturn(true);
480479
$this->checkSetupController
481480
->expects($this->once())
482-
->method('isOpcacheProperlySetup')
483-
->willReturn(false);
481+
->method('getOpcacheSetupRecommendations')
482+
->willReturn(['recommendation1', 'recommendation2']);
484483
$this->checkSetupController
485484
->method('hasFreeTypeSupport')
486485
->willReturn(false);
@@ -505,10 +504,6 @@ public function testCheck() {
505504
->expects($this->once())
506505
->method('hasFileinfoInstalled')
507506
->willReturn(true);
508-
$this->checkSetupController
509-
->expects($this->once())
510-
->method('hasOpcacheLoaded')
511-
->willReturn(true);
512507
$this->checkSetupController
513508
->expects($this->once())
514509
->method('hasWorkingFileLocking')
@@ -624,9 +619,7 @@ public function testCheck() {
624619
'isCorrectMemcachedPHPModuleInstalled' => true,
625620
'hasPassedCodeIntegrityCheck' => true,
626621
'codeIntegrityCheckerDocumentation' => 'http://docs.example.org/server/go.php?to=admin-code-integrity',
627-
'isOpcacheProperlySetup' => false,
628-
'hasOpcacheLoaded' => true,
629-
'phpOpcacheDocumentation' => 'http://docs.example.org/server/go.php?to=admin-php-opcache',
622+
'OpcacheSetupRecommendations' => ['recommendation1', 'recommendation2'],
630623
'isSettimelimitAvailable' => true,
631624
'hasFreeTypeSupport' => false,
632625
'isSqliteUsed' => false,

core/js/setupchecks.js

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -328,18 +328,16 @@
328328
type: OC.SetupChecks.MESSAGE_TYPE_ERROR
329329
});
330330
}
331-
if(!data.hasOpcacheLoaded) {
332-
messages.push({
333-
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.')
334-
.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
335-
.replace('{linkend}', '</a>'),
336-
type: OC.SetupChecks.MESSAGE_TYPE_INFO
331+
if(data.OpcacheSetupRecommendations.length > 0) {
332+
var listOfOPcacheRecommendations = "";
333+
data.OpcacheSetupRecommendations.forEach(function(element){
334+
listOfOPcacheRecommendations += "<li>" + element + "</li>";
337335
});
338-
} else if(!data.isOpcacheProperlySetup) {
339336
messages.push({
340-
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>:')
341-
.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
342-
.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>",
337+
msg: t(
338+
'core',
339+
'The PHP OPcache module is not properly configured:'
340+
) + "<ul>" + listOfOPcacheRecommendations + "</ul>",
343341
type: OC.SetupChecks.MESSAGE_TYPE_INFO
344342
});
345343
}

0 commit comments

Comments
 (0)