pytest: [prerelease] Performance regression?

Tested on several internal projects, noticed almost 2x slowdown compared to 6.2.5 on py3.8

Unfortunately no reproducer, but I got .prof dumps from them: Dropbox - pytest - Simplify your life

Opening this to remind myself to have a look at those before the final release - but ideally we’d have a proper reproducer so we can e.g. bisect this. Does anyone have any guess what could be the culprit? I only see a very small slowdown for my own project.

@hroncok Is there perhaps a way to find out which packages in Fedora took significantly longer to run tests with pytest 7 compared to 6? This would mainly be for runs which still pass, because if there are failures, all bets regarding the runtime are off anyways.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 32 (32 by maintainers)

Commits related to this issue

config: fix incorrect cache hit check in _getconftestmodules This made the cache not work as intended, causing a major slowdown. See #9478 for discussion and context. Authored-by: Anthony Sottile <... — committed to bluetech/pytest by bluetech 2 years ago
config: avoid stat storm in _getconftestmodules Fix #9478. — committed to bluetech/pytest by bluetech 2 years ago

Most upvoted comments

I might be able to test that in Fedroa with the git HEAD as well even without rc.

hroncok on Jan 7, 2022

Oh, also, how do folks feel about a quick 7.0.0rc2 now? We still need to take care of #9002 and #9471 (plus potentially “unknown unknowns” from #9415?), but that would mean it’s easy for people (and probably Fedora?) to test the fixes which we have so far (and it makes it easier to ask the original reporter to test this fix too).

The-Compiler on Jan 7, 2022

Thanks to @bluetech @asottile @hroncok for an incredibly quick turnaround on what seemed like a tricky issue without much detail! ❤️

The-Compiler on Jan 7, 2022

an idea to get back to the original caching but without a memory leak would be to assign a per-instance lru_cache:

diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py
index dee3d4974..15a9c0ef1 100644
--- a/src/_pytest/config/__init__.py
+++ b/src/_pytest/config/__init__.py
@@ -354,6 +354,7 @@ class PytestPluginManager(PluginManager):
         self._confcutdir: Optional[Path] = None
         self._noconftest = False
         self._duplicatepaths: Set[Path] = set()
+        self._getconftestmodules = lru_cache(maxsize=128)(self._getconftestmodules_uncached)
 
         # plugins that were explicitly skipped with pytest.skip
         # list of (module name, skip reason)
@@ -524,7 +525,7 @@ class PytestPluginManager(PluginManager):
                 if x.is_dir():
                     self._getconftestmodules(x, importmode, rootpath)
 
-    def _getconftestmodules(
+    def _getconftestmodules_uncached(
         self, path: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> List[types.ModuleType]:
         if self._noconftest:

I used the same technique in my text editor though admittedly for a different reason

asottile on Jan 7, 2022

I’ll definitely look at some of them.

I’m going to bet that the slowness comes from the extra stats due to the pathlib migration, possibly with the addition of a slow filesystem setup on the Fedora/user’s system (maybe running NFS or such?). But I’m not making assumptions 😃

bluetech on Jan 6, 2022