Skip to content

Commit

Permalink
performance(markers): performance fix for marker simplifications
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@radoering @dimbleby @neersighted
2 people authored and neersighted committed Jan 22, 2023
1 parent 5a6a250 commit 8872e68
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 6 deletions.
74 changes: 74 additions & 0 deletions src/poetry/core/version/markers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion tests/packages/utils/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
{
"python_version": [
[("<", "3.6")],
[("<", "3.3")],
[("<", "3.6"), (">=", "3.3")],
[("<", "3.3")],
],
"sys_platform": [
[("==", "win32")],
Expand Down
69 changes: 64 additions & 5 deletions tests/version/test_markers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 8872e68

Please sign in to comment.