From 901db9cf8dc01477dda0601a3d0bc20eaa16073e Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Sun, 6 Aug 2023 09:08:16 -0700 Subject: [PATCH] Use a set for TargetPython.get_tags for performance (#12204) --- news/12204.feature.rst | 1 + src/pip/_internal/commands/debug.py | 2 +- src/pip/_internal/index/package_finder.py | 4 ++-- src/pip/_internal/models/target_python.py | 18 +++++++++++++++--- .../_internal/resolution/resolvelib/factory.py | 2 +- tests/unit/test_target_python.py | 18 +++++++++--------- 6 files changed, 29 insertions(+), 16 deletions(-) create mode 100644 news/12204.feature.rst diff --git a/news/12204.feature.rst b/news/12204.feature.rst new file mode 100644 index 00000000000..6ffdf5123b1 --- /dev/null +++ b/news/12204.feature.rst @@ -0,0 +1 @@ +Improve use of datastructures to make candidate selection 1.6x faster diff --git a/src/pip/_internal/commands/debug.py b/src/pip/_internal/commands/debug.py index 2a3e7d298f3..88a4f798d46 100644 --- a/src/pip/_internal/commands/debug.py +++ b/src/pip/_internal/commands/debug.py @@ -105,7 +105,7 @@ def show_tags(options: Values) -> None: tag_limit = 10 target_python = make_target_python(options) - tags = target_python.get_tags() + tags = target_python.get_sorted_tags() # Display the target options that were explicitly provided. formatted_target = target_python.format_given() diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index b6f8d57e854..2121ca327e6 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -198,7 +198,7 @@ def evaluate_link(self, link: Link) -> Tuple[LinkType, str]: reason = f"wrong project name (not {self.project_name})" return (LinkType.different_project, reason) - supported_tags = self._target_python.get_tags() + supported_tags = self._target_python.get_unsorted_tags() if not wheel.supported(supported_tags): # Include the wheel's tags in the reason string to # simplify troubleshooting compatibility issues. @@ -414,7 +414,7 @@ def create( if specifier is None: specifier = specifiers.SpecifierSet() - supported_tags = target_python.get_tags() + supported_tags = target_python.get_sorted_tags() return cls( project_name=project_name, diff --git a/src/pip/_internal/models/target_python.py b/src/pip/_internal/models/target_python.py index 744bd7ef58b..67ea5da73a5 100644 --- a/src/pip/_internal/models/target_python.py +++ b/src/pip/_internal/models/target_python.py @@ -1,5 +1,5 @@ import sys -from typing import List, Optional, Tuple +from typing import List, Optional, Set, Tuple from pip._vendor.packaging.tags import Tag @@ -22,6 +22,7 @@ class TargetPython: "py_version", "py_version_info", "_valid_tags", + "_valid_tags_set", ] def __init__( @@ -61,8 +62,9 @@ def __init__( self.py_version = py_version self.py_version_info = py_version_info - # This is used to cache the return value of get_tags(). + # This is used to cache the return value of get_(un)sorted_tags. self._valid_tags: Optional[List[Tag]] = None + self._valid_tags_set: Optional[Set[Tag]] = None def format_given(self) -> str: """ @@ -84,7 +86,7 @@ def format_given(self) -> str: f"{key}={value!r}" for key, value in key_values if value is not None ) - def get_tags(self) -> List[Tag]: + def get_sorted_tags(self) -> List[Tag]: """ Return the supported PEP 425 tags to check wheel candidates against. @@ -108,3 +110,13 @@ def get_tags(self) -> List[Tag]: self._valid_tags = tags return self._valid_tags + + def get_unsorted_tags(self) -> Set[Tag]: + """Exactly the same as get_sorted_tags, but returns a set. + + This is important for performance. + """ + if self._valid_tags_set is None: + self._valid_tags_set = set(self.get_sorted_tags()) + + return self._valid_tags_set diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 0331297b85b..ed78580ab97 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -132,7 +132,7 @@ def _fail_if_link_is_unsupported_wheel(self, link: Link) -> None: if not link.is_wheel: return wheel = Wheel(link.filename) - if wheel.supported(self._finder.target_python.get_tags()): + if wheel.supported(self._finder.target_python.get_unsorted_tags()): return msg = f"{link.filename} is not a supported wheel on this platform." raise UnsupportedWheel(msg) diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index d3e27e39ae8..b659c61fe08 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -88,12 +88,12 @@ def test_format_given(self, kwargs: Dict[str, Any], expected: str) -> None: ((3, 7, 3), "37"), # Check a minor version with two digits. ((3, 10, 1), "310"), - # Check that versions=None is passed to get_tags(). + # Check that versions=None is passed to get_sorted_tags(). (None, None), ], ) @mock.patch("pip._internal.models.target_python.get_supported") - def test_get_tags( + def test_get_sorted_tags( self, mock_get_supported: mock.Mock, py_version_info: Optional[Tuple[int, ...]], @@ -102,7 +102,7 @@ def test_get_tags( mock_get_supported.return_value = ["tag-1", "tag-2"] target_python = TargetPython(py_version_info=py_version_info) - actual = target_python.get_tags() + actual = target_python.get_sorted_tags() assert actual == ["tag-1", "tag-2"] actual = mock_get_supported.call_args[1]["version"] @@ -111,14 +111,14 @@ def test_get_tags( # Check that the value was cached. assert target_python._valid_tags == ["tag-1", "tag-2"] - def test_get_tags__uses_cached_value(self) -> None: + def test_get_unsorted_tags__uses_cached_value(self) -> None: """ - Test that get_tags() uses the cached value. + Test that get_unsorted_tags() uses the cached value. """ target_python = TargetPython(py_version_info=None) - target_python._valid_tags = [ + target_python._valid_tags_set = { Tag("py2", "none", "any"), Tag("py3", "none", "any"), - ] - actual = target_python.get_tags() - assert actual == [Tag("py2", "none", "any"), Tag("py3", "none", "any")] + } + actual = target_python.get_unsorted_tags() + assert actual == {Tag("py2", "none", "any"), Tag("py3", "none", "any")}