[WIP] Experimental Gemfile checksum feature #5584

hmistry · 2017-04-13T01:57:04Z

Generate checksum for Gemfile and save to lock file. Use the previous…ly saved checksum in the lock file to compare against the current Gemfile checksum to determine if there were any changes made. If no changes, then skip resolution.

This is a quick hack to evaluate if there is any performance benefit of this feature. For a single rails gem Gemfile, it saved 130ms on my local machine (0.430s to 0.3s) to run bundle install.

Submitting PR for feedback (@indirect , @segiddins) and other possible performance tests to evaluate further.

Issue #4871

…ly saved checksum in the lock file to compare against the current Gemfile checksum to determine if there were any changes made. If no changes, then skip resolution.

indirect · 2017-04-13T02:04:20Z

@hmistry thanks! I'm looking forward to seeing how this turns out.

@jules2689 I'm interested to see if this works for you, and if it saves any time on exec or install.

colby-swandale · 2017-04-13T03:28:51Z

@indirect IIRC we were talking about this a while back and there was talk that this could be an issue for Gemfiles that has ruby that is evaluated? Does this need to have an opt-out/opt-in flag?

segiddins · 2017-04-13T03:34:37Z

lib/bundler/installer.rb

@@ -75,6 +75,10 @@ def run(options)
        return
      end

+
+      # EXPERIMENT: Evaluate benefit if we skip resolution by comparing Gemfile checksum against the checksum saved in the lockfile.
+      return if Definition.build(Bundler.default_gemfile, Bundler.default_lockfile, nil).checksum_matches?


this will also skip installation even if none of the gems are actually installed.... I think maybe you might've meant something like the following?

diff --git a/lib/bundler/installer.rb b/lib/bundler/installer.rb index cbfdddeaf..6474d182f 100644 --- a/lib/bundler/installer.rb +++ b/lib/bundler/installer.rb @@ -212,6 +212,8 @@ module Bundler end def resolve_if_need(options) + return if @definition.nothing_changed? + if !options[“update”] && !options[:inline] && Bundler.default_lockfile.file? local = Bundler.ui.silence do begin

I already mentioned to @segiddins but for documentation purposes, this did not result in any time improvement.

jules2689 · 2017-04-13T14:18:32Z

Gemfiles that has ruby that is evaluated

That would be a problem for us, yea. We do some juggling magic to support 2 Rails versions at a time.

Could this be used for the bundle check command maybe?

I'm interested to see if this works for you, and if it saves any time on exec or install.

@indirect I'm happy to test this out, but it needs a rebase on master. It's missing the work that we've put in. I rebased locally to get the numbers below.

	1.14.6	master	this branch
`bundle install` with all deps met	3.67s	3.4s	0.67s
`bundle install` with a missing dep	15-16s	15-16s	ERROR

The big difference I noticed is that there is less output, which is likely saving a lot of time so 💯

ERROR

~/src/github.com/Shopify/shopify(master*) ➜ time bundle install
Could not find json_schema-0.14.3 in any of the sources

real	0m0.639s
user	0m0.542s
sys	0m0.057s

jules2689 · 2017-04-13T14:35:37Z

Gemfiles that has ruby that is evaluated

Actually, looking at the way we generate the checksum we can likely hit most use cases by supporting a separate gemfile lockfile (shopify's use case) via an env var*, combine that with an opt-out and we should be good.

* We have a single Gemfile so that people don't forget to add stuff, and resolve to separate lockfiles.

jules2689 · 2017-04-13T16:59:13Z

lib/bundler/dsl.rb

@@ -449,6 +451,10 @@ def warn_deprecated_git_source(name, repo_string)
      EOS
    end

+    def generate_checksum(gemfile_source)
+      Digest::MD5.hexdigest(gemfile_source)


We can't use MD5 because of FIPS :( #4989

jules2689 · 2017-04-13T17:00:26Z

Another thing to note here is that gemfiles can source other gemfiles, which has been suggested in many instances such as #183 (comment)

hmistry · 2017-04-13T17:14:23Z

@jules2689 and @colby-swandale thanks for all your feedback. This quick hack does not account for all the edge cases and was simply done to evaluate what performance benefit we could get from this approach.
Looking at the initial data, thanks to @jules2689, there is a good gain so now the challenge will be to see how to address each of the edge cases and trade off with the time improvement.

bundlerbot · 2017-06-16T21:49:07Z

☔ The latest upstream changes (presumably #5718) made this pull request unmergeable. Please resolve the merge conflicts.

colby-swandale · 2017-10-03T21:00:21Z

ping @hmistry is still being worked on?

hmistry · 2017-10-03T23:56:14Z

@colby-swandale I haven't had a chance to work on this since then. It's more of an exploration to see if there's any improvement. Is this something needed soon?

colby-swandale · 2017-10-08T01:40:31Z

No but let us know if you're not working on this anymore.

agrim123 · 2018-03-14T14:45:17Z

@hmistry Are you still working on this? If not, I would be happy to continue the work!

segiddins · 2018-03-14T15:03:39Z

I had something a bit more advanced in https://github.com/bundler/bundler/compare/seg-definition-from-lockfile, you might want to take a look at that as well

hmistry · 2018-03-14T15:27:02Z

@agrim123 No, I haven't had time to continue working on this. Please go ahead.

agrim123 · 2018-03-16T00:01:44Z

@segiddins Can you explain what optional_groups here means exactly?
Why do we need static_gemfile option?
Is there a need to store as key-value pair for Gemfile, when the only checksum of the corresponding gemfile matters, as did here? I didn't quite understand this.

segiddins · 2018-03-17T16:59:49Z

It’s needed because some people use dynamic ruby code in their gemfile, so simply checking the checksum matches is insufficient to guarantee a semantic match

agrim123 · 2018-03-18T10:11:21Z

@segiddins I had one more doubt.
If I add a gem manually to the gems.rb and then run bundle install, then the variable static_gemfile does not gets updated and thus no GEMFILE CHECKSUMS section is created. When do we want to save the checksum?

segiddins · 2018-03-18T23:55:17Z

As things are on my branch, you need to manually call_static_gemfile!() in your Gemfile to opt-in to the behavior

Generate checksum for Gemfile and save to lock file. Use the previous…

d39330c

…ly saved checksum in the lock file to compare against the current Gemfile checksum to determine if there were any changes made. If no changes, then skip resolution.

segiddins reviewed Apr 13, 2017

View reviewed changes

jules2689 reviewed Apr 13, 2017

View reviewed changes

agrim123 mentioned this pull request Mar 19, 2018

[WIP] Lockfile checksum #6448

Closed

hsbt closed this Apr 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Experimental Gemfile checksum feature #5584

[WIP] Experimental Gemfile checksum feature #5584

hmistry commented Apr 13, 2017 •

edited

Loading

indirect commented Apr 13, 2017

colby-swandale commented Apr 13, 2017 •

edited

Loading

segiddins Apr 13, 2017

hmistry Apr 13, 2017

jules2689 commented Apr 13, 2017

jules2689 commented Apr 13, 2017

jules2689 Apr 13, 2017

jules2689 commented Apr 13, 2017

hmistry commented Apr 13, 2017

bundlerbot commented Jun 16, 2017

colby-swandale commented Oct 3, 2017

hmistry commented Oct 3, 2017

colby-swandale commented Oct 8, 2017

agrim123 commented Mar 14, 2018

segiddins commented Mar 14, 2018

hmistry commented Mar 14, 2018

agrim123 commented Mar 16, 2018 •

edited

Loading

segiddins commented Mar 17, 2018

agrim123 commented Mar 18, 2018

segiddins commented Mar 18, 2018

[WIP] Experimental Gemfile checksum feature #5584

[WIP] Experimental Gemfile checksum feature #5584

Conversation

hmistry commented Apr 13, 2017 • edited Loading

indirect commented Apr 13, 2017

colby-swandale commented Apr 13, 2017 • edited Loading

segiddins Apr 13, 2017

Choose a reason for hiding this comment

hmistry Apr 13, 2017

Choose a reason for hiding this comment

jules2689 commented Apr 13, 2017

ERROR

jules2689 commented Apr 13, 2017

jules2689 Apr 13, 2017

Choose a reason for hiding this comment

jules2689 commented Apr 13, 2017

hmistry commented Apr 13, 2017

bundlerbot commented Jun 16, 2017

colby-swandale commented Oct 3, 2017

hmistry commented Oct 3, 2017

colby-swandale commented Oct 8, 2017

agrim123 commented Mar 14, 2018

segiddins commented Mar 14, 2018

hmistry commented Mar 14, 2018

agrim123 commented Mar 16, 2018 • edited Loading

segiddins commented Mar 17, 2018

agrim123 commented Mar 18, 2018

segiddins commented Mar 18, 2018

hmistry commented Apr 13, 2017 •

edited

Loading

colby-swandale commented Apr 13, 2017 •

edited

Loading

agrim123 commented Mar 16, 2018 •

edited

Loading