Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Auto merge of #6145 - halfbyte:reduce_dependency_trees_bailout, r=seg…
Browse files Browse the repository at this point in the history
…iddins

Bail out of reducing depency trees on huge dependency conflict sets

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

The problem is described in #6114 - An unusually large set of conflicting dependencies leads to bundler get stuck and lead to very unhealthy amounts of memory and CPU time be consumed.

### What was your diagnosis of the problem?

This happens because because a too large array is fed into Array#combination in the "reduce_trees" lambda in `version_conflict_message`.

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

By simply bailing out when the conflict set is bigger than a certain size,
we'll get a result that is similar to what happened in earlier bundler versions
but skip a ton of CPU cycles and memory allocations.

I've chosen the limit rather unscientifically by playing around with the result
set sizes. 15 seems to be a good compromise but really anything larger than 10
should work (and with work I mean "should not usually not have to be invoked").

### Why did you choose this fix out of the possible options?

Reducing the conflict trees has been a rather new addition to bundler, so defaulting back to the old behavior of not reducing the trees seems like an OK option. It may be possible to also optimize the reduction algorithm, but since this is very much an edge case that only happens when using Bundler slightly out of the normal procedures, I think a simple bail out is sufficient.

(cherry picked from commit fc341ed)
  • Loading branch information
bundlerbot authored and segiddins committed Dec 12, 2017
1 parent 8e852c8 commit 3b71498
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/bundler/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ def version_conflict_message(e)
:solver_name => "Bundler",
:possibility_type => "gem",
:reduce_trees => lambda do |trees|
# bail out if tree size is too big for Array#combination to make any sense
return trees if trees.size > 15
maximal = 1.upto(trees.size).map do |size|
trees.map(&:last).flatten(1).combination(size).to_a
end.flatten(1).select do |deps|
Expand Down

0 comments on commit 3b71498

Please sign in to comment.