Skip to content

Commit 394f651

Browse files
radoeringdimbleby
andcommitted
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 <[email protected]>
1 parent 8718681 commit 394f651

File tree

3 files changed

+139
-6
lines changed

3 files changed

+139
-6
lines changed

src/poetry/core/version/markers.py

+74
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,18 @@ def of(cls, *markers: BaseMarker) -> BaseMarker:
420420
intersected = True
421421
break
422422

423+
# If we have a MarkerUnion then we can look for the simplifications
424+
# implemented in intersect_simplify().
425+
elif isinstance(mark, MarkerUnion):
426+
intersection = mark.intersect_simplify(marker)
427+
if intersection is not None:
428+
new_markers[i] = intersection
429+
intersected = True
430+
break
431+
423432
if intersected:
433+
# flatten again because intersect_simplify may return a multi
434+
new_markers = _flatten_markers(new_markers, MultiMarker)
424435
continue
425436

426437
new_markers.append(marker)
@@ -451,6 +462,9 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None:
451462
452463
- union between two multimarkers where one is contained by the other is just
453464
the larger of the two
465+
466+
- union between two multimarkers where there are some common markers
467+
and the union of unique markers is a single marker
454468
"""
455469
if other in self._markers:
456470
return other
@@ -465,6 +479,22 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None:
465479
if their_markers.issubset(our_markers):
466480
return other
467481

482+
shared_markers = our_markers.intersection(their_markers)
483+
if not shared_markers:
484+
return None
485+
486+
unique_markers = our_markers - their_markers
487+
other_unique_markers = their_markers - our_markers
488+
unique_union = MultiMarker(*unique_markers).union(
489+
MultiMarker(*other_unique_markers)
490+
)
491+
if isinstance(unique_union, (SingleMarker, AnyMarker)):
492+
# Use list instead of set for deterministic order.
493+
common_markers = [
494+
marker for marker in self.markers if marker in shared_markers
495+
]
496+
return unique_union.intersect(MultiMarker(*common_markers))
497+
468498
return None
469499

470500
def validate(self, environment: dict[str, Any] | None) -> bool:
@@ -611,6 +641,50 @@ def intersect(self, other: BaseMarker) -> BaseMarker:
611641
def union(self, other: BaseMarker) -> BaseMarker:
612642
return union(self, other)
613643

644+
def intersect_simplify(self, other: BaseMarker) -> BaseMarker | None:
645+
"""
646+
Finds a couple of easy simplifications for intersection on MarkerUnions:
647+
648+
- intersection with any marker that appears as part of the union is just
649+
that marker
650+
651+
- intersection between two markerunions where one is contained by the other
652+
is just the smaller of the two
653+
654+
- intersection between two markerunions where there are some common markers
655+
and the intersection of unique markers is not a single marker
656+
"""
657+
if other in self._markers:
658+
return other
659+
660+
if isinstance(other, MarkerUnion):
661+
our_markers = set(self.markers)
662+
their_markers = set(other.markers)
663+
664+
if our_markers.issubset(their_markers):
665+
return self
666+
667+
if their_markers.issubset(our_markers):
668+
return other
669+
670+
shared_markers = our_markers.intersection(their_markers)
671+
if not shared_markers:
672+
return None
673+
674+
unique_markers = our_markers - their_markers
675+
other_unique_markers = their_markers - our_markers
676+
unique_intersection = MarkerUnion(*unique_markers).intersect(
677+
MarkerUnion(*other_unique_markers)
678+
)
679+
if isinstance(unique_intersection, (SingleMarker, EmptyMarker)):
680+
# Use list instead of set for deterministic order.
681+
common_markers = [
682+
marker for marker in self.markers if marker in shared_markers
683+
]
684+
return unique_intersection.union(MarkerUnion(*common_markers))
685+
686+
return None
687+
614688
def validate(self, environment: dict[str, Any] | None) -> bool:
615689
return any(m.validate(environment) for m in self._markers)
616690

tests/packages/utils/test_utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
{
2626
"python_version": [
2727
[("<", "3.6")],
28-
[("<", "3.3")],
2928
[("<", "3.6"), (">=", "3.3")],
29+
[("<", "3.3")],
3030
],
3131
"sys_platform": [
3232
[("==", "win32")],

tests/version/test_markers.py

+64-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
from poetry.core.version.markers import SingleMarker
1414
from poetry.core.version.markers import cnf
1515
from poetry.core.version.markers import dnf
16+
from poetry.core.version.markers import intersection
1617
from poetry.core.version.markers import parse_marker
18+
from poetry.core.version.markers import union
1719

1820

1921
if TYPE_CHECKING:
@@ -1282,11 +1284,6 @@ def test_union_should_drop_markers_if_their_complement_is_present(
12821284
),
12831285
),
12841286
MultiMarker(
1285-
MarkerUnion(
1286-
SingleMarker("sys_platform", "!=win32"),
1287-
SingleMarker("python_version", "<3.8"),
1288-
SingleMarker("python_version", ">=3.9"),
1289-
),
12901287
MarkerUnion(
12911288
SingleMarker("python_version", "<3.8"),
12921289
SingleMarker("python_version", ">=3.9"),
@@ -1553,6 +1550,68 @@ def test_empty_marker_is_found_in_complex_parse() -> None:
15531550
assert marker.is_empty()
15541551

15551552

1553+
def test_complex_union() -> None:
1554+
# real world example on the way to get mutually exclusive markers
1555+
# for numpy(>=1.21.2) of https://pypi.org/project/opencv-python/4.6.0.66/
1556+
markers = [
1557+
parse_marker(m)
1558+
for m in [
1559+
(
1560+
'python_version < "3.7" and python_version >= "3.6"'
1561+
' and platform_system == "Darwin" and platform_machine == "arm64"'
1562+
),
1563+
(
1564+
'python_version >= "3.10" or python_version >= "3.9"'
1565+
' and platform_system == "Darwin" and platform_machine == "arm64"'
1566+
),
1567+
(
1568+
'python_version >= "3.8" and platform_system == "Darwin"'
1569+
' and platform_machine == "arm64" and python_version < "3.9"'
1570+
),
1571+
(
1572+
'python_version >= "3.7" and platform_system == "Darwin"'
1573+
' and platform_machine == "arm64" and python_version < "3.8"'
1574+
),
1575+
]
1576+
]
1577+
assert (
1578+
str(union(*markers))
1579+
== 'platform_system == "Darwin" and platform_machine == "arm64"'
1580+
' and python_version >= "3.6" or python_version >= "3.10"'
1581+
)
1582+
1583+
1584+
def test_complex_intersection() -> None:
1585+
# inverse of real world example on the way to get mutually exclusive markers
1586+
# for numpy(>=1.21.2) of https://pypi.org/project/opencv-python/4.6.0.66/
1587+
markers = [
1588+
parse_marker(m).invert()
1589+
for m in [
1590+
(
1591+
'python_version < "3.7" and python_version >= "3.6"'
1592+
' and platform_system == "Darwin" and platform_machine == "arm64"'
1593+
),
1594+
(
1595+
'python_version >= "3.10" or python_version >= "3.9"'
1596+
' and platform_system == "Darwin" and platform_machine == "arm64"'
1597+
),
1598+
(
1599+
'python_version >= "3.8" and platform_system == "Darwin"'
1600+
' and platform_machine == "arm64" and python_version < "3.9"'
1601+
),
1602+
(
1603+
'python_version >= "3.7" and platform_system == "Darwin"'
1604+
' and platform_machine == "arm64" and python_version < "3.8"'
1605+
),
1606+
]
1607+
]
1608+
assert (
1609+
str(dnf(intersection(*markers).invert()))
1610+
== 'platform_system == "Darwin" and platform_machine == "arm64"'
1611+
' and python_version >= "3.6" or python_version >= "3.10"'
1612+
)
1613+
1614+
15561615
@pytest.mark.parametrize(
15571616
"python_version, python_full_version, "
15581617
"expected_intersection_version, expected_union_version",

0 commit comments

Comments
 (0)