From 64cce29ec2bd0a0c650683526f8930426576db17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 25 Jan 2025 18:01:35 +0100 Subject: [PATCH] improve performance for merging marker overrides - volume 2 --- src/poetry/puzzle/solver.py | 31 ++++++++++++++--- tests/puzzle/test_solver_internals.py | 48 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index 7bbce2e4b5b..3bb4c43851d 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -9,6 +9,8 @@ from poetry.core.version.markers import AnyMarker from poetry.core.version.markers import EmptyMarker +from poetry.core.version.markers import MultiMarker +from poetry.core.version.markers import SingleMarker from poetry.core.version.markers import parse_marker from poetry.mixology import resolve_version @@ -428,11 +430,20 @@ def merge_override_packages( for dep in deps.values(): override_marker = override_marker.intersect(dep.marker.without_extras()) for package, info in o_packages.items(): + for group, marker in info.markers.items(): + # `override_marker` is often a SingleMarker or a MultiMarker, + # `marker` often is a MultiMarker that contains override_marker. + # We can "remove" the `override_marker` from `marker` + # because we will do an intersection later anyway. + # By removing it now, it is more likely that we hit + # the performance shortcut instead of the fallback algorithm. + info.markers[group] = remove_other_from_marker(marker, override_marker) all_packages.setdefault(package, []).append( (package, info, override_marker) ) for package_duplicates in all_packages.values(): base = package_duplicates[0] + remaining = package_duplicates[1:] package = base[0] package_info = base[1] first_override_marker = base[2] @@ -441,9 +452,7 @@ def merge_override_packages( package_info.groups = { g for _, info, _ in package_duplicates for g in info.groups } - if all( - info.markers == package_info.markers for _, info, _ in package_duplicates - ): + if all(info.markers == package_info.markers for _, info, _ in remaining): # performance shortcut: # if markers are the same for all overrides, # we can use less expensive marker operations @@ -458,18 +467,30 @@ def merge_override_packages( # fallback / general algorithm with performance issues for group, marker in package_info.markers.items(): package_info.markers[group] = first_override_marker.intersect(marker) - for _, info, override_marker in package_duplicates[1:]: + for _, info, override_marker in remaining: for group, marker in info.markers.items(): package_info.markers[group] = package_info.markers.get( group, EmptyMarker() ).union(override_marker.intersect(marker)) - for duplicate_package, _, _ in package_duplicates[1:]: + for duplicate_package, _, _ in remaining: for dep in duplicate_package.requires: if dep not in package.requires: package.add_dependency(dep) return result +def remove_other_from_marker(marker: BaseMarker, other: BaseMarker) -> BaseMarker: + if isinstance(other, SingleMarker): + other_markers: set[BaseMarker] = {other} + elif isinstance(other, MultiMarker): + other_markers = set(other.markers) + else: + return marker + if isinstance(marker, MultiMarker) and other_markers.issubset(marker.markers): + return MultiMarker.of(*(m for m in marker.markers if m not in other_markers)) + return marker + + @functools.cache def simplify_marker( marker: BaseMarker, python_constraint: VersionConstraint diff --git a/tests/puzzle/test_solver_internals.py b/tests/puzzle/test_solver_internals.py index 9cfbac30da6..5fa07a26628 100644 --- a/tests/puzzle/test_solver_internals.py +++ b/tests/puzzle/test_solver_internals.py @@ -513,4 +513,52 @@ def test_merge_override_packages_groups(package: ProjectPackage) -> None: } +def test_merge_override_packages_shortcut(package: ProjectPackage) -> None: + a = Package("a", "1") + common_marker = ( + 'extra == "test" and sys_platform == "win32" or platform_system == "Windows"' + ' or sys_platform == "linux" and extra == "stretch"' + ) + override_marker1 = 'python_version >= "3.12" and platform_system != "Emscripten"' + override_marker2 = 'python_version >= "3.12" and platform_system == "Emscripten"' + + packages = merge_override_packages( + [ + ( + {package: {"a": dep("b", override_marker1)}}, + { + a: TransitivePackageInfo( + 0, + {"main"}, + { + "main": parse_marker( + f"{override_marker1} and ({common_marker})" + ) + }, + ) + }, + ), + ( + {package: {"a": dep("b", override_marker2)}}, + { + a: TransitivePackageInfo( + 0, + {"main"}, + { + "main": parse_marker( + f"{override_marker2} and ({common_marker})" + ) + }, + ) + }, + ), + ] + ) + assert len(packages) == 1 + assert packages[a].groups == {"main"} + assert tm(packages[a]) == { + "main": f'({common_marker}) and python_version >= "3.12"' + } + + # TODO: root extras