Skip to content

Commit

Permalink
[8.2.x] fixtures: fix catastrophic performance problem in `reorder_it…
Browse files Browse the repository at this point in the history
…ems`

Manual minimal backport from commit e89d23b.

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.
  • Loading branch information
bluetech committed Jun 4, 2024
1 parent b41d5a5 commit 153a436
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
1 change: 1 addition & 0 deletions changelog/12355.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix possible catastrophic performance slowdown on a certain parametrization pattern involving many higher-scoped parameters.
19 changes: 11 additions & 8 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Check warning on line 220 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L220

Added line #L220 was not covered by tests
items_dict = dict.fromkeys(items, None)
return list(
reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session)
Expand All @@ -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]

Check warning on line 233 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L233

Added line #L233 was not covered by tests
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)

Check warning on line 236 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L235-L236

Added lines #L235 - L236 were not covered by tests


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:
Expand Down
19 changes: 19 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 153a436

Please sign in to comment.