Handle restricted dependencies as implicit multiple-constraints dependencies

[radoering]

Pull Request Check List

Resolves: #5506

• Added tests for changed code.
• Updated documentation for changed code.

Although I think that this PR makes the solver more correct it comes with a massive performance regression that is far from acceptible.

I carried out some measurements with example pyproject.toml files from other PRs. If locking succeeds without this PR, the same lock file is generated with this PR, it just takes longer...

Times for poetry lock with a warm cache:

pyproject.toml from ...	time without PR	time with PR
#3367	0.8 s	2.8 s
#4670	1.8 s	4.2 s
#4870	71 s	11800 s (not a typo)
#5506	error after 4.5 s	1090 s
shootout example	3.9 s	250 s

Number of overrides:

pyproject.toml from ...	number of overrides without PR	number of overrides with PR
#3367	4	10
#4670	16	19
#4870	46	4179
#5506	-	288
shootout example	0	69

The data shows that the time seems to correlate with the number of overrides. Thus, I assume a more sophisticated algorithm to reduce the number of overrides or even a complete overhaul of how to handle multiple-constraints dependencies might be necessary. I can imagine to make the VersionSolver marker aware so that a version conflict is only a conflict if the intersection of markers is not empty. This way, overrides would not be necessary anymore and everything could be solved at once. However, that's probably a huge task.

[jeertmans]

Thanks for linking this to #8670 @radoering :-)

Maybe we should rewrite Poetry in Rust if speed is an issue ^^'

Jokes aside, having a resolving time this long is really an issue..

[jorenham]

I left some notes on potential performance improvements; perhaps it could help speed things up :)

[jorenham]

Suggested change

	return [dep for deps in by_name.values() for dep in deps]
	return itertools.chain.from_iterable(by_name.values())

[jorenham]

The return value here is used only for _get_dependencies_with_overrides, which (unlike the annotations suggest), should accept any Iterable[Dependency].
So it doesn't need to return a list; any iterable will do:

Suggested change

	) -> list[Dependency]:
	) -> Iterable[Dependency]:

With this, you can avoid creating the entire dependency list, e.g. using itertools, or by turning this method into a generator .

[jorenham]

These loops could be merged if 1) you use itertools.groupby, with e.g. operators.attrgetter('name') as key function, and 2) turn this method into a generator (e.g. with a yield from in the first if statement, and yield in the second).
This way you can avoid creating temporary lists altogether, for a significant speedup.

[jorenham]

Is marker_union also needed when e.g. len(deps) == 1?
Because, at a glance, marker_union looks like a rather expensive function call.

[jorenham]

Since _dependencies is only used once, it's probably better to skip the variable assignment, by inlining in into the _add_implicit_dependencies call

[jorenham]

The list.pop method can be very slow operation, and I think that it can be avoided here, by using a "blacklist" approach, e.g.

blacklist = set()
for dep in dependencies:
    if dep.constraint.is_empty():
        blacklist.add(dep)
        break

Then later on in itertools.product use
repeat=len(dependencies) - len(blacklist).
And when looping over dep in dependencies again, simply skip it if dep in blacklist.

This avoids the list.pop operation, which has a time-complexity O(n), by relying on set.__contains__, which is only O(1).

[jorenham]

I don't think python<3.7 is relevant anymore

[dimbleby]

I left some notes on potential performance improvements; perhaps it could help speed things up :)

I think it is likely that you are micro-optimizing essentially irrelevant parts of the code. If you want to make performance improvements - recommend that the first thing to do is to profile, so that you spend your time optimizing the right things

But perhaps I am wrong, and you are now seeing results much better than those in the comment at the top of the thread? If so - submit a merge request!

[jorenham]

I left some notes on potential performance improvements; perhaps it could help speed things up :)

I think it is likely that you are micro-optimizing essentially irrelevant parts of the code.

I don't agree that improvements to the runtime complexity are the same as "micro-optimizing".

Plus, my suggestions will also result in fewer lines of code, without harming readability. So even if the performance benefits are minimal, at the very least there are no disadvantages.