Skip to content

pytester: patch GC_COLLECT_ITERATIONS to speed up the test suite #13513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicoddemus
Copy link
Member

Because pytester.runpytest() executes the full session cycle (including pytest_unconfigure), it was calling gc.collect() in a loop multiple times—even for small, fast tests. This significantly increased the total test suite runtime.

To optimize performance, pytester now patches GC_COLLECT_ITERATIONS to skip gc.collect() entirely, matching the behavior before #12958.

Locally the test suite runtime improved dramatically, dropping from 425s to 160s.

Fixes #13482.

@nicoddemus nicoddemus added skip news used on prs to opt out of the changelog requirement backport 8.4.x apply to PRs at any point; backports the changes to the 8.4.x branch labels Jun 14, 2025
@nicoddemus nicoddemus requested a review from graingert June 14, 2025 15:02
@nicoddemus
Copy link
Member Author

The unittestextras failure is being fixed in #13502.

Because `pytester.runpytest()` executes the full session cycle (including `pytest_unconfigure`), it was calling `gc.collect()` in a loop multiple times—even for small, fast tests. This significantly increased the total test suite runtime.

To optimize performance, pytester now patches `GC_COLLECT_ITERATIONS` to skip `gc.collect()` entirely, matching the behavior before pytest-dev#12958.

Locally the test suite runtime improved dramatically, dropping from 425s to 160s.

Fixes pytest-dev#13482.
@jakkdl
Copy link
Member

jakkdl commented Jun 15, 2025

are these hooks before or after collection? Or some other way to disable it once we've done collection.
Could maybe be nice to disable the hard-gc-collect whenever a user only runs a single test, so they don't get test-time degradation either.
but maybe it's not noticeable for end users

@nicoddemus
Copy link
Member Author

nicoddemus commented Jun 15, 2025

are these hooks before or after collection? Or some other way to disable it once we've done collection.
Could maybe be nice to disable the hard-gc-collect whenever a user only runs a single test, so they don't get test-time degradation either.
but maybe it's not noticeable for end users

The gc_collect_harder call happens only during pytest_unconfigure. For end users, gc_collect_harder is only called when pytest is about to shutdown, so they have minimal impact.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am inclined against the mocking approach mostly because it modifies global state and thus is not thread safe. Obviously pytest doesn't run tests in parallel in multiple threads, but it's always in the back of my head that someday it would be nice to allow this, though definitely not a blocker (the unraisableexception plugin itself modifies global state after all...).

If we want to avoid global state, I figure it has to go through the sub-pytest config. We can achieve this using a plugin, e.g.

diff --git i/src/_pytest/unraisableexception.py w/src/_pytest/unraisableexception.py
index 7826aeccd..332230041 100644
--- i/src/_pytest/unraisableexception.py
+++ w/src/_pytest/unraisableexception.py
@@ -24,10 +24,11 @@ if sys.version_info < (3, 11):
     from exceptiongroup import ExceptionGroup
 
 
-def gc_collect_harder() -> None:
-    # A single collection doesn't necessarily collect everything.
-    # Constant determined experimentally by the Trio project.
-    for _ in range(5):
+gc_collect_iterations_key = StashKey[int]()
+
+
+def gc_collect_harder(iterations: int) -> None:
+    for _ in range(iterations):
         gc.collect()
 
 
@@ -84,9 +85,12 @@ def collect_unraisable(config: Config) -> None:
 def cleanup(
     *, config: Config, prev_hook: Callable[[sys.UnraisableHookArgs], object]
 ) -> None:
+    # A single collection doesn't necessarily collect everything.
+    # Constant determined experimentally by the Trio project.
+    gc_collect_iterations = config.stash.get(gc_collect_iterations_key, 5)
     try:
         try:
-            gc_collect_harder()
+            gc_collect_harder(gc_collect_iterations)
             collect_unraisable(config)
         finally:
             sys.unraisablehook = prev_hook
diff --git i/src/_pytest/pytester.py w/src/_pytest/pytester.py
index 38f4643bd..9a47a1146 100644
--- i/src/_pytest/pytester.py
+++ w/src/_pytest/pytester.py
@@ -65,6 +65,7 @@ from _pytest.pathlib import make_numbered_dir
 from _pytest.reports import CollectReport
 from _pytest.reports import TestReport
 from _pytest.tmpdir import TempPathFactory
+from _pytest.unraisableexception import gc_collect_iterations_key
 from _pytest.warning_types import PytestFDWarning
 
 
@@ -1115,12 +1116,15 @@ class Pytester:
 
             rec = []
 
-            class Collect:
+            class PytesterHelperPlugin:
                 @staticmethod
                 def pytest_configure(config: Config) -> None:
                     rec.append(self.make_hook_recorder(config.pluginmanager))
 
-            plugins.append(Collect())
+                    # The unraisable plugin GC collect slows down pytester too much.
+                    config.stash[gc_collect_iterations_key] = 0
+
+            plugins.append(PytesterHelperPlugin())
             ret = main([str(x) for x in args], plugins=plugins)
             if len(rec) == 1:
                 reprec = rec.pop()

Another approach can be to just turn off the unraisableexception plugin in pytester by default, and require tests that want it to explicitly enable it.


I'd also note that that the mock approach (and the helper plugin approach) don't work for pytester subprocess mode.

@nicoddemus
Copy link
Member Author

nicoddemus commented Jun 16, 2025

@bluetech I'm OK with that approach! Could you go ahead and push that patch you posted to this branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 8.4.x apply to PRs at any point; backports the changes to the 8.4.x branch skip news used on prs to opt out of the changelog requirement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[self-tests] test time degradation
3 participants