Skip to content

Commit b4b7729

Browse files
authored
improve performance for merging marker overrides - volume 2 (#10111)
1 parent 9afecf3 commit b4b7729

File tree

2 files changed

+74
-5
lines changed

2 files changed

+74
-5
lines changed

src/poetry/puzzle/solver.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
from poetry.core.version.markers import AnyMarker
1111
from poetry.core.version.markers import EmptyMarker
12+
from poetry.core.version.markers import MultiMarker
13+
from poetry.core.version.markers import SingleMarker
1214
from poetry.core.version.markers import parse_marker
1315

1416
from poetry.mixology import resolve_version
@@ -428,11 +430,20 @@ def merge_override_packages(
428430
for dep in deps.values():
429431
override_marker = override_marker.intersect(dep.marker.without_extras())
430432
for package, info in o_packages.items():
433+
for group, marker in info.markers.items():
434+
# `override_marker` is often a SingleMarker or a MultiMarker,
435+
# `marker` often is a MultiMarker that contains `override_marker`.
436+
# We can "remove" `override_marker` from `marker`
437+
# because we will do an intersection later anyway.
438+
# By removing it now, it is more likely that we hit
439+
# the performance shortcut instead of the fallback algorithm.
440+
info.markers[group] = remove_other_from_marker(marker, override_marker)
431441
all_packages.setdefault(package, []).append(
432442
(package, info, override_marker)
433443
)
434444
for package_duplicates in all_packages.values():
435445
base = package_duplicates[0]
446+
remaining = package_duplicates[1:]
436447
package = base[0]
437448
package_info = base[1]
438449
first_override_marker = base[2]
@@ -441,9 +452,7 @@ def merge_override_packages(
441452
package_info.groups = {
442453
g for _, info, _ in package_duplicates for g in info.groups
443454
}
444-
if all(
445-
info.markers == package_info.markers for _, info, _ in package_duplicates
446-
):
455+
if all(info.markers == package_info.markers for _, info, _ in remaining):
447456
# performance shortcut:
448457
# if markers are the same for all overrides,
449458
# we can use less expensive marker operations
@@ -458,18 +467,30 @@ def merge_override_packages(
458467
# fallback / general algorithm with performance issues
459468
for group, marker in package_info.markers.items():
460469
package_info.markers[group] = first_override_marker.intersect(marker)
461-
for _, info, override_marker in package_duplicates[1:]:
470+
for _, info, override_marker in remaining:
462471
for group, marker in info.markers.items():
463472
package_info.markers[group] = package_info.markers.get(
464473
group, EmptyMarker()
465474
).union(override_marker.intersect(marker))
466-
for duplicate_package, _, _ in package_duplicates[1:]:
475+
for duplicate_package, _, _ in remaining:
467476
for dep in duplicate_package.requires:
468477
if dep not in package.requires:
469478
package.add_dependency(dep)
470479
return result
471480

472481

482+
def remove_other_from_marker(marker: BaseMarker, other: BaseMarker) -> BaseMarker:
483+
if isinstance(other, SingleMarker):
484+
other_markers: set[BaseMarker] = {other}
485+
elif isinstance(other, MultiMarker):
486+
other_markers = set(other.markers)
487+
else:
488+
return marker
489+
if isinstance(marker, MultiMarker) and other_markers.issubset(marker.markers):
490+
return MultiMarker.of(*(m for m in marker.markers if m not in other_markers))
491+
return marker
492+
493+
473494
@functools.cache
474495
def simplify_marker(
475496
marker: BaseMarker, python_constraint: VersionConstraint

tests/puzzle/test_solver_internals.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,4 +513,52 @@ def test_merge_override_packages_groups(package: ProjectPackage) -> None:
513513
}
514514

515515

516+
def test_merge_override_packages_shortcut(package: ProjectPackage) -> None:
517+
a = Package("a", "1")
518+
common_marker = (
519+
'extra == "test" and sys_platform == "win32" or platform_system == "Windows"'
520+
' or sys_platform == "linux" and extra == "stretch"'
521+
)
522+
override_marker1 = 'python_version >= "3.12" and platform_system != "Emscripten"'
523+
override_marker2 = 'python_version >= "3.12" and platform_system == "Emscripten"'
524+
525+
packages = merge_override_packages(
526+
[
527+
(
528+
{package: {"a": dep("b", override_marker1)}},
529+
{
530+
a: TransitivePackageInfo(
531+
0,
532+
{"main"},
533+
{
534+
"main": parse_marker(
535+
f"{override_marker1} and ({common_marker})"
536+
)
537+
},
538+
)
539+
},
540+
),
541+
(
542+
{package: {"a": dep("b", override_marker2)}},
543+
{
544+
a: TransitivePackageInfo(
545+
0,
546+
{"main"},
547+
{
548+
"main": parse_marker(
549+
f"{override_marker2} and ({common_marker})"
550+
)
551+
},
552+
)
553+
},
554+
),
555+
]
556+
)
557+
assert len(packages) == 1
558+
assert packages[a].groups == {"main"}
559+
assert tm(packages[a]) == {
560+
"main": f'({common_marker}) and python_version >= "3.12"'
561+
}
562+
563+
516564
# TODO: root extras

0 commit comments

Comments
 (0)