From 153a436bc40c9e89d90d62255ef5a89e9a762dca Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 2 Jun 2024 17:57:38 +0300 Subject: [PATCH] [8.2.x] fixtures: fix catastrophic performance problem in `reorder_items` Manual minimal backport from commit e89d23b24741c001e8651a77303992cfa41c1664. 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 | 19 +++++++++++-------- testing/python/fixtures.py | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 8 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 7fd63f937c1..647328e3bea 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -23,6 +23,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 @@ -75,8 +76,6 @@ if TYPE_CHECKING: - from typing import Deque - from _pytest.main import Session from _pytest.python import CallSpec2 from _pytest.python import Function @@ -207,16 +206,18 @@ def get_parametrized_fixture_keys( def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]] = {} - 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_cache = argkeys_cache[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: keys = dict.fromkeys(get_parametrized_fixture_keys(item, scope), None) if keys: scoped_argkeys_cache[item] = keys for key in keys: - scoped_items_by_argkey[key].append(item) + scoped_items_by_argkey[key][item] = None items_dict = dict.fromkeys(items, None) return list( reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session) @@ -226,17 +227,19 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: def fix_cache_order( item: nodes.Item, argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], - items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], + items_by_argkey: Dict[Scope, Dict[FixtureArgKey, OrderedDict[nodes.Item, None]]], ) -> None: for scope in HIGH_SCOPES: + scoped_items_by_argkey = items_by_argkey[scope] for key in argkeys_cache[scope].get(item, []): - items_by_argkey[scope][key].appendleft(item) + scoped_items_by_argkey[key][item] = None + scoped_items_by_argkey[key].move_to_end(item, last=False) def reorder_items_atscope( items: Dict[nodes.Item, None], argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], - items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], + items_by_argkey: Dict[Scope, Dict[FixtureArgKey, OrderedDict[nodes.Item, None]]], scope: Scope, ) -> Dict[nodes.Item, None]: if scope is Scope.Function or len(items) < 3: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 741cf7dcf42..ebdf451e40b 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: