Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(autoloader): Force own autoloader #8578

Merged
merged 2 commits into from
Jan 25, 2023

Conversation

ChristophWurst
Copy link
Member

And don't register any generic PSR-4 nor PSR-0 loader in server.

https://github.com/nextcloud/server/blob/473c546b5c562e19ba8f9d3aef9cd64d2458cda7/lib/private/legacy/OC_App.php#L284-L290

Benchmarks

Blackfire comparison graph on IO wait

Bildschirmfoto vom 2023-01-20 13-51-25

Blackfire recommendations before

Bildschirmfoto vom 2023-01-20 13-53-14

Blackfire recommendations after

Bildschirmfoto vom 2023-01-20 13-50-49

🖼️ Screenshots

🏚️ Before 🏡 After
🦥 🐌

🏁 Checklist

@ChristophWurst
Copy link
Member Author

ChristophWurst commented Jan 20, 2023

metrics.composer.autoload.find_file.count 117 <= 50

Server and this app have authorative autoloaders. But I think the generic autoloader in server tries to load this app's files before the shipped autoloader gets a change. If the generic autoloader isn't registered the shipped autoloader gets a chance to load the file.

@nickvergessen
Copy link
Member

Thanks, I was always tempted to add one, but I disliked the "ah forgot to run bash build/autoload..." from server. I didn't know it's possible to do it for 3rdparty but not the own classes :)

@nickvergessen
Copy link
Member

Seems CI doesn't like it
grafik

@ChristophWurst
Copy link
Member Author

that means composer i didn't happen

ChristophWurst and others added 2 commits January 25, 2023 09:47
And don't register any generic PSR-4 nor PSR-0 loader in server.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
…the app

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the perf/autoloader/force-own-autoloader branch from 398ee88 to a2d87ad Compare January 25, 2023 08:49
@nickvergessen
Copy link
Member

that means composer i didn't happen

Right, it was run only after the app was enabled.
Fixed now

@ChristophWurst
Copy link
Member Author

Tip: nextcloud/mail#8020 to relax the autoloader on dev setups and make it find new files too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants