From e89d23b24741c001e8651a77303992cfa41c1664 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 2 Jun 2024 17:57:38 +0300 Subject: [PATCH] fixtures: fix catastrophic performance problem in `reorder_items` Fix #12355. In the issue, it was reported that the `reorder_items` has quadratic (or worse...) behavior with certain simple parametrizations. After some debugging I found that the problem happens because the "Fix items_by_argkey order" loop keeps adding the same item to the deque, and it reaches epic sizes which causes the slowdown. I don't claim to understand how the `reorder_items` algorithm works, but if as far as I understand, if an item already exists in the deque, the correct thing to do is to move it to the front. Since a deque doesn't have such an (efficient) operation, this switches to `OrderedDict` which can efficiently append from both sides, deduplicate and move to front. --- changelog/12355.bugfix.rst | 1 + src/_pytest/fixtures.py | 20 +++++++++++++------- testing/python/fixtures.py | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 changelog/12355.bugfix.rst diff --git a/changelog/12355.bugfix.rst b/changelog/12355.bugfix.rst new file mode 100644 index 00000000000..1ce43e60ebd --- /dev/null +++ b/changelog/12355.bugfix.rst @@ -0,0 +1 @@ +Fix possible catastrophic performance slowdown on a certain parametrization pattern involving many higher-scoped parameters. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 37fc44ff9c9..353082c17e1 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -25,6 +25,7 @@ from typing import MutableMapping from typing import NoReturn from typing import Optional +from typing import OrderedDict from typing import overload from typing import Sequence from typing import Set @@ -77,8 +78,6 @@ if TYPE_CHECKING: - from typing import Deque - from _pytest.main import Session from _pytest.python import CallSpec2 from _pytest.python import Function @@ -215,16 +214,18 @@ def get_parametrized_fixture_argkeys( def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: argkeys_by_item: Dict[Scope, Dict[nodes.Item, OrderedSet[FixtureArgKey]]] = {} - items_by_argkey: Dict[Scope, Dict[FixtureArgKey, Deque[nodes.Item]]] = {} + items_by_argkey: Dict[ + Scope, Dict[FixtureArgKey, OrderedDict[nodes.Item, None]] + ] = {} for scope in HIGH_SCOPES: scoped_argkeys_by_item = argkeys_by_item[scope] = {} - scoped_items_by_argkey = items_by_argkey[scope] = defaultdict(deque) + scoped_items_by_argkey = items_by_argkey[scope] = defaultdict(OrderedDict) for item in items: argkeys = dict.fromkeys(get_parametrized_fixture_argkeys(item, scope)) if argkeys: scoped_argkeys_by_item[item] = argkeys for argkey in argkeys: - scoped_items_by_argkey[argkey].append(item) + scoped_items_by_argkey[argkey][item] = None items_set = dict.fromkeys(items) return list( @@ -237,7 +238,9 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: def reorder_items_atscope( items: OrderedSet[nodes.Item], argkeys_by_item: Mapping[Scope, Mapping[nodes.Item, OrderedSet[FixtureArgKey]]], - items_by_argkey: Mapping[Scope, Mapping[FixtureArgKey, "Deque[nodes.Item]"]], + items_by_argkey: Mapping[ + Scope, Mapping[FixtureArgKey, OrderedDict[nodes.Item, None]] + ], scope: Scope, ) -> OrderedSet[nodes.Item]: if scope is Scope.Function or len(items) < 3: @@ -274,7 +277,10 @@ def reorder_items_atscope( for other_scope in HIGH_SCOPES: other_scoped_items_by_argkey = items_by_argkey[other_scope] for argkey in argkeys_by_item[other_scope].get(i, ()): - other_scoped_items_by_argkey[argkey].appendleft(i) + other_scoped_items_by_argkey[argkey][i] = None + other_scoped_items_by_argkey[argkey].move_to_end( + i, last=False + ) break if no_argkey_items: reordered_no_argkey_items = reorder_items_atscope( diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index d54bcc4d4eb..d3cff38f977 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2219,6 +2219,25 @@ def test_check(): reprec = pytester.inline_run("-s") reprec.assertoutcome(passed=2) + def test_reordering_catastrophic_performance(self, pytester: Pytester) -> None: + """Check that a certain high-scope parametrization pattern doesn't cause + a catasrophic slowdown. + + Regression test for #12355. + """ + pytester.makepyfile(""" + import pytest + + params = tuple("abcdefghijklmnopqrstuvwxyz") + @pytest.mark.parametrize(params, [range(len(params))] * 3, scope="module") + def test_parametrize(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z): + pass + """) + + result = pytester.runpytest() + + result.assert_outcomes(passed=3) + class TestFixtureMarker: def test_parametrize(self, pytester: Pytester) -> None: