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

Automatically generate classmaps for all PSR-0 packages to speed things up #811

Merged
merged 2 commits into from
Aug 14, 2012

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Jun 19, 2012

According to https://github.com/Seldaek/autoload-bench and other sources classmap is so much faster that it's worth generating it always. This is a bit slow though. Caching should probably be added before merging this PR.

Note that the PSR0 stuff is still registered, so you get the speed benefit of classmap, but any new class added to the tree will be loaded even if the classmap isn't regenerated since the PSR0 fallback will find it.

@travisbot
Copy link

This pull request passes (merged 3ae30b4c into 4bbb168).

@RobLoach
Copy link
Contributor

This is huge! Tested locally against Symfony. It paused more than usual on "Generating autoload files", but for only a second. I am on a pretty beefy machine though. In general, I'd say this is a huge step forward in making Composer autoloading the best choice for a speedy class loading.

This also doesn't impact development at all. If a new class you're working on isn't in the class map, it will fallback to checking the PSR-0 namespaces and your class will be resolved. Well done, Seldaek. I like this a lot.

Would be great to have additional tests on different machines to test the speed of installation/updating though! Thanks.

@travisbot
Copy link

This pull request passes (merged f2f5adf5 into 4bbb168).

@beberlei
Copy link
Contributor

This heavily depends on the filesystem, if you do this on a VM its much slower than on the host. For the Symfony Azure integration i have to do something similar and it can take ~20-60 seconds on VM guest machines to crawl a medium sized vendor with symfony2 and others. Why not have a dedicated command for this? The class map only has to check before the psr-0 autoloading and then its even no problem if classes are missing from the map.

@RobLoach
Copy link
Contributor

Yeah, I'm on an almost 4GHz machine with a nice SSD, so the class map generation was quite fast. What would the command be named, do you think?

php composer.phar install --classmap

?

@HelloGrayson
Copy link

+1 on

php composer.phar install --classmap

@HelloGrayson
Copy link

We should check performance on Travis, with a rather large library like ZF1. I'd be happy to give this a go after merge.

@Seldaek
Copy link
Member Author

Seldaek commented Jun 21, 2012

As I said, I'd rather add caching than make this a specific command. Because it'll benefit everyone to have it enabled, and if we make it a separate command many people won't use it.

Maybe a flag to disable it in the cases where it's really annoying (like travis maybe makes not much sense since they build from scratch every time).

@HelloGrayson
Copy link

Interesting - I like the idea for classmap generation to be enabled by default and adding a way to disable it if necessary.

php composer.phar install --disable-classmap

@travisbot
Copy link

This pull request passes (merged e5e3c367 into f1320bf).

@henrikbjorn
Copy link
Contributor

php composer.phar install --no-classmap

Might be a better option name as many unix tools use this style

@schmittjoh
Copy link
Contributor

btw, has someone tested up to how many classes the map is faster, or what the memory impact of loading the map is?

@Seldaek
Copy link
Member Author

Seldaek commented Jun 26, 2012

@schmittjoh I did not check on memory, but that'd be interesting I guess. I tested up to a couple thousands, and it is so much faster I think it'd take a looot more to make it slower than PSR0. See https://github.com/Seldaek/autoload-bench for details. Feel free to add memory reporting to that.

@schmittjoh
Copy link
Contributor

Will do so when I find the time, just wanted to make sure that we have some scaling benchmarks (e.g. up until how many classes does this work).

@RobLoach
Copy link
Contributor

I did some benchmarks on this against Drupal 8: http://drupal.org/node/1663404 . In other news, this PR needs an update with minimum-stability.

@stof
Copy link
Contributor

stof commented Jul 14, 2012

@Seldaek any news here ? It seems that @weaverryan is about to update the Symfony doc about optimizing performances in prod, and this PR would make the autoloading section obsolete according to your benchmark (for which I don't find the gist with the results anymore) as it was faster than the ApcClassLoader.

weaverryan added a commit to symfony/symfony-docs that referenced this pull request Jul 14, 2012
…#1558 and thanks to @stof

Note, this may change again soon to include class map information with composer - see composer/composer#811
@Seldaek
Copy link
Member Author

Seldaek commented Jul 15, 2012

No news, I don't have time for this at the moment. The APC loader is a bit slower than classmap but still way faster than the universal one, so it's good enough for now I would say.

@travisbot
Copy link

This pull request fails (merged ee14950 into 289d23b).

@Seldaek Seldaek merged commit ee14950 into composer:master Aug 14, 2012
@Seldaek
Copy link
Member Author

Seldaek commented Aug 14, 2012

Ok I merged this but disabled it by default now. Use composer dump-autoload --optimize (or -o) to dump a faster autoloader. I'd love to get feedback especially if any errors occurs.

@HelloGrayson
Copy link

This is great. I'll check it out with my ZF1 project a bit later and report back.

https://github.com/breerly/zf1

@camspiers
Copy link

This is brilliant. @Seldaek no errors so far. Class map appears exactly as expected.

@halilim
Copy link

halilim commented Feb 22, 2013

Since this is not enabled by default -which would be better IMHO as @Seldaek pointed out, especially if the gains could be proven to be good on average- what about at least having this as a config item, e.g. "dump-autoload-optimized-auto" : true ? I guess having a way to vary this per environment (e.g. Travis) would be preferred, but AFAIK this would need changes that are quite out of the scope of this PR?

@Seldaek
Copy link
Member Author

Seldaek commented Feb 23, 2013

@halilim IMO the main benefit of this is in production, and for production you can usually easily add the -o flag on your install command in your deployment script. I don't really think an option makes it much easier.

@halilim
Copy link

halilim commented Feb 23, 2013

@Seldaek makes sense, thanks. Actually I meant my comment for situations where a deploy script/automation is not available. But I agree that implementing for those would be needlessly backwards.

@spawn-guy
Copy link

Is there a way to pre-generate this classmap and put it to project's repository? (as far as we have a .lock file with stored lib versions)

I've experienced an OutOfMemory when deployed a ZF1 project to Beanstalk's t1.micro, but it passed fine on a m1.small.

@Seldaek
Copy link
Member Author

Seldaek commented Apr 29, 2013

@spawn-guy you can pregenerate it, and deploy the whole project including vendor dir.

@spawn-guy
Copy link

i see a few downsides to this approach: we'll end up committing the whole vendor dir to repo; updating 1 lib will require +1 commit (and it might be not just a few files); and we Loose the whole Composer "idea".

and regarding Beanstalk itself:
Amazon does not support git-submodules, but they added Composer support.
BT's deployment process is the following: I git-push All my code to Amazon's GIT server, they clone it from inside Instances from their repo, they run Composer.

before Composer i had to support a different git-branch with ZF lib included in git tree, and a "clean" master branch just for this scenario. and i didn't liked that.

@Seldaek
Copy link
Member Author

Seldaek commented Apr 29, 2013

OK if you use beanstalk and don't really control the deployment process
that's tricky. Maybe you should just disable the optimization for now..

I wasn't suggesting you commit everything though, but if you deploy by
rsyncing from local dir for example, then you don't need to run composer
on the server.

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

Successfully merging this pull request may close these issues.