Skip to content

Gemspec cache resurrected, avoid side effects #1829

Merged
merged 3 commits into from Apr 13, 2012

2 participants

@dekellum
dekellum commented Apr 9, 2012

The gemspec cache is per #1635, previously merged to master at 3d4163a, but suffering a compatibly setback with d24e788 merged from the 1.1 branch, and reverted in 6b45e28. This pull restores the feature and adds a fix for the spec failure.

@indirect
Bundler member
indirect commented Apr 9, 2012

Great, thanks! Do you happen to have benchmarks for the new version of the patch?

@dekellum
dekellum commented Apr 9, 2012

The impact of this cache is very dependent on the what is computed in each gemspec and how many gemspecs the project has. What I've observed without the cache on master is that each gemspec is otherwise loaded (eval) twice as part of bundler/setup. I have a 17 gemspec project with a top level Gemfile. Without the cache, the second load of each gemspec is taking ~30ms per gem real time, for a total of ~500ms. This is time removed with the cache, and is perceptively worth while for my case given bundler/setup is required for each test run.

Obviously the gain will be much more incremental on a single gemspec project assuming low constant time for eval of the gemspec. But the change is pretty minimal and possibly more "correct": load each gemspec once? There have only been a few spec compatibility issues found when merging new specs from the 1-1 branch to master and no observed problems with actual bundler usage (I've been using the patch for a while with above referenced project and everything else in my dev environments.)

Can you accommodate this on second review?

@indirect indirect merged commit 51424a7 into bundler:master Apr 13, 2012
@indirect
Bundler member

Thanks for the fix, and I have merged now that the tests all pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.