From 91c4fc45193fe3e8b83a6da88d069f15e78ac45f Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 22:33:36 -0700 Subject: [PATCH 01/10] Use a set for TargetPython.get_tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This speeds up a representative pip-compile by about 40% ``` λ hyperfine -w 1 -M 3 -p 'rm req.txt || true' 'python -m piptools compile req.in --output-file req.txt --strip-extras --resolver=backtracking' Benchmark 1: python -m piptools compile req.in --output-file req.txt --strip-extras --resolver=backtracking Time (mean ± σ): 28.552 s ± 2.552 s [User: 20.477 s, System: 1.547 s] Range (min … max): 26.446 s … 31.390 s 3 runs λ hyperfine -w 1 -M 3 -p 'rm req.txt || true' 'python -m piptools compile req.in --output-file req.txt --strip-extras --resolver=backtracking' Benchmark 1: python -m piptools compile req.in --output-file req.txt --strip-extras --resolver=backtracking Time (mean ± σ): 17.570 s ± 0.450 s [User: 11.537 s, System: 1.345 s] Range (min … max): 17.095 s … 17.989 s 3 runs ``` This makes the set.isdisjoint operation done over here much cheaper: https://github.com/pypa/pip/blob/593b85f4a/src/pip/_internal/models/wheel.py#L92 The reason for this is because a Python usually has a lot of supported tags. The Python I used above has 2000 supported tags! Whereas a wheel usually only has one or two file tags. The CPython code will unfortunately iterate over all 2000 tags to check if there's a match. Only if the other collection is a set will CPython think to swap the operands to iterate over the shorter collection: https://github.com/python/cpython/blob/35963da40f/Objects/setobject.c#L1352 --- src/pip/_internal/commands/debug.py | 7 ++++--- src/pip/_internal/index/package_finder.py | 2 +- src/pip/_internal/models/target_python.py | 8 ++++---- src/pip/_internal/models/wheel.py | 4 ++-- tests/unit/test_finder.py | 10 +++++----- tests/unit/test_index.py | 6 +++--- tests/unit/test_target_python.py | 4 ++-- 7 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/pip/_internal/commands/debug.py b/src/pip/_internal/commands/debug.py index 2a3e7d298f3..77713f67cde 100644 --- a/src/pip/_internal/commands/debug.py +++ b/src/pip/_internal/commands/debug.py @@ -5,10 +5,11 @@ import sys from optparse import Values from types import ModuleType -from typing import Any, Dict, List, Optional +from typing import Any, Collection, Dict, List, Optional import pip._vendor from pip._vendor.certifi import where +from pip._vendor.packaging.tags import Tag from pip._vendor.packaging.version import parse as parse_version from pip._internal.cli import cmdoptions @@ -105,7 +106,7 @@ def show_tags(options: Values) -> None: tag_limit = 10 target_python = make_target_python(options) - tags = target_python.get_tags() + tags: Collection[Tag] = target_python.get_tags() # Display the target options that were explicitly provided. formatted_target = target_python.format_given() @@ -118,7 +119,7 @@ def show_tags(options: Values) -> None: if options.verbose < 1 and len(tags) > tag_limit: tags_limited = True - tags = tags[:tag_limit] + tags = list(tags)[:tag_limit] else: tags_limited = False diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index b6f8d57e854..2b42168da2f 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -428,7 +428,7 @@ def create( def __init__( self, project_name: str, - supported_tags: List[Tag], + supported_tags: Set[Tag], specifier: specifiers.BaseSpecifier, prefer_binary: bool = False, allow_all_prereleases: bool = False, diff --git a/src/pip/_internal/models/target_python.py b/src/pip/_internal/models/target_python.py index 744bd7ef58b..b01fd6aded1 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 @@ -62,7 +62,7 @@ def __init__( self.py_version_info = py_version_info # This is used to cache the return value of get_tags(). - self._valid_tags: Optional[List[Tag]] = None + self._valid_tags: Optional[Set[Tag]] = None def format_given(self) -> str: """ @@ -84,7 +84,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_tags(self) -> Set[Tag]: """ Return the supported PEP 425 tags to check wheel candidates against. @@ -105,6 +105,6 @@ def get_tags(self) -> List[Tag]: abis=self.abis, impl=self.implementation, ) - self._valid_tags = tags + self._valid_tags = set(tags) return self._valid_tags diff --git a/src/pip/_internal/models/wheel.py b/src/pip/_internal/models/wheel.py index a5dc12bdd63..c05e95b1c56 100644 --- a/src/pip/_internal/models/wheel.py +++ b/src/pip/_internal/models/wheel.py @@ -2,7 +2,7 @@ name that have meaning. """ import re -from typing import Dict, Iterable, List +from typing import Dict, Iterable, List, Set from pip._vendor.packaging.tags import Tag @@ -64,7 +64,7 @@ def support_index_min(self, tags: List[Tag]) -> int: raise ValueError() def find_most_preferred_tag( - self, tags: List[Tag], tag_to_priority: Dict[Tag, int] + self, tags: Set[Tag], tag_to_priority: Dict[Tag, int] ) -> int: """Return the priority of the most preferred tag that one of the wheel's file tag combinations achieves in the given list of supported tags using the given diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 3404d1498e3..a8b608d7365 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -137,7 +137,7 @@ def test_not_find_wheel_not_supported(self, data: TestData) -> None: req = install_req_from_line("simple.dist") target_python = TargetPython() # Make sure no tags will match. - target_python._valid_tags = [] + target_python._valid_tags = set() finder = make_test_finder( find_links=[data.find_links], target_python=target_python, @@ -221,11 +221,11 @@ def test_link_sorting(self) -> None: Link("simple-1.0.tar.gz"), ), ] - valid_tags = [ + valid_tags = { Tag("pyT", "none", "TEST"), Tag("pyT", "TEST", "any"), Tag("pyT", "none", "any"), - ] + } specifier = SpecifierSet() evaluator = CandidateEvaluator( "my-project", @@ -289,11 +289,11 @@ def test_build_tag_is_less_important_than_other_tags(self) -> None: Link("simple-1.0.tar.gz"), ), ] - valid_tags = [ + valid_tags = { Tag("py3", "abi3", "linux_x86_64"), Tag("py3", "abi3", "linux_i386"), Tag("py3", "any", "none"), - ] + } evaluator = CandidateEvaluator( "my-project", supported_tags=valid_tags, diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index 78837b94e8b..c83a8d30bdd 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -220,7 +220,7 @@ def test_evaluate_link__incompatible_wheel(self) -> None: """ target_python = TargetPython(py_version_info=(3, 6, 4)) # Set the valid tags to an empty list to make sure nothing matches. - target_python._valid_tags = [] + target_python._valid_tags = set() evaluator = LinkEvaluator( project_name="sample", canonical_name="sample", @@ -373,7 +373,7 @@ class TestCandidateEvaluator: ) def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: target_python = TargetPython() - target_python._valid_tags = [Tag("py36", "none", "any")] + target_python._valid_tags = {Tag("py36", "none", "any")} specifier = SpecifierSet() evaluator = CandidateEvaluator.create( project_name="my-project", @@ -786,7 +786,7 @@ def test_make_candidate_evaluator( prefer_binary: bool, ) -> None: target_python = TargetPython() - target_python._valid_tags = [Tag("py36", "none", "any")] + target_python._valid_tags = {Tag("py36", "none", "any")} candidate_prefs = CandidatePreferences( prefer_binary=prefer_binary, allow_all_prereleases=allow_all_prereleases, diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index d3e27e39ae8..d1ed0b77cc8 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -116,9 +116,9 @@ def test_get_tags__uses_cached_value(self) -> None: Test that get_tags() uses the cached value. """ target_python = TargetPython(py_version_info=None) - target_python._valid_tags = [ + target_python._valid_tags = { Tag("py2", "none", "any"), Tag("py3", "none", "any"), - ] + } actual = target_python.get_tags() assert actual == [Tag("py2", "none", "any"), Tag("py3", "none", "any")] From bcde0322bdde460f6d38b6a1ec06e01008fb2010 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 22:47:16 -0700 Subject: [PATCH 02/10] news --- news/12204.feature.rst | 1 + 1 file changed, 1 insertion(+) 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..f905480d543 --- /dev/null +++ b/news/12204.feature.rst @@ -0,0 +1 @@ +Improve use of datastructures to speed up candidate selection by 40% From 47f21a701d324d4beea58fff25fa5dbab18c4215 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 22:52:04 -0700 Subject: [PATCH 03/10] fix test --- tests/unit/test_target_python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index d1ed0b77cc8..e99fc2c86c8 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -109,7 +109,7 @@ def test_get_tags( assert actual == expected_version # Check that the value was cached. - assert target_python._valid_tags == ["tag-1", "tag-2"] + assert target_python._valid_tags == {"tag-1", "tag-2"} def test_get_tags__uses_cached_value(self) -> None: """ From c17ff03147e8524fb14c78f0f640bf04c77aef7e Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 22:58:02 -0700 Subject: [PATCH 04/10] get_sorted_tags --- src/pip/_internal/commands/debug.py | 7 +++---- src/pip/_internal/index/package_finder.py | 4 ++-- src/pip/_internal/models/target_python.py | 21 +++++++++++++++---- .../resolution/resolvelib/factory.py | 2 +- tests/unit/test_finder.py | 2 +- tests/unit/test_index.py | 6 +++--- tests/unit/test_target_python.py | 10 ++++----- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/pip/_internal/commands/debug.py b/src/pip/_internal/commands/debug.py index 77713f67cde..88a4f798d46 100644 --- a/src/pip/_internal/commands/debug.py +++ b/src/pip/_internal/commands/debug.py @@ -5,11 +5,10 @@ import sys from optparse import Values from types import ModuleType -from typing import Any, Collection, Dict, List, Optional +from typing import Any, Dict, List, Optional import pip._vendor from pip._vendor.certifi import where -from pip._vendor.packaging.tags import Tag from pip._vendor.packaging.version import parse as parse_version from pip._internal.cli import cmdoptions @@ -106,7 +105,7 @@ def show_tags(options: Values) -> None: tag_limit = 10 target_python = make_target_python(options) - tags: Collection[Tag] = target_python.get_tags() + tags = target_python.get_sorted_tags() # Display the target options that were explicitly provided. formatted_target = target_python.format_given() @@ -119,7 +118,7 @@ def show_tags(options: Values) -> None: if options.verbose < 1 and len(tags) > tag_limit: tags_limited = True - tags = list(tags)[:tag_limit] + tags = tags[:tag_limit] else: tags_limited = False diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index 2b42168da2f..5dd79682e8d 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_unsorted_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 b01fd6aded1..16c6e5b8907 100644 --- a/src/pip/_internal/models/target_python.py +++ b/src/pip/_internal/models/target_python.py @@ -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(). - self._valid_tags: Optional[Set[Tag]] = None + # 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) -> Set[Tag]: + def get_sorted_tags(self) -> List[Tag]: """ Return the supported PEP 425 tags to check wheel candidates against. @@ -105,6 +107,17 @@ def get_tags(self) -> Set[Tag]: abis=self.abis, impl=self.implementation, ) - self._valid_tags = set(tags) + 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 can be important for performance. + """ + if self._valid_tags_set is None: + self._valid_tags_set = set(self.get_sorted_tags()) + + assert self._valid_tags_set is not None + 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_finder.py b/tests/unit/test_finder.py index a8b608d7365..6891bc97384 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -137,7 +137,7 @@ def test_not_find_wheel_not_supported(self, data: TestData) -> None: req = install_req_from_line("simple.dist") target_python = TargetPython() # Make sure no tags will match. - target_python._valid_tags = set() + target_python._valid_tags = [] finder = make_test_finder( find_links=[data.find_links], target_python=target_python, diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index c83a8d30bdd..78837b94e8b 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -220,7 +220,7 @@ def test_evaluate_link__incompatible_wheel(self) -> None: """ target_python = TargetPython(py_version_info=(3, 6, 4)) # Set the valid tags to an empty list to make sure nothing matches. - target_python._valid_tags = set() + target_python._valid_tags = [] evaluator = LinkEvaluator( project_name="sample", canonical_name="sample", @@ -373,7 +373,7 @@ class TestCandidateEvaluator: ) def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: target_python = TargetPython() - target_python._valid_tags = {Tag("py36", "none", "any")} + target_python._valid_tags = [Tag("py36", "none", "any")] specifier = SpecifierSet() evaluator = CandidateEvaluator.create( project_name="my-project", @@ -786,7 +786,7 @@ def test_make_candidate_evaluator( prefer_binary: bool, ) -> None: target_python = TargetPython() - target_python._valid_tags = {Tag("py36", "none", "any")} + target_python._valid_tags = [Tag("py36", "none", "any")] candidate_prefs = CandidatePreferences( prefer_binary=prefer_binary, allow_all_prereleases=allow_all_prereleases, diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index e99fc2c86c8..04ce0dc2ffd 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -102,23 +102,23 @@ 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"] assert actual == expected_version # Check that the value was cached. - assert target_python._valid_tags == {"tag-1", "tag-2"} + assert target_python._valid_tags == ["tag-1", "tag-2"] def test_get_tags__uses_cached_value(self) -> None: """ Test that get_tags() uses the cached value. """ target_python = TargetPython(py_version_info=None) - target_python._valid_tags = { + target_python._valid_tags = [ Tag("py2", "none", "any"), Tag("py3", "none", "any"), - } - actual = target_python.get_tags() + ] + actual = target_python.get_sorted_tags() assert actual == [Tag("py2", "none", "any"), Tag("py3", "none", "any")] From 53f9ce7a163b5741c6669ad6a3b5c2f2d969ff49 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 23:11:04 -0700 Subject: [PATCH 05/10] get_unsorted_tags --- src/pip/_internal/models/target_python.py | 46 ++++++++++------------- tests/unit/test_finder.py | 2 +- tests/unit/test_index.py | 6 +-- tests/unit/test_target_python.py | 12 +++--- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/pip/_internal/models/target_python.py b/src/pip/_internal/models/target_python.py index 16c6e5b8907..1337f758f03 100644 --- a/src/pip/_internal/models/target_python.py +++ b/src/pip/_internal/models/target_python.py @@ -22,7 +22,6 @@ class TargetPython: "py_version", "py_version_info", "_valid_tags", - "_valid_tags_set", ] def __init__( @@ -62,9 +61,8 @@ def __init__( self.py_version = py_version self.py_version_info = py_version_info - # 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 + # This is used to cache the return value of get_unsorted_tags. + self._valid_tags: Optional[Set[Tag]] = None def format_given(self) -> str: """ @@ -92,32 +90,28 @@ def get_sorted_tags(self) -> List[Tag]: The tags are returned in order of preference (most preferred first). """ - if self._valid_tags is None: - # Pass versions=None if no py_version_info was given since - # versions=None uses special default logic. - py_version_info = self._given_py_version_info - if py_version_info is None: - version = None - else: - version = version_info_to_nodot(py_version_info) - - tags = get_supported( - version=version, - platforms=self.platforms, - abis=self.abis, - impl=self.implementation, - ) - self._valid_tags = tags + # Pass versions=None if no py_version_info was given since + # versions=None uses special default logic. + py_version_info = self._given_py_version_info + if py_version_info is None: + version = None + else: + version = version_info_to_nodot(py_version_info) - return self._valid_tags + tags = get_supported( + version=version, + platforms=self.platforms, + abis=self.abis, + impl=self.implementation, + ) + return tags def get_unsorted_tags(self) -> Set[Tag]: """Exactly the same as get_sorted_tags, but returns a set. - This can be important for performance. + This is important for performance. """ - if self._valid_tags_set is None: - self._valid_tags_set = set(self.get_sorted_tags()) + if self._valid_tags is None: + self._valid_tags = set(self.get_sorted_tags()) - assert self._valid_tags_set is not None - return self._valid_tags_set + return self._valid_tags diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 6891bc97384..a8b608d7365 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -137,7 +137,7 @@ def test_not_find_wheel_not_supported(self, data: TestData) -> None: req = install_req_from_line("simple.dist") target_python = TargetPython() # Make sure no tags will match. - target_python._valid_tags = [] + target_python._valid_tags = set() finder = make_test_finder( find_links=[data.find_links], target_python=target_python, diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index 78837b94e8b..c83a8d30bdd 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -220,7 +220,7 @@ def test_evaluate_link__incompatible_wheel(self) -> None: """ target_python = TargetPython(py_version_info=(3, 6, 4)) # Set the valid tags to an empty list to make sure nothing matches. - target_python._valid_tags = [] + target_python._valid_tags = set() evaluator = LinkEvaluator( project_name="sample", canonical_name="sample", @@ -373,7 +373,7 @@ class TestCandidateEvaluator: ) def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: target_python = TargetPython() - target_python._valid_tags = [Tag("py36", "none", "any")] + target_python._valid_tags = {Tag("py36", "none", "any")} specifier = SpecifierSet() evaluator = CandidateEvaluator.create( project_name="my-project", @@ -786,7 +786,7 @@ def test_make_candidate_evaluator( prefer_binary: bool, ) -> None: target_python = TargetPython() - target_python._valid_tags = [Tag("py36", "none", "any")] + target_python._valid_tags = {Tag("py36", "none", "any")} candidate_prefs = CandidatePreferences( prefer_binary=prefer_binary, allow_all_prereleases=allow_all_prereleases, diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index 04ce0dc2ffd..cf28ba4da99 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -93,7 +93,7 @@ def test_format_given(self, kwargs: Dict[str, Any], expected: str) -> 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, ...]], @@ -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. """ target_python = TargetPython(py_version_info=None) - target_python._valid_tags = [ + target_python._valid_tags = { Tag("py2", "none", "any"), Tag("py3", "none", "any"), - ] - actual = target_python.get_sorted_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")} From a0c804f1581416c777cfe957c9caea11e80c52b1 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 23:12:33 -0700 Subject: [PATCH 06/10] test failure --- tests/unit/test_index.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index c83a8d30bdd..04e7cbd4b4d 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -385,14 +385,14 @@ def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: assert evaluator._allow_all_prereleases == allow_all_prereleases assert evaluator._prefer_binary == prefer_binary assert evaluator._specifier is specifier - assert evaluator._supported_tags == [Tag("py36", "none", "any")] + assert evaluator._supported_tags == {Tag("py36", "none", "any")} def test_create__target_python_none(self) -> None: """ Test passing target_python=None. """ evaluator = CandidateEvaluator.create("my-project") - expected_tags = get_supported() + expected_tags = set(get_supported()) assert evaluator._supported_tags == expected_tags def test_create__specifier_none(self) -> None: @@ -815,7 +815,7 @@ def test_make_candidate_evaluator( assert evaluator._prefer_binary == prefer_binary assert evaluator._project_name == "my-project" assert evaluator._specifier is specifier - assert evaluator._supported_tags == [Tag("py36", "none", "any")] + assert evaluator._supported_tags == {Tag("py36", "none", "any")} @pytest.mark.parametrize( From ba7863d2e8e0d4efb029ff56fbf031729e9a438d Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 23:20:08 -0700 Subject: [PATCH 07/10] remove caching test --- tests/unit/test_target_python.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index cf28ba4da99..64a3b8043e3 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -108,9 +108,6 @@ def test_get_sorted_tags( actual = mock_get_supported.call_args[1]["version"] assert actual == expected_version - # Check that the value was cached. - assert target_python._valid_tags == ["tag-1", "tag-2"] - def test_get_unsorted_tags__uses_cached_value(self) -> None: """ Test that get_tags() uses the cached value. From f8fbbb1221797fa32bc8e819232657b0957de765 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 23:25:44 -0700 Subject: [PATCH 08/10] fix ordering --- src/pip/_internal/index/package_finder.py | 4 +-- src/pip/_internal/models/target_python.py | 43 +++++++++++++---------- src/pip/_internal/models/wheel.py | 2 +- tests/unit/test_finder.py | 10 +++--- tests/unit/test_index.py | 12 +++---- tests/unit/test_target_python.py | 5 ++- 6 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index 5dd79682e8d..2121ca327e6 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -414,7 +414,7 @@ def create( if specifier is None: specifier = specifiers.SpecifierSet() - supported_tags = target_python.get_unsorted_tags() + supported_tags = target_python.get_sorted_tags() return cls( project_name=project_name, @@ -428,7 +428,7 @@ def create( def __init__( self, project_name: str, - supported_tags: Set[Tag], + supported_tags: List[Tag], specifier: specifiers.BaseSpecifier, prefer_binary: bool = False, allow_all_prereleases: bool = False, diff --git a/src/pip/_internal/models/target_python.py b/src/pip/_internal/models/target_python.py index 1337f758f03..67ea5da73a5 100644 --- a/src/pip/_internal/models/target_python.py +++ b/src/pip/_internal/models/target_python.py @@ -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_unsorted_tags. - self._valid_tags: Optional[Set[Tag]] = None + # 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: """ @@ -90,28 +92,31 @@ def get_sorted_tags(self) -> List[Tag]: The tags are returned in order of preference (most preferred first). """ - # Pass versions=None if no py_version_info was given since - # versions=None uses special default logic. - py_version_info = self._given_py_version_info - if py_version_info is None: - version = None - else: - version = version_info_to_nodot(py_version_info) + if self._valid_tags is None: + # Pass versions=None if no py_version_info was given since + # versions=None uses special default logic. + py_version_info = self._given_py_version_info + if py_version_info is None: + version = None + else: + version = version_info_to_nodot(py_version_info) + + tags = get_supported( + version=version, + platforms=self.platforms, + abis=self.abis, + impl=self.implementation, + ) + self._valid_tags = tags - tags = get_supported( - version=version, - platforms=self.platforms, - abis=self.abis, - impl=self.implementation, - ) - return 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 is None: - self._valid_tags = set(self.get_sorted_tags()) + if self._valid_tags_set is None: + self._valid_tags_set = set(self.get_sorted_tags()) - return self._valid_tags + return self._valid_tags_set diff --git a/src/pip/_internal/models/wheel.py b/src/pip/_internal/models/wheel.py index c05e95b1c56..74beb517093 100644 --- a/src/pip/_internal/models/wheel.py +++ b/src/pip/_internal/models/wheel.py @@ -64,7 +64,7 @@ def support_index_min(self, tags: List[Tag]) -> int: raise ValueError() def find_most_preferred_tag( - self, tags: Set[Tag], tag_to_priority: Dict[Tag, int] + self, tags: List[Tag], tag_to_priority: Dict[Tag, int] ) -> int: """Return the priority of the most preferred tag that one of the wheel's file tag combinations achieves in the given list of supported tags using the given diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index a8b608d7365..3404d1498e3 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -137,7 +137,7 @@ def test_not_find_wheel_not_supported(self, data: TestData) -> None: req = install_req_from_line("simple.dist") target_python = TargetPython() # Make sure no tags will match. - target_python._valid_tags = set() + target_python._valid_tags = [] finder = make_test_finder( find_links=[data.find_links], target_python=target_python, @@ -221,11 +221,11 @@ def test_link_sorting(self) -> None: Link("simple-1.0.tar.gz"), ), ] - valid_tags = { + valid_tags = [ Tag("pyT", "none", "TEST"), Tag("pyT", "TEST", "any"), Tag("pyT", "none", "any"), - } + ] specifier = SpecifierSet() evaluator = CandidateEvaluator( "my-project", @@ -289,11 +289,11 @@ def test_build_tag_is_less_important_than_other_tags(self) -> None: Link("simple-1.0.tar.gz"), ), ] - valid_tags = { + valid_tags = [ Tag("py3", "abi3", "linux_x86_64"), Tag("py3", "abi3", "linux_i386"), Tag("py3", "any", "none"), - } + ] evaluator = CandidateEvaluator( "my-project", supported_tags=valid_tags, diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py index 04e7cbd4b4d..78837b94e8b 100644 --- a/tests/unit/test_index.py +++ b/tests/unit/test_index.py @@ -220,7 +220,7 @@ def test_evaluate_link__incompatible_wheel(self) -> None: """ target_python = TargetPython(py_version_info=(3, 6, 4)) # Set the valid tags to an empty list to make sure nothing matches. - target_python._valid_tags = set() + target_python._valid_tags = [] evaluator = LinkEvaluator( project_name="sample", canonical_name="sample", @@ -373,7 +373,7 @@ class TestCandidateEvaluator: ) def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: target_python = TargetPython() - target_python._valid_tags = {Tag("py36", "none", "any")} + target_python._valid_tags = [Tag("py36", "none", "any")] specifier = SpecifierSet() evaluator = CandidateEvaluator.create( project_name="my-project", @@ -385,14 +385,14 @@ def test_create(self, allow_all_prereleases: bool, prefer_binary: bool) -> None: assert evaluator._allow_all_prereleases == allow_all_prereleases assert evaluator._prefer_binary == prefer_binary assert evaluator._specifier is specifier - assert evaluator._supported_tags == {Tag("py36", "none", "any")} + assert evaluator._supported_tags == [Tag("py36", "none", "any")] def test_create__target_python_none(self) -> None: """ Test passing target_python=None. """ evaluator = CandidateEvaluator.create("my-project") - expected_tags = set(get_supported()) + expected_tags = get_supported() assert evaluator._supported_tags == expected_tags def test_create__specifier_none(self) -> None: @@ -786,7 +786,7 @@ def test_make_candidate_evaluator( prefer_binary: bool, ) -> None: target_python = TargetPython() - target_python._valid_tags = {Tag("py36", "none", "any")} + target_python._valid_tags = [Tag("py36", "none", "any")] candidate_prefs = CandidatePreferences( prefer_binary=prefer_binary, allow_all_prereleases=allow_all_prereleases, @@ -815,7 +815,7 @@ def test_make_candidate_evaluator( assert evaluator._prefer_binary == prefer_binary assert evaluator._project_name == "my-project" assert evaluator._specifier is specifier - assert evaluator._supported_tags == {Tag("py36", "none", "any")} + assert evaluator._supported_tags == [Tag("py36", "none", "any")] @pytest.mark.parametrize( diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index 64a3b8043e3..f14d599f1f1 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -108,12 +108,15 @@ def test_get_sorted_tags( actual = mock_get_supported.call_args[1]["version"] assert actual == expected_version + # Check that the value was cached. + assert target_python._valid_tags == ["tag-1", "tag-2"] + def test_get_unsorted_tags__uses_cached_value(self) -> None: """ Test that get_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"), } From 18926cb32c23d017fd8793d6ea47b9b1b66f5682 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 3 Aug 2023 23:35:03 -0700 Subject: [PATCH 09/10] delint --- src/pip/_internal/models/wheel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/models/wheel.py b/src/pip/_internal/models/wheel.py index 74beb517093..a5dc12bdd63 100644 --- a/src/pip/_internal/models/wheel.py +++ b/src/pip/_internal/models/wheel.py @@ -2,7 +2,7 @@ name that have meaning. """ import re -from typing import Dict, Iterable, List, Set +from typing import Dict, Iterable, List from pip._vendor.packaging.tags import Tag From dfdf4ec6a401310c6f42c00bef81fdad0196da16 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Fri, 4 Aug 2023 01:35:11 -0700 Subject: [PATCH 10/10] fix nits --- news/12204.feature.rst | 2 +- tests/unit/test_target_python.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/news/12204.feature.rst b/news/12204.feature.rst index f905480d543..6ffdf5123b1 100644 --- a/news/12204.feature.rst +++ b/news/12204.feature.rst @@ -1 +1 @@ -Improve use of datastructures to speed up candidate selection by 40% +Improve use of datastructures to make candidate selection 1.6x faster diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index f14d599f1f1..b659c61fe08 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -88,7 +88,7 @@ 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), ], ) @@ -113,7 +113,7 @@ def test_get_sorted_tags( 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_set = {