nextcloud / nextcloud/server

Reorder server and apps class loaders cleverly

Open
#38,835 3 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage enhancement performance 🚀
Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

How to use GitHub
  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.

We have spent some time on autoloader optimizations recently. E.g. authoritative class loader for apps, apcu caching in server, etc. While debugging https://github.com/nextcloud/calendar/issues/5304 I stepped through class resolution and noticed that the many registered autoloaders are currently executed in order of the app loading. This seems to be in alphabetical order.

There are three classes of autoloaders

  • Authoritative classloader for shipped apps
  • Authoritative classloader for server classes and deps
  • App's own classloader
  • Dynamic classloader for apps with no own classloader

Example: twofactor_webauthn needs \Webauthn\Credential and the autoloaders get triggered

  1. activity uses an authoritative classloader and returns quickly with a negative result
  2. admin_audit uses an authoritative classloader and returns quickly with a negative result
  3. calendar uses its own classloader that checks the filesystem but returns a negative result
  4. collectives uses Nextcloud's default classloader that checks the filesystem but returns a negative result (only loads OCA)
  5. contacts uses its own classloader that checks the filesystem but returns a negative result
  6. deck uses its own classloader that checks the filesystem but returns a negative result
  7. deck uses its own classloader that checks the filesystem but returns a negative result
    ...
  8. twofactor_webauthn uses its own classloader that checks the filesystem and returns the class path

Describe the solution you'd like

  1. Assume server has an optimized class loader for own classes and 3rdparty
  2. Assume all shipped apps have an optimized and quick autoloader
  3. Assume app store apps with their own autoloader have a slower autoloader
  4. Assume apps without an own autoloader load the slowest

Based on this, reorder the autoloading order to:

  1. Ask authoritative classloader for server classes and 3rdparty
  2. Ask authoritative classloaders of shipped apps
  3. Ask app autoloaders
  4. Ask dynamic classloader

Side effect: If server and an app use the same third party class, the server class will now take precedence.

Describe alternatives you've considered

Authoritative class loaders for app store apps -> 💥
APCu class loader caching -> 💥

Additional context


cc @juliushaertl @kesselb @nickvergessen

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace how Nextcloud registers and invokes the server, shipped-app, app, and dynamic class loaders during class resolution. Compare the current order with the proposed four-step order, including the stated third-party precedence change; done means the ordering is implemented and its behavior is verified without relying on authoritative loaders for app-store apps.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend, performance
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.