From 614f5394b50ac88b7aa4e97b43acb08aca383d95 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Oct 2021 14:28:16 +0300 Subject: [PATCH 1/2] Avoid `@lru_cache` on methods The problem with `@lru_cache` on methods is that it also captures `self` and leaks it until the entry is evicted (if ever). --- src/_pytest/assertion/rewrite.py | 7 ++----- src/_pytest/config/__init__.py | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 8c85240ef..99416aed2 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -554,6 +554,7 @@ def set_location(node, lineno, col_offset): return node +@functools.lru_cache(maxsize=1) def _get_assertion_exprs(src: bytes) -> Dict[int, str]: """Return a mapping from {lineno: "assertion test expression"}.""" ret: Dict[int, str] = {} @@ -675,10 +676,6 @@ class AssertionRewriter(ast.NodeVisitor): self.enable_assertion_pass_hook = False self.source = source - @functools.lru_cache(maxsize=1) - def _assert_expr_to_lineno(self) -> Dict[int, str]: - return _get_assertion_exprs(self.source) - def run(self, mod: ast.Module) -> None: """Find all assert statements in *mod* and rewrite them.""" if not mod.body: @@ -906,7 +903,7 @@ class AssertionRewriter(ast.NodeVisitor): # Passed fmt_pass = self.helper("_format_explanation", msg) - orig = self._assert_expr_to_lineno()[assert_.lineno] + orig = _get_assertion_exprs(self.source)[assert_.lineno] hook_call_pass = ast.Expr( self.helper( "_call_assertion_pass", diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 0af41215b..b4fce09a8 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -522,7 +522,6 @@ class PytestPluginManager(PluginManager): if x.is_dir(): self._getconftestmodules(x, importmode, rootpath) - @lru_cache(maxsize=128) def _getconftestmodules( self, path: Path, importmode: Union[str, ImportMode], rootpath: Path ) -> List[types.ModuleType]: From 16e5fbe3714b0e1b0ab0d27bf3938bf2218c90cd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Oct 2021 14:50:57 +0300 Subject: [PATCH 2/2] config: optimize PytestPluginManager._getconftestmodules Now that it's no longer using `@lru_cache`, use another check to avoid re-computation. Although `@lru_cache` is faster than the full function call + checks, this approach also has the advantage that the caching works for more than 128 entries. --- src/_pytest/config/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index b4fce09a8..de5b80e30 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -533,12 +533,19 @@ class PytestPluginManager(PluginManager): else: directory = path + # Optimization: avoid repeated searches in the same directory. + # Assumes always called with same importmode and rootpath. + existing_clist = self._dirpath2confmods.get(directory) + if existing_clist: + return existing_clist + # XXX these days we may rather want to use config.rootpath # and allow users to opt into looking into the rootdir parent # directories instead of requiring to specify confcutdir. clist = [] + confcutdir_parents = self._confcutdir.parents if self._confcutdir else [] for parent in reversed((directory, *directory.parents)): - if self._confcutdir and parent in self._confcutdir.parents: + if parent in confcutdir_parents: continue conftestpath = parent / "conftest.py" if conftestpath.is_file():