Merge pull request #12409 from bluetech/reorder-items-perf

fixtures: fix catastrophic performance problem in `reorder_items`

Author: bluetech

changelog/12355.bugfix.rst

@@ -0,0 +1 @@
+Fix possible catastrophic performance slowdown on a certain parametrization pattern involving many higher-scoped parameters.

src/_pytest/fixtures.py

@@ -21,9 +21,11 @@
 from typing import Iterable
 from typing import Iterator
 from typing import List
+from typing import Mapping
 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
@@ -76,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
@@ -161,6 +161,12 @@ def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]:
     )
 
 
+# Algorithm for sorting on a per-parametrized resource setup basis.
+# It is called for Session scope first and performs sorting
+# down to the lower scopes such as to minimize number of "high scope"
+# setups and teardowns.
+
+
 @dataclasses.dataclass(frozen=True)
 class FixtureArgKey:
     argname: str
@@ -169,97 +175,95 @@ class FixtureArgKey:
     item_cls: Optional[type]
 
 
-def get_parametrized_fixture_keys(
+_V = TypeVar("_V")
+OrderedSet = Dict[_V, None]
+
+
+def get_parametrized_fixture_argkeys(
     item: nodes.Item, scope: Scope
 ) -> Iterator[FixtureArgKey]:
     """Return list of keys for all parametrized arguments which match
     the specified scope."""
     assert scope is not Scope.Function
+
     try:
         callspec: CallSpec2 = item.callspec  # type: ignore[attr-defined]
     except AttributeError:
         return
+
+    item_cls = None
+    if scope is Scope.Session:
+        scoped_item_path = None
+    elif scope is Scope.Package:
+        # Package key = module's directory.
+        scoped_item_path = item.path.parent
+    elif scope is Scope.Module:
+        scoped_item_path = item.path
+    elif scope is Scope.Class:
+        scoped_item_path = item.path
+        item_cls = item.cls  # type: ignore[attr-defined]
+    else:
+        assert_never(scope)
+
     for argname in callspec.indices:
         if callspec._arg2scope[argname] != scope:
             continue
-
-        item_cls = None
-        if scope is Scope.Session:
-            scoped_item_path = None
-        elif scope is Scope.Package:
-            # Package key = module's directory.
-            scoped_item_path = item.path.parent
-        elif scope is Scope.Module:
-            scoped_item_path = item.path
-        elif scope is Scope.Class:
-            scoped_item_path = item.path
-            item_cls = item.cls  # type: ignore[attr-defined]
-        else:
-            assert_never(scope)
-
         param_index = callspec.indices[argname]
         yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls)
 
 
-# Algorithm for sorting on a per-parametrized resource setup basis.
-# It is called for Session scope first and performs sorting
-# down to the lower scopes such as to minimize number of "high scope"
-# setups and teardowns.
-
-
 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]]] = {}
+    argkeys_by_item: Dict[Scope, Dict[nodes.Item, OrderedSet[FixtureArgKey]]] = {}
+    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_argkeys_by_item = argkeys_by_item[scope] = {}
+        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)
-    items_dict = dict.fromkeys(items, None)
+            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][item] = None
+
+    items_set = dict.fromkeys(items)
     return list(
-        reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session)
+        reorder_items_atscope(
+            items_set, argkeys_by_item, items_by_argkey, Scope.Session
+        )
     )
 
 
-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]"]],
-) -> None:
-    for scope in HIGH_SCOPES:
-        for key in argkeys_cache[scope].get(item, []):
-            items_by_argkey[scope][key].appendleft(item)
-
-
 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: OrderedSet[nodes.Item],
+    argkeys_by_item: Mapping[Scope, Mapping[nodes.Item, OrderedSet[FixtureArgKey]]],
+    items_by_argkey: Mapping[
+        Scope, Mapping[FixtureArgKey, OrderedDict[nodes.Item, None]]
+    ],
     scope: Scope,
-) -> Dict[nodes.Item, None]:
+) -> OrderedSet[nodes.Item]:
     if scope is Scope.Function or len(items) < 3:
         return items
-    ignore: Set[Optional[FixtureArgKey]] = set()
-    items_deque = deque(items)
-    items_done: Dict[nodes.Item, None] = {}
+
     scoped_items_by_argkey = items_by_argkey[scope]
-    scoped_argkeys_cache = argkeys_cache[scope]
+    scoped_argkeys_by_item = argkeys_by_item[scope]
+
+    ignore: Set[FixtureArgKey] = set()
+    items_deque = deque(items)
+    items_done: OrderedSet[nodes.Item] = {}
     while items_deque:
-        no_argkey_group: Dict[nodes.Item, None] = {}
+        no_argkey_items: OrderedSet[nodes.Item] = {}
         slicing_argkey = None
         while items_deque:
             item = items_deque.popleft()
-            if item in items_done or item in no_argkey_group:
+            if item in items_done or item in no_argkey_items:
                 continue
             argkeys = dict.fromkeys(
-                (k for k in scoped_argkeys_cache.get(item, []) if k not in ignore), None
+                k for k in scoped_argkeys_by_item.get(item, ()) if k not in ignore
             )
             if not argkeys:
-                no_argkey_group[item] = None
+                no_argkey_items[item] = None
             else:
                 slicing_argkey, _ = argkeys.popitem()
                 # We don't have to remove relevant items from later in the
@@ -268,16 +272,23 @@ def reorder_items_atscope(
                     i for i in scoped_items_by_argkey[slicing_argkey] if i in items
                 ]
                 for i in reversed(matching_items):
-                    fix_cache_order(i, argkeys_cache, items_by_argkey)
                     items_deque.appendleft(i)
+                    # Fix items_by_argkey order.
+                    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][i] = None
+                            other_scoped_items_by_argkey[argkey].move_to_end(
+                                i, last=False
+                            )
                 break
-        if no_argkey_group:
-            no_argkey_group = reorder_items_atscope(
-                no_argkey_group, argkeys_cache, items_by_argkey, scope.next_lower()
+        if no_argkey_items:
+            reordered_no_argkey_items = reorder_items_atscope(
+                no_argkey_items, argkeys_by_item, items_by_argkey, scope.next_lower()
             )
-            for item in no_argkey_group:
-                items_done[item] = None
-        ignore.add(slicing_argkey)
+            items_done.update(reordered_no_argkey_items)
+        if slicing_argkey is not None:
+            ignore.add(slicing_argkey)
     return items_done
 
 

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: