From 8872e6860d517969ba3403c61f4ee5e8db45e80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Tue, 27 Dec 2022 16:31:28 +0100 Subject: [PATCH] performance(markers): performance fix for marker simplifications * intersect_simplify to reduce the number of items of itertools.product in cnf (analoguous to union_simplify which primarily affects dnf) * revival of common_markers/unique_markers simplification, which has been removed in poetry-core#530 but helps massively in reducing the cost of cnf/dnf Co-authored-by: David Hotham --- src/poetry/core/version/markers.py | 74 ++++++++++++++++++++++++++++++ tests/packages/utils/test_utils.py | 2 +- tests/version/test_markers.py | 69 ++++++++++++++++++++++++++-- 3 files changed, 139 insertions(+), 6 deletions(-) diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py index 047d8fa71..7c12a34db 100644 --- a/src/poetry/core/version/markers.py +++ b/src/poetry/core/version/markers.py @@ -420,7 +420,18 @@ def of(cls, *markers: BaseMarker) -> BaseMarker: intersected = True break + # If we have a MarkerUnion then we can look for the simplifications + # implemented in intersect_simplify(). + elif isinstance(mark, MarkerUnion): + intersection = mark.intersect_simplify(marker) + if intersection is not None: + new_markers[i] = intersection + intersected = True + break + if intersected: + # flatten again because intersect_simplify may return a multi + new_markers = _flatten_markers(new_markers, MultiMarker) continue new_markers.append(marker) @@ -451,6 +462,9 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None: - union between two multimarkers where one is contained by the other is just the larger of the two + + - union between two multimarkers where there are some common markers + and the union of unique markers is a single marker """ if other in self._markers: return other @@ -465,6 +479,22 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None: if their_markers.issubset(our_markers): return other + shared_markers = our_markers.intersection(their_markers) + if not shared_markers: + return None + + unique_markers = our_markers - their_markers + other_unique_markers = their_markers - our_markers + unique_union = MultiMarker(*unique_markers).union( + MultiMarker(*other_unique_markers) + ) + if isinstance(unique_union, (SingleMarker, AnyMarker)): + # Use list instead of set for deterministic order. + common_markers = [ + marker for marker in self.markers if marker in shared_markers + ] + return unique_union.intersect(MultiMarker(*common_markers)) + return None def validate(self, environment: dict[str, Any] | None) -> bool: @@ -596,6 +626,50 @@ def intersect(self, other: BaseMarker) -> BaseMarker: def union(self, other: BaseMarker) -> BaseMarker: return union(self, other) + def intersect_simplify(self, other: BaseMarker) -> BaseMarker | None: + """ + Finds a couple of easy simplifications for intersection on MarkerUnions: + + - intersection with any marker that appears as part of the union is just + that marker + + - intersection between two markerunions where one is contained by the other + is just the smaller of the two + + - intersection between two markerunions where there are some common markers + and the intersection of unique markers is not a single marker + """ + if other in self._markers: + return other + + if isinstance(other, MarkerUnion): + our_markers = set(self.markers) + their_markers = set(other.markers) + + if our_markers.issubset(their_markers): + return self + + if their_markers.issubset(our_markers): + return other + + shared_markers = our_markers.intersection(their_markers) + if not shared_markers: + return None + + unique_markers = our_markers - their_markers + other_unique_markers = their_markers - our_markers + unique_intersection = MarkerUnion(*unique_markers).intersect( + MarkerUnion(*other_unique_markers) + ) + if isinstance(unique_intersection, (SingleMarker, EmptyMarker)): + # Use list instead of set for deterministic order. + common_markers = [ + marker for marker in self.markers if marker in shared_markers + ] + return unique_intersection.union(MarkerUnion(*common_markers)) + + return None + def validate(self, environment: dict[str, Any] | None) -> bool: return any(m.validate(environment) for m in self._markers) diff --git a/tests/packages/utils/test_utils.py b/tests/packages/utils/test_utils.py index 96db7e07e..757e98a4a 100644 --- a/tests/packages/utils/test_utils.py +++ b/tests/packages/utils/test_utils.py @@ -25,8 +25,8 @@ { "python_version": [ [("<", "3.6")], - [("<", "3.3")], [("<", "3.6"), (">=", "3.3")], + [("<", "3.3")], ], "sys_platform": [ [("==", "win32")], diff --git a/tests/version/test_markers.py b/tests/version/test_markers.py index 2e7de7a24..dd45c2f7a 100644 --- a/tests/version/test_markers.py +++ b/tests/version/test_markers.py @@ -13,7 +13,9 @@ from poetry.core.version.markers import SingleMarker from poetry.core.version.markers import cnf from poetry.core.version.markers import dnf +from poetry.core.version.markers import intersection from poetry.core.version.markers import parse_marker +from poetry.core.version.markers import union if TYPE_CHECKING: @@ -1285,11 +1287,6 @@ def test_union_should_drop_markers_if_their_complement_is_present( ), ), MultiMarker( - MarkerUnion( - SingleMarker("sys_platform", "!=win32"), - SingleMarker("python_version", "<3.8"), - SingleMarker("python_version", ">=3.9"), - ), MarkerUnion( SingleMarker("python_version", "<3.8"), SingleMarker("python_version", ">=3.9"), @@ -1556,6 +1553,68 @@ def test_empty_marker_is_found_in_complex_parse() -> None: assert marker.is_empty() +def test_complex_union() -> None: + # real world example on the way to get mutually exclusive markers + # for numpy(>=1.21.2) of https://pypi.org/project/opencv-python/4.6.0.66/ + markers = [ + parse_marker(m) + for m in [ + ( + 'python_version < "3.7" and python_version >= "3.6"' + ' and platform_system == "Darwin" and platform_machine == "arm64"' + ), + ( + 'python_version >= "3.10" or python_version >= "3.9"' + ' and platform_system == "Darwin" and platform_machine == "arm64"' + ), + ( + 'python_version >= "3.8" and platform_system == "Darwin"' + ' and platform_machine == "arm64" and python_version < "3.9"' + ), + ( + 'python_version >= "3.7" and platform_system == "Darwin"' + ' and platform_machine == "arm64" and python_version < "3.8"' + ), + ] + ] + assert ( + str(union(*markers)) + == 'platform_system == "Darwin" and platform_machine == "arm64"' + ' and python_version >= "3.6" or python_version >= "3.10"' + ) + + +def test_complex_intersection() -> None: + # inverse of real world example on the way to get mutually exclusive markers + # for numpy(>=1.21.2) of https://pypi.org/project/opencv-python/4.6.0.66/ + markers = [ + parse_marker(m).invert() + for m in [ + ( + 'python_version < "3.7" and python_version >= "3.6"' + ' and platform_system == "Darwin" and platform_machine == "arm64"' + ), + ( + 'python_version >= "3.10" or python_version >= "3.9"' + ' and platform_system == "Darwin" and platform_machine == "arm64"' + ), + ( + 'python_version >= "3.8" and platform_system == "Darwin"' + ' and platform_machine == "arm64" and python_version < "3.9"' + ), + ( + 'python_version >= "3.7" and platform_system == "Darwin"' + ' and platform_machine == "arm64" and python_version < "3.8"' + ), + ] + ] + assert ( + str(dnf(intersection(*markers).invert())) + == 'platform_system == "Darwin" and platform_machine == "arm64"' + ' and python_version >= "3.6" or python_version >= "3.10"' + ) + + @pytest.mark.parametrize( "python_version, python_full_version, " "expected_intersection_version, expected_union_version",