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

Restore 2.1.4 resolution times #4134

Merged
merged 15 commits into from
Dec 14, 2020

Conversation

deivid-rodriguez
Copy link
Member

What was the end-user or developer problem that led to this PR?

The problem is that the fixes we made in 2.2.0 to get a correct resolution regarding platform specific gems made the resolution much slower.

What is your fix for the problem, implemented in this PR?

My fix is to make several performance improvements to get the times back to what they were.

The improvements (not always performance, sometimes are refactorings to prepare for the performance improvements) are explained on each commit.

The main changes are:

  • Stop resolving for both RUBY and the specific platform. Resolving for the specific platform is enough since it includes fallback code already. This improvement makes the sample Gemfile resolve again, although it takes more than 1 minute on my machine.
  • Cache the set of spec groups for each dependency. This one drops the resolution time back to ~2.1.4 times.

Fixes #4128.

Make sure the following tasks are checked

The `unless` is never met since otherwise the previous line would raise.
This should be more efficient, since it doesn't resolve platforms every
time, but also helps me with adding a `force_ruby_platform` option to
the gem DSL, since one top level dependency using this option will be
resolved, all transitive dependencies will automatically use the
memoized value and also respect the option.
It's only used here, so no need for memoization.
On 2.6.6, the default version of bundler is 1.17.2. If we use 1.10.1
here, rubygems will actually activate the highest version of bundler 1.x
available, thus, the default. At least, rubygems versions before we
started doing "exact version switching".

We never want our specs to activate the default version of bundler, but
our own code. So I change the spec to use a dummy version of bundler
higher than 1.17.2.
With the new platform specific behavior where resolving happens against
the specific platform bundler being run from, we no longer need to keep
resolving for the generic ruby platform. The specific_platform logic
already includes fallback spec groups so that we always default to the
generic platform when no more specific variants are found.

Resolving both for the generic ruby platform and the specific platform
was creating a big overhead in performance to the point where some
couldn't be resolved in reasonable time.

The ruby platform is no longer added to the lockfile unless we're using
the `force_ruby_platform` setting. To play nice with old Gemfiles
including the RUBY platform in the list of platforms while running in
frozen mode, I added some compatibility code to deal with that.

I also added a realworld spec to prove the performance improvements. With
these changes, it takes ~10s (around the same as in 2.1.4), without
these changes it hangs.
@deivid-rodriguez deivid-rodriguez merged commit 4c16f55 into master Dec 14, 2020
@deivid-rodriguez deivid-rodriguez deleted the slow_platforms_specific_resolution branch December 14, 2020 09:36
deivid-rodriguez added a commit that referenced this pull request Dec 14, 2020
…tion

Restore 2.1.4 resolution times

(cherry picked from commit 4c16f55)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite loop when bundling gems
2 participants