Skip to content

Conversation

@provokateurin
Copy link
Member

@provokateurin provokateurin commented Jul 8, 2025

Resolves: #54157

Summary

The entire AppConfig is loaded on every request to figure out the enabled apps and their versions (and other things as well). By putting the AppConfig into the local cache we can make the access much faster and reduce the overhead of each request. A very low TTL is used to avoid syncing issues in clustered setups. I also tested with a higher TTL, but it does not improve the performance (as long as the instance is hit by many requests per second).

During my testing of the AppConfig caching I had to disable the global cache prefix logic, as it had a circular dependency on the AppConfig. By using a hardcoded global cache prefix the performance improvement was a bit better than the final result, but it's not possible to do this on a production instance, so the global cache prefix logic had to be changed to not rely on the AppConfig.

In theory the global prefix calculation could be made even faster, by either checking if the setup is clustered (not sure if that is possible?) or by checking if no distributed cache is present. If either of them is true, then the mtimes of the appinfo/info.xml files could be used directly as the global cache prefix and it would not even be necessary to have a local cache for parsing the appinfo/info.xml. I tried this (just hardcoded, not with any of the mentioned checks) just to see if there is any actual improvement, but it was only about 1-2ms and could very well be noise in the data.

Testing setup

docker run --rm -it --network host -e POSTGRES_PASSWORD=postgres postgres:17

rm -rf data config/config.php
./occ maintenance:install --admin-pass admin --database pgsql --database-name postgres --database-host localhost --database-user postgres --database-pass postgres
./occ config:system:set memcache.local --value '\OC\Memcache\APCu'
PHP_CLI_SERVER_WORKERS=100 php -S 0.0.0.0:8080

sudo tc qdisc add dev lo root handle 1:0 netem delay 10msec

k6 run -e BASEURI=http://localhost:8080 basic.js

sudo tc qdisc del dev lo root

Measurements

https://github.com/come-nc/k6_nc_scripts/blob/main/basic.js (with timeout disabled)

Branch Added latency (ms) Median request duration (ms) Absolute improvement (ms) Relative improvement (%)
master 0 56
perf/appconfig/caching 0 56 0 0
master 1 76
perf/appconfig/caching 1 68 8 11.8
master 10 233
perf/appconfig/caching 10 207 26 12.6
master 100 2040
perf/appconfig/caching 100 1640 400 24.4

Checklist

@provokateurin provokateurin added this to the Nextcloud 32 milestone Jul 8, 2025
@provokateurin provokateurin requested a review from a team as a code owner July 8, 2025 12:57
@provokateurin provokateurin requested review from come-nc, nfebe and skjnldsv and removed request for a team July 8, 2025 12:57
@provokateurin provokateurin force-pushed the perf/appconfig/caching branch from 4324dd6 to 906bfcf Compare July 8, 2025 13:30
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1s sounds crazy low, but apart from that the code looks good.

@provokateurin
Copy link
Member Author

I'm fine with increasing it, but like I said in the description it does not make much of a difference. The higher we set it the easier it will become to be desync, so I'd just avoid it by keeping it at 1s.

@come-nc
Copy link
Contributor

come-nc commented Jul 9, 2025

There is no mechanism to reset or adapt cache when an appconfig var is set?
That may break behaviors when an app sets a value and reads it in the next request.

@provokateurin
Copy link
Member Author

Indeed, this part is missing and making the integration tests fail.

@provokateurin provokateurin deleted the perf/appconfig/caching branch September 27, 2025 16:36
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(AppConfig): Cache loading the app config from the database

4 participants