Skip to content

Commit 183a6de

Browse files
committed
perf: cache comparison key for choosing the next package to resolve
1 parent ab12e13 commit 183a6de

File tree

1 file changed

+108
-89
lines changed

1 file changed

+108
-89
lines changed

src/poetry/mixology/version_solver.py

Lines changed: 108 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@
3131
_conflict = object()
3232

3333

34+
class Preference:
35+
"""
36+
Preference is one of the criteria for choosing which dependency to solve
37+
first. A higher value means that there are "more options" to satisfy
38+
a dependency. A lower value takes precedence.
39+
"""
40+
41+
DIRECT_ORIGIN = 0
42+
NO_CHOICE = 1
43+
USE_LATEST = 2
44+
LOCKED = 3
45+
DEFAULT = 4
46+
47+
3448
DependencyCacheKey = tuple[
3549
str, Optional[str], Optional[str], Optional[str], Optional[str]
3650
]
@@ -149,6 +163,7 @@ def __init__(self, root: ProjectPackage, provider: Provider) -> None:
149163
int, set[Incompatibility]
150164
] = collections.defaultdict(set)
151165
self._solution = PartialSolution()
166+
self._get_comp_key_cached = functools.cache(self._get_comp_key)
152167

153168
@property
154169
def solution(self) -> PartialSolution:
@@ -432,107 +447,111 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility
432447

433448
raise SolveFailureError(incompatibility)
434449

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

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
542+
if num_packages < 2:
543+
preference = Preference.NO_CHOICE
544+
elif use_latest:
545+
preference = Preference.USE_LATEST
546+
else:
547+
preference = Preference.DEFAULT
548+
return preference, -num_deps_upper_bound, not has_deps, -num_packages
534549

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

537556
def _choose_package_version(self) -> str | None:
538557
"""

0 commit comments

Comments
 (0)