Skip to content

Commit a67cc46

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 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. Signed-off-by: MichaIng <micha@dietpi.com>
1 parent 27847a9 commit a67cc46

File tree

3 files changed

+48
-34
lines changed

3 files changed

+48
-34
lines changed

apps/settings/lib/Controller/CheckSetupController.php

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
* @author timm2k <timm2k@gmx.de>
2626
* @author Timo Förster <tfoerster@webfoersterei.de>
2727
* @author Valdnet <47037905+Valdnet@users.noreply.github.com>
28+
* @author MichaIng <micha@dietpi.com>
2829
*
2930
* @license AGPL-3.0
3031
*
@@ -220,7 +221,7 @@ protected function getCurlVersion() {
220221
}
221222

222223
/**
223-
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
224+
* Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do
224225
* have multiple bugs which likely lead to problems in combination with
225226
* functionality required by ownCloud such as SNI.
226227
*
@@ -426,31 +427,39 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse {
426427
}
427428

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

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

441-
if ($this->iniGetWrapper->getNumeric('opcache.max_accelerated_files') < 10000) {
442-
return false;
443-
}
448+
// Recommend to raise value, if more than 90% of max value is reached
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+
}
444452

445-
if ($this->iniGetWrapper->getNumeric('opcache.memory_consumption') < 128) {
446-
return false;
447-
}
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+
}
448456

449-
if ($this->iniGetWrapper->getNumeric('opcache.interned_strings_buffer') < 8) {
450-
return false;
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)