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

OC_Autoloader is slow #13241

Open
LukasReschke opened this Issue Jan 10, 2015 · 19 comments

Comments

Projects
None yet
10 participants
@LukasReschke
Copy link
Member

LukasReschke commented Jan 10, 2015

Our custom autoloader OC_Autoloader is actually very slow and requires alone about 100ms for every request. That's a huge amount when we compare it to the Composer loader:

screen shot 2015-01-10 at 13 08 16
screen shot 2015-01-10 at 13 08 27

We can see that the Composer autoloader is actually called about twice the time but only takes a quarter of the time. It can also be seen that this seems to be a problem in the logic as 82ms are spent on CPU on our autoloader.

@LukasReschke

This comment has been minimized.

Copy link
Member Author

LukasReschke commented Jan 10, 2015

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 10, 2015

Once we have composer within core we can use it to handle loading of our core classes as well.
Topic for 8.1

@DeepDiver1975 DeepDiver1975 added this to the 8.1-next milestone Jan 10, 2015

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 10, 2015

@karlitschek FYI - just another argument pro composer in core

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 10, 2015

@LukasReschke which classes did you load for testing? This has an impact on the code path execution. THX

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Jan 12, 2015

refs #9643

@bantu

This comment has been minimized.

Copy link
Member

bantu commented Jan 18, 2015

refs #9619

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Apr 23, 2015

Do we want to invest 8.1 time into making our own autoloader better ? Else defer to 8.2

@Xenopathic

This comment has been minimized.

Copy link
Member

Xenopathic commented Apr 23, 2015

Probably best to defer. This could break things.

Another advantage of the Composer autoloader: when releasing to production, we can create a Composer classmap file, which reads the location of classes from an array instead of trying to find them.

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.2-next, 8.1-current Apr 23, 2015

@DeepDiver1975

This comment has been minimized.

Copy link
Member

DeepDiver1975 commented Apr 23, 2015

This is basically blocked by the issue where we move composer into core

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Sep 21, 2015

We're in feature freeze => 9.0

@PVince81 PVince81 modified the milestones: 9.0-next, 8.2-current Sep 21, 2015

@rullzer

This comment has been minimized.

Copy link
Contributor

rullzer commented Dec 23, 2015

Our autoload has been improved significantly. But moving composer into core has other benefits as well. e.g. classmap generation for releases etc.

Getting composer into core is actually not hard (getting composer for 3rdparty apps requires some thinking). But we can easily run 2 autoloaders next to each other. Composers autoloader that loads proper PSR-4 classes and our current autoload that loads everything else.

I'll come up with a POC PR soonish.

@rullzer

This comment has been minimized.

Copy link
Contributor

rullzer commented Feb 12, 2016

Ongoing effort. #21582 could help here eventually.
But nothing for 9.0 @PVince81 @cmonteroluque

@PVince81

This comment has been minimized.

Copy link
Member

PVince81 commented Feb 12, 2016

Moving to 9.1

@PVince81 PVince81 modified the milestones: 9.1-next, 9.0-current Feb 12, 2016

@cmonteroluque

This comment has been minimized.

Copy link
Contributor

cmonteroluque commented Feb 13, 2016

ok

@rullzer

This comment has been minimized.

Copy link
Contributor

rullzer commented Apr 20, 2016

Just to update here. I'm slowly moving all the stuff in lib/private to PSR-4. So we are moving in the right direction.

@rullzer

This comment has been minimized.

Copy link
Contributor

rullzer commented May 17, 2016

\OC is now all PSR-4 and handled by composer.
\OCP is also moving towards that

@bantu

This comment has been minimized.

Copy link
Member

bantu commented May 17, 2016

@nickvergessen

This comment has been minimized.

Copy link
Contributor

nickvergessen commented May 18, 2016

\OCA is also handled by PSR-4 composer now, core apps are already updated or have PRs pending,
documentation is at: https://doc.owncloud.org/server/9.1/developer_manual/app/classloader.html

@rullzer

This comment has been minimized.

Copy link
Contributor

rullzer commented May 30, 2016

The OC autoloader is now only used to load legacy classes (we should migate away from those!) and apps that have not converted to the fancy new naming scheme.

@PVince81 PVince81 modified the milestones: 9.1-current, 9.2-next Jun 15, 2016

@PVince81 PVince81 modified the milestones: backlog, 10.0 Jan 27, 2017

@felixboehm felixboehm removed the sev2-high label Apr 9, 2018

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