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

Provider.find_matches shoudn't be called while adding initial requirements #59

Open
McSinyx opened this issue Oct 30, 2020 · 2 comments

Comments

@McSinyx
Copy link
Contributor

McSinyx commented Oct 30, 2020

This is like pypa/pip#9071, but in a more generic sense. I suppose that this may ties to the algorithm, but I don't think the current algo requires this to be done. In short, before doing any resolution work, the resolver tries to find all the matches for each of the initial requirements:

for r in requirements:
try:
name, crit = self._merge_into_criterion(r, parent=None)
except RequirementsConflicted as e:
raise ResolutionImpossible(e.criterion.information)
self.state.criteria[name] = crit
self._r.starting()

Now suppose I have [A, B>6], where the first match for A is version 42 and A 42 depends on B<9, and that Provider.find_matches([B>6]) and Provider.find_matches([B>6,B<9]) do totally different things (or that the result of the underlying expensive operations can't be shared), then Provider.find_matches([B>6]) is wasteful.

In pip's case, of course the majority of the time it's reaching to simple repositories, which has an index shared for all B, so with caching/memoization it's not a problem though.

@uranusjr
Copy link
Member

uranusjr commented Nov 1, 2020

The initial round is implemented partially as a “fail fast” mechanism to report immediately if the user-supplied requirement set already contains conflicts. But the same can be achieved with appropriate ordering (e.g. pip’s current approach of prioritising resolution of user-requested packages), so indeed it’s possible maybe we can do away with that initial round. I have not thought too deep into the algorithm to figure out whether that’s correct or not, however. It’d probably be a good start to just remove that part of the code, and run the test suite with a tweaked get_preference() implementation.

@webknjaz
Copy link
Contributor

webknjaz commented Nov 4, 2020

👍 I'm currently integrating resolvelib into ansible-galaxy CLI and I've also noticed that find_matches happen earlier than the obvious conflicts are detected. So it does sound reasonable to do such an optimization.

One optimization (hack?) I see, on the caller side, is supplying a virtual requirement that depends on all the actual requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants