From 65eb4f0bb6cf91a7d010ee888e690f228bf2c5eb Mon Sep 17 00:00:00 2001 From: Damian Shaw Date: Thu, 27 Mar 2025 19:34:00 -0400 Subject: [PATCH 1/4] Remove lru_cache from methods --- src/pip/_internal/index/package_finder.py | 37 ++++++++++++++++--- .../resolution/resolvelib/found_candidates.py | 11 ++++-- .../resolution/resolvelib/provider.py | 10 ++++- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index ce0842b07e6..f9393d668e3 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -6,7 +6,17 @@ import logging import re from dataclasses import dataclass -from typing import TYPE_CHECKING, FrozenSet, Iterable, List, Optional, Set, Tuple, Union +from typing import ( + TYPE_CHECKING, + Dict, + FrozenSet, + Iterable, + List, + Optional, + Set, + Tuple, + Union, +) from pip._vendor.packaging import specifiers from pip._vendor.packaging.tags import Tag @@ -601,6 +611,13 @@ def __init__( # These are boring links that have already been logged somehow. self._logged_links: Set[Tuple[Link, LinkType, str]] = set() + # Cache of the result of finding candidates + self._all_candidates: Dict[str, List[InstallationCandidate]] = {} + self._best_candidates: Dict[ + Tuple[str, Optional[specifiers.BaseSpecifier], Optional[Hashes]], + BestCandidateResult, + ] = {} + # Don't include an allow_yanked default value to make sure each call # site considers whether yanked releases are allowed. This also causes # that decision to be made explicit in the calling code, which helps @@ -800,7 +817,6 @@ def process_project_url( return package_links - @functools.lru_cache(maxsize=None) def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]: """Find all available InstallationCandidate for project_name @@ -810,6 +826,9 @@ def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]: See LinkEvaluator.evaluate_link() for details on which files are accepted. """ + if project_name in self._all_candidates: + return self._all_candidates[project_name] + link_evaluator = self.make_link_evaluator(project_name) collected_sources = self._link_collector.collect_sources( @@ -851,7 +870,9 @@ def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]: logger.debug("Local files found: %s", ", ".join(paths)) # This is an intentional priority ordering - return file_candidates + page_candidates + self._all_candidates[project_name] = file_candidates + page_candidates + + return self._all_candidates[project_name] def make_candidate_evaluator( self, @@ -870,7 +891,6 @@ def make_candidate_evaluator( hashes=hashes, ) - @functools.lru_cache(maxsize=None) def find_best_candidate( self, project_name: str, @@ -885,13 +905,20 @@ def find_best_candidate( :return: A `BestCandidateResult` instance. """ + if (project_name, specifier, hashes) in self._best_candidates: + return self._best_candidates[project_name, specifier, hashes] + candidates = self.find_all_candidates(project_name) candidate_evaluator = self.make_candidate_evaluator( project_name=project_name, specifier=specifier, hashes=hashes, ) - return candidate_evaluator.compute_best_candidate(candidates) + self._best_candidates[project_name, specifier, hashes] = ( + candidate_evaluator.compute_best_candidate(candidates) + ) + + return self._best_candidates[project_name, specifier, hashes] def find_requirement( self, req: InstallRequirement, upgrade: bool diff --git a/src/pip/_internal/resolution/resolvelib/found_candidates.py b/src/pip/_internal/resolution/resolvelib/found_candidates.py index dd07a4072f2..3a9c2ed723d 100644 --- a/src/pip/_internal/resolution/resolvelib/found_candidates.py +++ b/src/pip/_internal/resolution/resolvelib/found_candidates.py @@ -8,7 +8,6 @@ something. """ -import functools import logging from collections.abc import Sequence from typing import Any, Callable, Iterator, Optional, Set, Tuple @@ -129,6 +128,7 @@ def __init__( self._installed = installed self._prefers_installed = prefers_installed self._incompatible_ids = incompatible_ids + self._bool: Optional[bool] = None def __getitem__(self, index: Any) -> Any: # Implemented to satisfy the ABC check. This is not needed by the @@ -152,8 +152,13 @@ def __len__(self) -> int: # performance reasons). raise NotImplementedError("don't do this") - @functools.lru_cache(maxsize=1) def __bool__(self) -> bool: + if self._bool is not None: + return self._bool + if self._prefers_installed and self._installed: + self._bool = True return True - return any(self) + + self._bool = any(self) + return self._bool diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index 617a993cb03..157870cb8de 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -257,11 +257,17 @@ def _eligible_for_upgrade(identifier: str) -> bool: is_satisfied_by=self.is_satisfied_by, ) - @lru_cache(maxsize=None) def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool: - return requirement.is_satisfied_by(candidate) + return _pip_provider_is_satisfied_by(requirement, candidate) def get_dependencies(self, candidate: Candidate) -> Iterable[Requirement]: with_requires = not self._ignore_dependencies # iter_dependencies() can perform nontrivial work so delay until needed. return (r for r in candidate.iter_dependencies(with_requires) if r is not None) + + +@lru_cache(maxsize=None) +def _pip_provider_is_satisfied_by( + requirement: Requirement, candidate: Candidate +) -> bool: + return requirement.is_satisfied_by(candidate) From 07d89e479786c264cf404dd0500bc940d8fe50ab Mon Sep 17 00:00:00 2001 From: Damian Shaw Date: Thu, 27 Mar 2025 19:39:55 -0400 Subject: [PATCH 2/4] Fix type hints --- src/pip/_internal/index/package_finder.py | 5 ++++- tests/unit/test_finder.py | 11 ++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index f9393d668e3..8ecce33ee02 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -929,9 +929,12 @@ def find_requirement( Returns a InstallationCandidate if found, Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise """ + name = req.name + assert name is not None, "find_requirement() called with no name" + hashes = req.hashes(trust_internet=False) best_candidate_result = self.find_best_candidate( - req.name, + name, specifier=req.specifier, hashes=hashes, ) diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 8c923dcd36f..74c3e49d62a 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -319,7 +319,10 @@ def test_finder_priority_file_over_page(data: TestData) -> None: find_links=[data.find_links], index_urls=["http://pypi.org/simple/"], ) - all_versions = finder.find_all_candidates(req.name) + name = req.name + assert name == "gmpy" + + all_versions = finder.find_all_candidates(name) # 1 file InstallationCandidate followed by all https ones assert all_versions[0].link.scheme == "file" assert all( @@ -335,9 +338,11 @@ def test_finder_priority_nonegg_over_eggfragments() -> None: """Test PackageFinder prefers non-egg links over "#egg=" links""" req = install_req_from_line("bar==1.0") links = ["http://foo/bar.py#egg=bar-1.0", "http://foo/bar-1.0.tar.gz"] + name = req.name + assert name == "bar" finder = make_test_finder(links) - all_versions = finder.find_all_candidates(req.name) + all_versions = finder.find_all_candidates(name) assert all_versions[0].link.url.endswith("tar.gz") assert all_versions[1].link.url.endswith("#egg=bar-1.0") @@ -349,7 +354,7 @@ def test_finder_priority_nonegg_over_eggfragments() -> None: links.reverse() finder = make_test_finder(links) - all_versions = finder.find_all_candidates(req.name) + all_versions = finder.find_all_candidates(name) assert all_versions[0].link.url.endswith("tar.gz") assert all_versions[1].link.url.endswith("#egg=bar-1.0") found = finder.find_requirement(req, False) From 2e1e7c96e167db0c354da3c98fd6fbc07377c738 Mon Sep 17 00:00:00 2001 From: Damian Shaw Date: Thu, 27 Mar 2025 19:41:40 -0400 Subject: [PATCH 3/4] Enable ruff rule cached-instance-method (B019) --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 23830a24881..d24aa36f6bd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -164,7 +164,6 @@ extend-exclude = [ [tool.ruff.lint] ignore = [ - "B019", "B020", "B904", # Ruff enables opinionated warnings by default "B905", # Ruff enables opinionated warnings by default From 82684ea0f784fec004a648d9552acda8eed9bf5c Mon Sep 17 00:00:00 2001 From: Damian Shaw Date: Fri, 28 Mar 2025 10:35:01 -0400 Subject: [PATCH 4/4] For PipProvider.is_satisfied_by switch to static method --- src/pip/_internal/resolution/resolvelib/provider.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/provider.py b/src/pip/_internal/resolution/resolvelib/provider.py index 157870cb8de..201545db0ce 100644 --- a/src/pip/_internal/resolution/resolvelib/provider.py +++ b/src/pip/_internal/resolution/resolvelib/provider.py @@ -257,17 +257,12 @@ def _eligible_for_upgrade(identifier: str) -> bool: is_satisfied_by=self.is_satisfied_by, ) - def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool: - return _pip_provider_is_satisfied_by(requirement, candidate) + @staticmethod + @lru_cache(maxsize=None) + def is_satisfied_by(requirement: Requirement, candidate: Candidate) -> bool: + return requirement.is_satisfied_by(candidate) def get_dependencies(self, candidate: Candidate) -> Iterable[Requirement]: with_requires = not self._ignore_dependencies # iter_dependencies() can perform nontrivial work so delay until needed. return (r for r in candidate.iter_dependencies(with_requires) if r is not None) - - -@lru_cache(maxsize=None) -def _pip_provider_is_satisfied_by( - requirement: Requirement, candidate: Candidate -) -> bool: - return requirement.is_satisfied_by(candidate)