Add simple cache to Bundler.load_gemspec #1635

Closed
wants to merge 1 commit into from

3 participants

@dekellum

In some projects, for example those with 10s of local gemspecs, use of `git ls-files` for spec.files, or specs requiring significant source to get at a spec.version; this change to cache load_gemspec results in notable performance gains.

The cache also avoids loading these same gemspecs repeatedly with different LOAD_PATH values.

This came up originally in #1481 (\cc @sunaku, @tenderlove). Only a sub-case (specs from rubygems) of that original issue was dealt with before it was closed (#1567). There also, @indirect suggests "[caching] seems like a good plan eventually"

Perhaps this is actually easy enough, safe enough (I find no spec regressions) and suitable for 1.1?

@dekellum dekellum Add simple cache to Bundler.load_gemspec
In some projects, for example those with 10s of local gemspecs, use of
`git ls-files` for spec.files, or specs requiring significant source
to get at a spec.version; this change to cache load_gemspec results in
notable performance gains.

The cache also avoids loading these same gemspecs repeatedly with
different LOAD_PATH values.
f8b5066
@indirect
Bundler member

This seems like a good idea to me. I'm wary of potentially introducing issues late in the 1.1 release cycle, but hopeful that we can squeeze this in to the next RC.

@sunaku

+1

@dekellum

Fair enough regarding 1.1 release cycle. Would master be the next development branch? Would it be reasonable to merge and vet this change there?

@indirect
Bundler member

I've merged this to master as 051c7a921de855139b3779cec1e5fdb7542b417c. Thanks for the patch!

@indirect indirect closed this Jan 21, 2012
@dekellum

@indirect Thanks! When you going to push this commit?

@dekellum

Hey @indirect, sorry to be a bother, but did this merge commit get lost in the shuffle?

@indirect
Bundler member

Oops! Not sure how I missed pushing before. Thanks for reminding me about this. I've pushed your patch to the tip of master now, as 3d4163a.

@dekellum

And reverted in 6b45e28. *sigh* Will try to bring it back.

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