Skip to content

Commit b712a6b

Browse files
committed
perf: cache comparison key for choosing the next package to resolve
1 parent b7b1cf5 commit b712a6b

File tree

1 file changed

+111
-89
lines changed

1 file changed

+111
-89
lines changed

src/poetry/mixology/version_solver.py

Lines changed: 111 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import functools
55
import time
66

7+
from enum import IntEnum
78
from typing import TYPE_CHECKING
89
from typing import Optional
910

@@ -31,6 +32,22 @@
3132
_conflict = object()
3233

3334

35+
class Preference(IntEnum):
36+
"""
37+
Preference is one of the criteria for choosing which dependency to solve
38+
first. A higher value means that there are "more options" to satisfy
39+
a dependency. A lower value takes precedence.
40+
"""
41+
42+
DIRECT_ORIGIN = 0
43+
NO_CHOICE = 1
44+
USE_LATEST = 2
45+
LOCKED = 3
46+
DEFAULT = 4
47+
48+
49+
CompKey = tuple[Preference, int, bool, int]
50+
3451
DependencyCacheKey = tuple[
3552
str, Optional[str], Optional[str], Optional[str], Optional[str]
3653
]
@@ -149,6 +166,7 @@ def __init__(self, root: ProjectPackage, provider: Provider) -> None:
149166
int, set[Incompatibility]
150167
] = collections.defaultdict(set)
151168
self._solution = PartialSolution()
169+
self._get_comp_key_cached = functools.cache(self._get_comp_key)
152170

153171
@property
154172
def solution(self) -> PartialSolution:
@@ -432,107 +450,111 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility
432450

433451
raise SolveFailureError(incompatibility)
434452

435-
def _choose_next(self, unsatisfied: list[Dependency]) -> Dependency:
453+
def _get_comp_key(self, dependency: Dependency) -> CompKey:
436454
"""
455+
Returns a tuple of
456+
- preference
457+
- num_deps_upper_bound
458+
- has_deps
459+
- num_packages
460+
that serves as priority for choosing the next package to resolve.
461+
(A lower value takes precedence.)
462+
463+
In order to provide results that are as deterministic as possible
464+
and consistent between `poetry lock` and `poetry update`, the return value
465+
of two different dependencies should not be equal if possible.
466+
467+
## preference
468+
469+
See Preference class.
470+
471+
## num_deps_upper_bound
472+
473+
A dependency with an upper bound is more likely to cause conflicts. Therefore,
474+
a package with more dependencies with upper bounds should be chosen first.
475+
476+
## has_deps
477+
478+
A package with dependencies should be chosen first
479+
because a package without dependencies is less likely to cause conflicts.
480+
481+
## num_packages
482+
437483
The original algorithm proposes to prefer packages with as few remaining
438484
versions as possible, so that if a conflict is necessary it's forced quickly.
439485
https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making
440486
However, this leads to the famous boto3 vs. urllib3 issue, so we prefer
441487
packages with more remaining versions (see
442488
https://github.com/python-poetry/poetry/pull/8255#issuecomment-1657198242
443489
for more details).
444-
In order to provide results that are as deterministic as possible
445-
and consistent between `poetry lock` and `poetry update`, the return value
446-
of two different dependencies should not be equal if possible.
447490
"""
448-
449-
class Preference:
450-
"""
451-
Preference is one of the criteria for choosing which dependency to solve
452-
first. A higher value means that there are "more options" to satisfy
453-
a dependency. A lower value takes precedence.
454-
"""
455-
456-
DIRECT_ORIGIN = 0
457-
NO_CHOICE = 1
458-
USE_LATEST = 2
459-
LOCKED = 3
460-
DEFAULT = 4
461-
462-
def _get_min(dependency: Dependency) -> tuple[int, int, bool, int]:
463-
"""
464-
Returns a tuple of:
465-
- preference: see Preference class
466-
- num_deps_upper_bound: a dependency with an upper bound is more likely to
467-
cause conflicts -> a package with more dependencies
468-
with upper bounds should be chosen first
469-
- has_deps: a package with dependencies should be chosen first because
470-
a package without dependencies is less likely to cause conflicts
471-
- num_packages: see explanation above
472-
"""
473-
# Direct origin dependencies must be handled first: we don't want to resolve
474-
# a regular dependency for some package only to find later that we had a
475-
# direct-origin dependency.
476-
if dependency.is_direct_origin():
477-
return Preference.DIRECT_ORIGIN, 0, False, 0
478-
479-
use_latest = dependency.name in self._provider.use_latest
480-
if not use_latest:
481-
locked = self._provider.get_locked(dependency)
482-
if locked:
483-
return Preference.LOCKED, 0, False, 0
484-
485-
packages = self._dependency_cache.search_for(
486-
dependency, self._solution.decision_level
487-
)
488-
num_packages = len(packages)
489-
if packages:
490-
package = packages[0].package
491-
if package.is_root():
492-
relevant_dependencies = package.all_requires
493-
else:
494-
if not package.is_direct_origin():
495-
# We have to get the package from the pool,
496-
# otherwise `requires` will be empty.
497-
#
498-
# We might need `package.source_reference` as fallback
499-
# for transitive dependencies without a source
500-
# if there is a top-level dependency
501-
# for the same package with an explicit source.
502-
for repo in (dependency.source_name, package.source_reference):
503-
try:
504-
package = self._provider.get_package_from_pool(
505-
package.pretty_name,
506-
package.version,
507-
repository_name=repo,
508-
)
509-
except Exception:
510-
pass
511-
else:
512-
break
513-
514-
relevant_dependencies = [
515-
r
516-
for r in package.requires
517-
if not r.in_extras or r.in_extras[0] in dependency.extras
518-
]
519-
has_deps = bool(relevant_dependencies)
520-
num_deps_upper_bound = sum(
521-
1 for d in relevant_dependencies if d.constraint.has_upper_bound()
522-
)
491+
# Direct origin dependencies must be handled first: we don't want to resolve
492+
# a regular dependency for some package only to find later that we had a
493+
# direct-origin dependency.
494+
if dependency.is_direct_origin():
495+
return Preference.DIRECT_ORIGIN, 0, False, 0
496+
497+
use_latest = dependency.name in self._provider.use_latest
498+
if not use_latest:
499+
locked = self._provider.get_locked(dependency)
500+
if locked:
501+
return Preference.LOCKED, 0, False, 0
502+
503+
packages = self._dependency_cache.search_for(
504+
dependency, self._solution.decision_level
505+
)
506+
num_packages = len(packages)
507+
if packages:
508+
package = packages[0].package
509+
if package.is_root():
510+
relevant_dependencies = package.all_requires
523511
else:
524-
has_deps = False
525-
num_deps_upper_bound = 0
512+
if not package.is_direct_origin():
513+
# We have to get the package from the pool,
514+
# otherwise `requires` will be empty.
515+
#
516+
# We might need `package.source_reference` as fallback
517+
# for transitive dependencies without a source
518+
# if there is a top-level dependency
519+
# for the same package with an explicit source.
520+
for repo in (dependency.source_name, package.source_reference):
521+
try:
522+
package = self._provider.get_package_from_pool(
523+
package.pretty_name,
524+
package.version,
525+
repository_name=repo,
526+
)
527+
except Exception:
528+
pass
529+
else:
530+
break
531+
532+
relevant_dependencies = [
533+
r
534+
for r in package.requires
535+
if not r.in_extras or r.in_extras[0] in dependency.extras
536+
]
537+
has_deps = bool(relevant_dependencies)
538+
num_deps_upper_bound = sum(
539+
1 for d in relevant_dependencies if d.constraint.has_upper_bound()
540+
)
541+
else:
542+
has_deps = False
543+
num_deps_upper_bound = 0
526544

527-
if num_packages < 2:
528-
preference = Preference.NO_CHOICE
529-
elif use_latest:
530-
preference = Preference.USE_LATEST
531-
else:
532-
preference = Preference.DEFAULT
533-
return preference, -num_deps_upper_bound, not has_deps, -num_packages
545+
if num_packages < 2:
546+
preference = Preference.NO_CHOICE
547+
elif use_latest:
548+
preference = Preference.USE_LATEST
549+
else:
550+
preference = Preference.DEFAULT
551+
return preference, -num_deps_upper_bound, not has_deps, -num_packages
534552

535-
return min(unsatisfied, key=_get_min)
553+
def _choose_next(self, unsatisfied: list[Dependency]) -> Dependency:
554+
"""
555+
Chooses the next package to resolve.
556+
"""
557+
return min(unsatisfied, key=self._get_comp_key_cached)
536558

537559
def _choose_package_version(self) -> str | None:
538560
"""

0 commit comments

Comments
 (0)