From f5480f10c7198bbf74e741e97cb0c6420c0f0ce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Thu, 28 Dec 2023 12:42:29 +0100 Subject: [PATCH 1/5] Cache is_satisfied_by --- .../resolution/resolvelib/provider.py | 2 ++ .../resolution/resolvelib/requirements.py | 36 +++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index 315fb9c8902..ad9bfb2b97b 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -1,5 +1,6 @@ import collections import math +from functools import lru_cache from typing import ( TYPE_CHECKING, Dict, @@ -236,6 +237,7 @@ def _eligible_for_upgrade(identifier: str) -> bool: incompatibilities=incompatibilities, ) + @lru_cache(maxsize=None) def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool: return requirement.is_satisfied_by(candidate) diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 4af4a9f25a6..7f6e72bad18 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -1,8 +1,11 @@ +from typing import Any + from pip._vendor.packaging.specifiers import SpecifierSet from pip._vendor.packaging.utils import NormalizedName, canonicalize_name from pip._internal.req.constructors import install_req_drop_extras from pip._internal.req.req_install import InstallRequirement +from pip._internal.utils.models import KeyBasedCompareMixin from .base import Candidate, CandidateLookup, Requirement, format_name @@ -17,6 +20,14 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"{self.__class__.__name__}({self.candidate!r})" + def __hash__(self) -> int: + return hash(self.candidate) + + def __eq__(self, other: Any) -> bool: + if not isinstance(other, ExplicitRequirement): + return False + return self.candidate == other.candidate + @property def project_name(self) -> NormalizedName: # No need to canonicalize - the candidate did this @@ -37,11 +48,14 @@ def is_satisfied_by(self, candidate: Candidate) -> bool: return candidate == self.candidate -class SpecifierRequirement(Requirement): +class SpecifierRequirement(Requirement, KeyBasedCompareMixin): def __init__(self, ireq: InstallRequirement) -> None: assert ireq.link is None, "This is a link, not a specifier" self._ireq = ireq self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras) + KeyBasedCompareMixin.__init__( + self, key=str(ireq), defining_class=SpecifierRequirement + ) def __str__(self) -> str: return str(self._ireq.req) @@ -97,6 +111,9 @@ def __init__(self, ireq: InstallRequirement) -> None: assert ireq.link is None, "This is a link, not a specifier" self._ireq = install_req_drop_extras(ireq) self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras) + KeyBasedCompareMixin.__init__( + self, key=str(ireq), defining_class=SpecifierRequirement + ) class RequiresPythonRequirement(Requirement): @@ -104,6 +121,7 @@ class RequiresPythonRequirement(Requirement): def __init__(self, specifier: SpecifierSet, match: Candidate) -> None: self.specifier = specifier + self._specifier_string = str(specifier) # for faster __eq__ self._candidate = match def __str__(self) -> str: @@ -112,6 +130,17 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"{self.__class__.__name__}({str(self.specifier)!r})" + def __hash__(self) -> int: + return hash((self._specifier_string, self._candidate)) + + def __eq__(self, other: Any) -> bool: + if not isinstance(other, RequiresPythonRequirement): + return False + return ( + self._specifier_string == other._specifier_string + and self._candidate == other._candidate + ) + @property def project_name(self) -> NormalizedName: return self._candidate.project_name @@ -136,11 +165,14 @@ def is_satisfied_by(self, candidate: Candidate) -> bool: return self.specifier.contains(candidate.version, prereleases=True) -class UnsatisfiableRequirement(Requirement): +class UnsatisfiableRequirement(Requirement, KeyBasedCompareMixin): """A requirement that cannot be satisfied.""" def __init__(self, name: NormalizedName) -> None: self._name = name + KeyBasedCompareMixin.__init__( + self, key=str(name), defining_class=UnsatisfiableRequirement + ) def __str__(self) -> str: return f"{self._name} (unavailable)" From e16867e20fcc7a77695fcd95f781d64ac1136638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Thu, 28 Dec 2023 13:18:58 +0100 Subject: [PATCH 2/5] Use the caching variant of is_satified_by in Factory.find_candidates --- src/pip/_internal/resolution/resolvelib/factory.py | 4 +++- src/pip/_internal/resolution/resolvelib/provider.py | 1 + tests/unit/resolution_resolvelib/test_requirement.py | 10 ++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 781c54fd83a..e36df15d459 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -3,6 +3,7 @@ import logging from typing import ( TYPE_CHECKING, + Callable, Dict, FrozenSet, Iterable, @@ -391,6 +392,7 @@ def find_candidates( incompatibilities: Mapping[str, Iterator[Candidate]], constraint: Constraint, prefers_installed: bool, + is_satisfied_by: Callable[[Requirement, Candidate], bool], ) -> Iterable[Candidate]: # Collect basic lookup information from the requirements. explicit_candidates: Set[Candidate] = set() @@ -456,7 +458,7 @@ def find_candidates( for c in explicit_candidates if id(c) not in incompat_ids and constraint.is_satisfied_by(c) - and all(req.is_satisfied_by(c) for req in requirements[identifier]) + and all(is_satisfied_by(req, c) for req in requirements[identifier]) ) def _make_requirements_from_install_req( diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index ad9bfb2b97b..fb0dd85f112 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -235,6 +235,7 @@ def _eligible_for_upgrade(identifier: str) -> bool: constraint=constraint, prefers_installed=(not _eligible_for_upgrade(identifier)), incompatibilities=incompatibilities, + is_satisfied_by=self.is_satisfied_by, ) @lru_cache(maxsize=None) diff --git a/tests/unit/resolution_resolvelib/test_requirement.py b/tests/unit/resolution_resolvelib/test_requirement.py index b8cd13cb566..b7b0395b037 100644 --- a/tests/unit/resolution_resolvelib/test_requirement.py +++ b/tests/unit/resolution_resolvelib/test_requirement.py @@ -23,6 +23,14 @@ # Editables +def _is_satisfied_by(requirement: Requirement, candidate: Candidate) -> bool: + """A helper function to check if a requirement is satisfied by a candidate. + + Used for mocking PipProvider.is_satified_by. + """ + return requirement.is_satisfied_by(candidate) + + @pytest.fixture def test_cases(data: TestData) -> Iterator[List[Tuple[str, str, int]]]: def _data_file(name: str) -> Path: @@ -80,6 +88,7 @@ def test_new_resolver_correct_number_of_matches( {}, Constraint.empty(), prefers_installed=False, + is_satisfied_by=_is_satisfied_by, ) assert sum(1 for _ in matches) == match_count @@ -98,6 +107,7 @@ def test_new_resolver_candidates_match_requirement( {}, Constraint.empty(), prefers_installed=False, + is_satisfied_by=_is_satisfied_by, ) for c in candidates: assert isinstance(c, Candidate) From 705ce7d5cf8300da8abe9456a245c9c4ac1c7173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Thu, 28 Dec 2023 16:33:41 +0100 Subject: [PATCH 3/5] Optimize hashing and comparison of AlreadyInstalledCandidate --- .../_internal/resolution/resolvelib/candidates.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 49a1f660ca6..034fc343975 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -20,6 +20,7 @@ from pip._internal.req.req_install import InstallRequirement from pip._internal.utils.direct_url_helpers import direct_url_from_link from pip._internal.utils.misc import normalize_version_info +from pip._internal.utils.models import KeyBasedCompareMixin from .base import Candidate, CandidateVersion, Requirement, format_name @@ -324,7 +325,7 @@ def _prepare_distribution(self) -> BaseDistribution: return self._factory.preparer.prepare_editable_requirement(self._ireq) -class AlreadyInstalledCandidate(Candidate): +class AlreadyInstalledCandidate(Candidate, KeyBasedCompareMixin): is_installed = True source_link = None @@ -346,20 +347,16 @@ def __init__( skip_reason = "already satisfied" factory.preparer.prepare_installed_requirement(self._ireq, skip_reason) + KeyBasedCompareMixin.__init__( + self, key=(self.name, self.version), defining_class=self.__class__ + ) + def __str__(self) -> str: return str(self.dist) def __repr__(self) -> str: return f"{self.__class__.__name__}({self.dist!r})" - def __hash__(self) -> int: - return hash((self.__class__, self.name, self.version)) - - def __eq__(self, other: Any) -> bool: - if isinstance(other, self.__class__): - return self.name == other.name and self.version == other.version - return False - @property def project_name(self) -> NormalizedName: return self.dist.canonical_name From efab55099b400b0fd77a8cac2ace66e6e19d7ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 21 Apr 2024 13:35:49 +0200 Subject: [PATCH 4/5] Add news --- news/12453.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/12453.feature.rst diff --git a/news/12453.feature.rst b/news/12453.feature.rst new file mode 100644 index 00000000000..704cd012a90 --- /dev/null +++ b/news/12453.feature.rst @@ -0,0 +1 @@ +Improve performance of resolution of large dependency trees, with more caching. From 38b56452ad23d68901409da30ee07ad2f0466581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 21 Apr 2024 14:01:47 +0200 Subject: [PATCH 5/5] Don't use KeyBasedCompareMixin --- .../resolution/resolvelib/candidates.py | 15 +++++--- .../resolution/resolvelib/requirements.py | 38 +++++++++++++------ 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 034fc343975..140aaafcb8f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -20,7 +20,6 @@ from pip._internal.req.req_install import InstallRequirement from pip._internal.utils.direct_url_helpers import direct_url_from_link from pip._internal.utils.misc import normalize_version_info -from pip._internal.utils.models import KeyBasedCompareMixin from .base import Candidate, CandidateVersion, Requirement, format_name @@ -325,7 +324,7 @@ def _prepare_distribution(self) -> BaseDistribution: return self._factory.preparer.prepare_editable_requirement(self._ireq) -class AlreadyInstalledCandidate(Candidate, KeyBasedCompareMixin): +class AlreadyInstalledCandidate(Candidate): is_installed = True source_link = None @@ -347,16 +346,20 @@ def __init__( skip_reason = "already satisfied" factory.preparer.prepare_installed_requirement(self._ireq, skip_reason) - KeyBasedCompareMixin.__init__( - self, key=(self.name, self.version), defining_class=self.__class__ - ) - def __str__(self) -> str: return str(self.dist) def __repr__(self) -> str: return f"{self.__class__.__name__}({self.dist!r})" + def __eq__(self, other: object) -> bool: + if not isinstance(other, AlreadyInstalledCandidate): + return NotImplemented + return self.name == other.name and self.version == other.version + + def __hash__(self) -> int: + return hash((self.name, self.version)) + @property def project_name(self) -> NormalizedName: return self.dist.canonical_name diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 7f6e72bad18..f980a356f18 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -5,7 +5,6 @@ from pip._internal.req.constructors import install_req_drop_extras from pip._internal.req.req_install import InstallRequirement -from pip._internal.utils.models import KeyBasedCompareMixin from .base import Candidate, CandidateLookup, Requirement, format_name @@ -48,14 +47,11 @@ def is_satisfied_by(self, candidate: Candidate) -> bool: return candidate == self.candidate -class SpecifierRequirement(Requirement, KeyBasedCompareMixin): +class SpecifierRequirement(Requirement): def __init__(self, ireq: InstallRequirement) -> None: assert ireq.link is None, "This is a link, not a specifier" self._ireq = ireq self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras) - KeyBasedCompareMixin.__init__( - self, key=str(ireq), defining_class=SpecifierRequirement - ) def __str__(self) -> str: return str(self._ireq.req) @@ -63,6 +59,14 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"{self.__class__.__name__}({str(self._ireq.req)!r})" + def __eq__(self, other: object) -> bool: + if not isinstance(other, SpecifierRequirement): + return NotImplemented + return str(self._ireq) == str(other._ireq) + + def __hash__(self) -> int: + return hash(str(self._ireq)) + @property def project_name(self) -> NormalizedName: assert self._ireq.req, "Specifier-backed ireq is always PEP 508" @@ -111,9 +115,14 @@ def __init__(self, ireq: InstallRequirement) -> None: assert ireq.link is None, "This is a link, not a specifier" self._ireq = install_req_drop_extras(ireq) self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras) - KeyBasedCompareMixin.__init__( - self, key=str(ireq), defining_class=SpecifierRequirement - ) + + def __eq__(self, other: object) -> bool: + if not isinstance(other, SpecifierWithoutExtrasRequirement): + return NotImplemented + return str(self._ireq) == str(other._ireq) + + def __hash__(self) -> int: + return hash(str(self._ireq)) class RequiresPythonRequirement(Requirement): @@ -165,14 +174,11 @@ def is_satisfied_by(self, candidate: Candidate) -> bool: return self.specifier.contains(candidate.version, prereleases=True) -class UnsatisfiableRequirement(Requirement, KeyBasedCompareMixin): +class UnsatisfiableRequirement(Requirement): """A requirement that cannot be satisfied.""" def __init__(self, name: NormalizedName) -> None: self._name = name - KeyBasedCompareMixin.__init__( - self, key=str(name), defining_class=UnsatisfiableRequirement - ) def __str__(self) -> str: return f"{self._name} (unavailable)" @@ -180,6 +186,14 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"{self.__class__.__name__}({str(self._name)!r})" + def __eq__(self, other: object) -> bool: + if not isinstance(other, UnsatisfiableRequirement): + return NotImplemented + return self._name == other._name + + def __hash__(self) -> int: + return hash(self._name) + @property def project_name(self) -> NormalizedName: return self._name