Remove LRU cache from methods [ruff rule cached-instance-method] (#13306)

Enables ruff rule cached-instance-method (B019).

By caching instance methods, the instances are kept alive until they are
evicted out of the LRU cache. If the cached instance is itself is
referencing other memory-heavy objects, this can lead to hard-to-debug
high memory usage.

Most of the objects fixed here are fairly long lived, but we don't
necessarily want them to be kept alive until the very end of the process.
Hypothetically, if the build isolation became in-process there will be
multiple instances of these classes that should only live for the
lifetime of any particular build isolation.

Author: notatallshaw

pyproject.toml

@@ -166,7 +166,6 @@ extend-exclude = [
 
 [tool.ruff.lint]
 ignore = [
-    "B019",
     "B020",
     "B904", # Ruff enables opinionated warnings by default
     "B905", # Ruff enables opinionated warnings by default

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
@@ -795,7 +812,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
 
@@ -805,6 +821,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(
@@ -846,7 +865,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,
@@ -865,7 +886,6 @@ def make_candidate_evaluator(
             hashes=hashes,
         )
 
-    @functools.lru_cache(maxsize=None)
     def find_best_candidate(
         self,
         project_name: str,
@@ -880,13 +900,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
@@ -897,9 +924,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,
         )

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

src/pip/_internal/resolution/resolvelib/provider.py

@@ -270,8 +270,9 @@ def _eligible_for_upgrade(identifier: str) -> bool:
             is_satisfied_by=self.is_satisfied_by,
         )
 
+    @staticmethod
     @lru_cache(maxsize=None)
-    def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool:
+    def is_satisfied_by(requirement: Requirement, candidate: Candidate) -> bool:
         return requirement.is_satisfied_by(candidate)
 
     def get_dependencies(self, candidate: Candidate) -> Iterable[Requirement]:

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)