Skip to content

Commit 2e19c83

Browse files
committed
Do OPcache recommendations based on actual cache usage
The current OPcache recommendations match the PHP defaults, but the values are much higher than what is required to run Nextcloud, even with a high number of installed apps. On the other hand, when other applications use the same PHP server/pool, hence the same OPcache instance, the recommended values might not be sufficient. Accurate recommendations can only be done when taking into account the actual OPcache usage. With this commit, warnings are shown when used values exceed 90% of max values, and the recommendations are to raise the config value, showing what is currently applied. Signed-off-by: MichaIng <micha@dietpi.com>
1 parent b3cfa18 commit 2e19c83

3 files changed

Lines changed: 48 additions & 34 deletions

File tree

apps/settings/lib/Controller/CheckSetupController.php

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ protected function getCurlVersion() {
220220
}
221221

222222
/**
223-
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
223+
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
224224
* have multiple bugs which likely lead to problems in combination with
225225
* functionality required by ownCloud such as SNI.
226226
*
@@ -426,31 +426,40 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
426426
}
427427

428428
/**
429-
* Checks whether a PHP opcache is properly set up
430-
* @return bool
429+
* Checks whether a PHP OPcache is properly set up
430+
* @return string[]
431431
*/
432-
protected function isOpcacheProperlySetup() {
432+
protected function isOpcacheProperlySetup(): array {
433+
$recommendations = [];
434+
433435
if (!$this->iniGetWrapper->getBool('opcache.enable')) {
434-
return false;
435-
}
436+
$recommendations[] = 'OPcache is disabled. For better performance, it is recommended to apply <code>opcache.enable=1</code> to your PHP configuration.';
436437

437-
if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
438-
return false;
439-
}
438+
// 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.
439+
if (!$this->iniGetWrapper->getBool('opcache.save_comments')) {
440+
$recommendations[] = 'OPcache is configured to remove code comments. With OPcache enabled, <code>opcache.save_comments=1</code> must be set for Nextcloud to function.';
441+
}
440442

441-
if ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') < 10000) {
442-
return false;
443-
}
443+
// Compare max values with used values and warn if more than 90% is used.
444+
} else {
445+
// Mute error when opcache.restrict_api is set and does not permit Nextcloud to use opcache_get_status(). All below conditions will then return false.
446+
// ToDo: Add a check for opcache.restrict_api, since it can cause issues when Nextcloud cannot evict cached scripts: https://github.com/nextcloud/server/pull/8188
447+
$status = @opcache_get_status();
444448

445-
if ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') < 128) {
446-
return false;
447-
}
449+
if ($status['opcache_statistics']['num_cached_keys'] / $status['opcache_statistics']['max_cached_keys'] > 0.9) {
450+
$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>.';
451+
}
448452

449-
if ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < 8) {
450-
return false;
453+
if ($status['memory_usage']['used_memory'] / $status['memory_usage']['free_memory'] > 9) {
454+
$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>.';
455+
}
456+
457+
if ($status['interned_strings_usage']['used_memory'] / $status['interned_strings_usage']['free_memory'] > 9) {
458+
$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>.';
459+
}
451460
}
452461

453-
return true;
462+
return $recommendations;
454463
}
455464

456465
/**

core/js/setupchecks.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,11 +329,16 @@
329329
.replace('{linkend}', '</a>'),
330330
type: OC.SetupChecks.MESSAGE_TYPE_INFO
331331
});
332-
} else if(!data.isOpcacheProperlySetup) {
332+
} else if(data.isOpcacheProperlySetup.length > 0) {
333+
var listOfOPcacheRecommends = "";
334+
data.isOpcacheProperlySetup.forEach(function(element){
335+
listOfOPcacheRecommends += "<li>" + element + "</li>";
336+
});
333337
messages.push({
334-
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>:')
335-
.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + data.phpOpcacheDocumentation + '">')
336-
.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>",
338+
msg: t(
339+
'core',
340+
'The PHP OPcache module is not properly configured:'
341+
) + "<ul>" + listOfOPcacheRecommends + "</ul>",
337342
type: OC.SetupChecks.MESSAGE_TYPE_INFO
338343
});
339344
}

core/js/tests/specs/setupchecksSpec.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ describe('OC.SetupChecks tests', function() {
235235
forwardedForHeadersWorking: true,
236236
isCorrectMemcachedPHPModuleInstalled: true,
237237
hasPassedCodeIntegrityCheck: true,
238-
isOpcacheProperlySetup: true,
238+
isOpcacheProperlySetup: [],
239239
hasOpcacheLoaded: true,
240240
isSettimelimitAvailable: true,
241241
hasFreeTypeSupport: true,
@@ -291,7 +291,7 @@ describe('OC.SetupChecks tests', function() {
291291
forwardedForHeadersWorking: true,
292292
isCorrectMemcachedPHPModuleInstalled: true,
293293
hasPassedCodeIntegrityCheck: true,
294-
isOpcacheProperlySetup: true,
294+
isOpcacheProperlySetup: [],
295295
hasOpcacheLoaded: true,
296296
isSettimelimitAvailable: true,
297297
hasFreeTypeSupport: true,
@@ -348,7 +348,7 @@ describe('OC.SetupChecks tests', function() {
348348
forwardedForHeadersWorking: true,
349349
isCorrectMemcachedPHPModuleInstalled: true,
350350
hasPassedCodeIntegrityCheck: true,
351-
isOpcacheProperlySetup: true,
351+
isOpcacheProperlySetup: [],
352352
hasOpcacheLoaded: true,
353353
isSettimelimitAvailable: true,
354354
hasFreeTypeSupport: true,
@@ -403,7 +403,7 @@ describe('OC.SetupChecks tests', function() {
403403
forwardedForHeadersWorking: true,
404404
isCorrectMemcachedPHPModuleInstalled: true,
405405
hasPassedCodeIntegrityCheck: true,
406-
isOpcacheProperlySetup: true,
406+
isOpcacheProperlySetup: [],
407407
hasOpcacheLoaded: true,
408408
isSettimelimitAvailable: true,
409409
hasFreeTypeSupport: true,
@@ -456,7 +456,7 @@ describe('OC.SetupChecks tests', function() {
456456
forwardedForHeadersWorking: true,
457457
isCorrectMemcachedPHPModuleInstalled: false,
458458
hasPassedCodeIntegrityCheck: true,
459-
isOpcacheProperlySetup: true,
459+
isOpcacheProperlySetup: [],
460460
hasOpcacheLoaded: true,
461461
isSettimelimitAvailable: true,
462462
hasFreeTypeSupport: true,
@@ -509,7 +509,7 @@ describe('OC.SetupChecks tests', function() {
509509
forwardedForHeadersWorking: true,
510510
isCorrectMemcachedPHPModuleInstalled: true,
511511
hasPassedCodeIntegrityCheck: true,
512-
isOpcacheProperlySetup: true,
512+
isOpcacheProperlySetup: [],
513513
hasOpcacheLoaded: true,
514514
isSettimelimitAvailable: true,
515515
hasFreeTypeSupport: true,
@@ -564,7 +564,7 @@ describe('OC.SetupChecks tests', function() {
564564
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
565565
isCorrectMemcachedPHPModuleInstalled: true,
566566
hasPassedCodeIntegrityCheck: true,
567-
isOpcacheProperlySetup: true,
567+
isOpcacheProperlySetup: [],
568568
hasOpcacheLoaded: true,
569569
isSettimelimitAvailable: true,
570570
hasFreeTypeSupport: true,
@@ -617,7 +617,7 @@ describe('OC.SetupChecks tests', function() {
617617
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
618618
isCorrectMemcachedPHPModuleInstalled: true,
619619
hasPassedCodeIntegrityCheck: true,
620-
isOpcacheProperlySetup: true,
620+
isOpcacheProperlySetup: [],
621621
hasOpcacheLoaded: true,
622622
isSettimelimitAvailable: false,
623623
hasFreeTypeSupport: true,
@@ -670,7 +670,7 @@ describe('OC.SetupChecks tests', function() {
670670
reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html',
671671
isCorrectMemcachedPHPModuleInstalled: true,
672672
hasPassedCodeIntegrityCheck: true,
673-
isOpcacheProperlySetup: true,
673+
isOpcacheProperlySetup: [],
674674
hasOpcacheLoaded: true,
675675
isSettimelimitAvailable: true,
676676
hasFreeTypeSupport: true,
@@ -744,7 +744,7 @@ describe('OC.SetupChecks tests', function() {
744744
phpSupported: {eol: true, version: '5.4.0'},
745745
isCorrectMemcachedPHPModuleInstalled: true,
746746
hasPassedCodeIntegrityCheck: true,
747-
isOpcacheProperlySetup: true,
747+
isOpcacheProperlySetup: [],
748748
hasOpcacheLoaded: true,
749749
isSettimelimitAvailable: true,
750750
hasFreeTypeSupport: true,
@@ -797,7 +797,7 @@ describe('OC.SetupChecks tests', function() {
797797
forwardedForHeadersWorking: true,
798798
isCorrectMemcachedPHPModuleInstalled: true,
799799
hasPassedCodeIntegrityCheck: true,
800-
isOpcacheProperlySetup: false,
800+
isOpcacheProperlySetup: ['recommendation1', 'recommendation2'],
801801
hasOpcacheLoaded: true,
802802
phpOpcacheDocumentation: 'https://example.org/link/to/doc',
803803
isSettimelimitAvailable: true,
@@ -822,7 +822,7 @@ describe('OC.SetupChecks tests', function() {
822822

823823
async.done(function( data, s, x ){
824824
expect(data).toEqual([{
825-
msg: 'The PHP OPcache module is not properly configured. <a target="_blank" rel="noreferrer noopener" class="external" href="https://example.org/link/to/doc">For better performance it is recommended ↗</a> to use the following settings in the <code>php.ini</code>:' + "<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>",
825+
msg: 'The PHP OPcache module is not properly configured:<ul><li>recommendation1</li><li>recommendation2</li></ul>',
826826
type: OC.SetupChecks.MESSAGE_TYPE_INFO
827827
}]);
828828
done();

0 commit comments

Comments
 (0)