Skip to content

improve performance for merging marker overrides - volume 2 #10111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions src/poetry/puzzle/solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" `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]
Expand All @@ -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
Expand All @@ -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
Expand Down
48 changes: 48 additions & 0 deletions tests/puzzle/test_solver_internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading