Create a checksum of the config file in memory.

[ativarsoft]

This fixes a massive performance bug when opcache is enabled.

I have tested it on the stable version. I copy-pasted the changes on the master branch. Please test it on the master branch and tell me if it worked.

Signed-off-by: Mateus de Lima Oliveira [email protected]

[kesselb]

Hi @M4t3uz 👋 ,

This fixes a massive performance bug when opcache is enabled.

Can you tell us a bit more about this performance bug? I assume that Opcache is enabled for most production instances out there and don't remember an issue here on GitHub about a massive performance bug when opcache is enabled.

I understand your patch and the idea "don't write the configuration file another time when the content is the same" makes sense to me. On the other hand it's new to me that something is changing the configuration regularly 😕

[ativarsoft]

Hello @kesselb 👋

Some apps and cron.php update the config file by setting some entries on the config to the same value. The writeData() method calls a php function to invalidate the config file on the opcache. So all the other files that include config.php and those that include them (and so on...) need to be recompiled too. My instance has only recommended apps from the store. You can try to install them all and you'll see that Nextcloud stable gets slow after a while without activity. The next time some user tries to use the web interface after a while without activity on the instance they experience a very long delay of about from 7 to 12 seconds.

I'm running Nextcloud stable on Debian 11. I experienced a major speed improvement.

[kesselb]

Some apps and cron.php update the config file by setting some entries on the config to the same value.

server/lib/private/Config.php

Line 158 in c2dafb0

if (!isset($this->cache[$key]) || $this->cache[$key] !== $value) {

Setting the same value will not trigger a write operation.

The writeData() method calls a php function to invalidate the config file on the opcache. So all the other files that include config.php and those that include them (and so on...) need to be recompiled too.

Good summary 👍

My instance has only recommended apps from the store. You can try to install them all and you'll see that Nextcloud stable gets slow after a while without activity. The next time some user tries to use the web interface after a while without activity on the instance they experience a very long delay of about from 7 to 12 seconds.

Would you mind to share the installed apps with us?

I don't see that my production instance is updating the config.php itself. It's usually the timestamp when I installed the last update.

[ativarsoft]

I had all the recommended apps from the stable version installed. No other apps were installed. I can't provide a list of the recommended apps because I would have to copy-and-paste one by one from the apps page.

I installed some more apps after the patch. I'm testing on this one right now and it works fine:

Enabled:

• accessibility: 1.10.0
• activity: 2.16.0
• bruteforcesettings: 2.4.0
• calendar: 3.5.0
• checksum: 1.1.4
• circles: 24.0.1
• cloud_federation_api: 1.7.0
• comments: 1.14.0
• contacts: 4.2.0
• contactsinteraction: 1.5.0
• dashboard: 7.4.0
• dav: 1.22.0
• deck: 1.7.1
• federatedfilesharing: 1.14.0
• federation: 1.14.0
• files: 1.19.0
• files_pdfviewer: 2.5.0
• files_rightclick: 1.3.0
• files_sharing: 1.16.2
• files_trashbin: 1.14.0
• files_versions: 1.17.0
• files_videoplayer: 1.13.0
• forms: 2.5.1
• groupfolders: 12.0.1
• integration_discourse: 1.0.4
• integration_github: 1.0.4
• integration_gitlab: 1.0.6
• integration_google: 1.0.8
• integration_mastodon: 1.0.3
• integration_onedrive: 1.1.3
• integration_twitter: 1.0.3
• logreader: 2.9.0
• lookup_server_connector: 1.12.0
• mail: 1.13.8
• maps: 0.2.1
• nextcloud_announcements: 1.13.0
• notifications: 2.12.1
• oauth2: 1.12.0
• password_policy: 1.14.0
• photos: 1.6.0
• privacy: 1.8.0
• provisioning_api: 1.14.0
• quota_warning: 1.14.0
• recommendations: 1.3.0
• richdocuments: 6.2.0
• serverinfo: 1.14.0
• settings: 1.6.0
• sharebymail: 1.14.0
• socialsharing_diaspora: 2.5.0
• socialsharing_email: 2.5.0
• socialsharing_facebook: 2.5.0
• socialsharing_twitter: 2.5.0
• spreed: 14.0.4
• survey_client: 1.12.0
• systemtags: 1.14.0
• tasks: 0.14.4
• text: 3.5.1
• theming: 1.15.0
• twofactor_backupcodes: 1.13.0
• unsplash: 1.2.5
• updatenotification: 1.14.0
• user_status: 1.4.0
• viewer: 1.8.0
• weather_status: 1.4.0
• workflow_ocr: 1.24.4
• workflowengine: 2.6.0
  Disabled:
• admin_audit
• encryption
• files_external
• files_fulltextsearch: 24.0.1
• files_fulltextsearch_tesseract: 24.0.0
• firstrunwizard: 2.13.0
• fulltextsearch: 24.0.0
• ncdownloader: 1.0.1
• news: 18.1.1
• support: 1.7.0
• user_ldap

[kesselb]

1. Would you mind explain the latest commit? When Nextcloud and/or apps are updated it's crucial to clear opcache. We already had cases when the cache was not cleared properly and some classes were not found during the upgrade. I would rather call opcache_reset to often, in an update situation, then not often enough. I would also not expect any performance improvement by your second commit as \OCP\Util::needUpgrade() is only true when an update is necessary.

2. Would you mind to provide us some details about your system? For example the Nextcloud version, PHP version, used database, etc.

3. Only a changed value will trigger a write operation. What your patch implements is already there just in a different version. Would you mind to add some logging to figure out what key/value changed on your system and trigger the write operation?

   server/lib/private/Config.php

   Line 159 in c2dafb0

   // Add change

[ativarsoft]

1. opcache_reset was being called when there is an update available but the user has not upgraded yet so the instance gets awfully slow if it can be upgraded somehow. My latest patch assures that the instance is in maintenance mode. It means that the administrator of the instance is expecting and wants an upgrade.
2. I will lookup this information to you tomorrow.
3. I will search for the pieces of code that update config.php this weekend and I will provide you the details later. I understand that you are skeptical about the usefulness of it but I don't have it tangible right now.

[ativarsoft]

OK, here is some more information, @kesselb.

2. My system.

OS: Linux 5.10.0-18-amd64 x86_64
CPU: AMD EPYC-Rome Processor (1 core)

PHP
Version: 7.4.30
Memory limir: 4 GB
Maximum execution time: 3600
Maximum upload size: 2 MB
Extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, Reflection, SPL, session, standard, sodium, apache2handler, mysqlnd, PDO, xml, bcmath, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, gmp, iconv, igbinary, imagick, intl, json, exif, msgpack, mysqli, pdo_mysql, Phar, posix, pspell, readline, shmop, SimpleXML, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlwriter, xsl, zip, memcached, Zend OPcache

Database
Type: mysql
Version: 10.5.15
Size: 11,7 MB

3. How config.php gets changed on disk

It seems that there is an error in the very same code you mentioned.

if (!isset($this->cache[$key]) || $this->cache[$key] !== $value) {

shoud be

if (!isset($this->cache[$key]) || $this->cache[$key] != $value) {

I have tested changing it and it sppeds my instance up even more as crc32() does not need to be calculated.

[kesselb]

Hey @M4t3uz,

I hope my questions did not drive you away. Did you find a solution?

[ativarsoft]

You din't drive me away. Don't worry. :)

I made some mistakes like creating a pull request from the master and merging the same commit twice. So I decided to create a diff and fork the project from scratch. My Nextcloud instance is running blazing fast with many plugins installed. The storage is on a mount of a remote filesystem. It is basically just a bunch of optimizations.

I made plenty of changes on the stable24 branch but they were incompatible with the master branch. I will test it more appropriately on my branch and then I will make a new pull request. I hope you people at the Nextcloud project will like it.

[come-nc]

It seems that there is an error in the very same code you mentioned.

if (!isset($this->cache[$key]) || $this->cache[$key] !== $value) {

shoud be

if (!isset($this->cache[$key]) || $this->cache[$key] != $value) {

I have tested changing it and it sppeds my instance up even more as crc32() does not need to be calculated.

@M4t3uz I am relunctant to change this equality check but would be very interested in the values that triggers it.
Could you add some logging of the values when $this->cache[$key] !== $value and $this->cache[$key] != $value differs?