Skip to content

Commit

Permalink
fixtures: fix catastrophic performance problem in reorder_items
Browse files Browse the repository at this point in the history
Fix pytest-dev#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 1eee63a commit e89d23b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 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.
20 changes: 13 additions & 7 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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:
Expand Down Expand Up @@ -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(
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 e89d23b

Please sign in to comment.