reduce gc_collect_harder default to 1 on CPython#14441
Open
miketheman wants to merge 3 commits into
Open
Conversation
Contributor
Author
|
Draft, since I'm not certain this is a good idea. |
Member
|
Im nit familiar with the plugin myself |
Contributor
Author
|
Unknown - I'm looking at history, and there's already once case that resets it to 0 no matter what - #13482 |
Member
|
I was uneasy with the multiple gc rounds but I couldn't entirely demonstrate the slowdown. But for me your report is sufficient. The unraisableexception plugin is non-deterministic and doing multiple gc rounds trying to make it so just seems too heavy. So basically +1 from me. I would also not add a special case for PyPI. |
Member
|
See also 391324e. Also cc @graingert. |
Member
|
Thanks for the CC, I have no opinion on this though |
The 5-iteration default was borrowed from the Trio project, where it was determined empirically to handle PyPy's object resurrection behavior: on PyPy, objects like coroutines can survive GC rounds because executing their __del__ can resurrect them. On CPython, reference counting frees most objects immediately. One GC pass is sufficient to handle reference cycles, as confirmed by all test_unraisableexception tests passing (including the refcycle variants). Use 1 pass on CPython and retain 5 on PyPy. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
miketheman
force-pushed
the
miketheman/speed-up-test-sutie
branch
from
07255a2 to
e2bc38b
Compare
Using `timeit`, I found a faster call:
```python
import sys
import timeit
def method_hasattr():
return 5 if hasattr(sys, "pypy_version_info") else 1
def method_implementation():
return 5 if sys.implementation.name == "pypy" else 1
time1 = timeit.timeit(method_hasattr, number=10000)
time2 = timeit.timeit(method_implementation, number=10000)
print(f"Method Hasattr: {time1:.5f} seconds")
print(f"Method Implementation: {time2:.5f} seconds")
```
Results:
```
$ PYENV_VERSION=3.14.5 python microbench.py
Method Hasattr: 0.00085 seconds
Method Implementation: 0.00054 seconds
$ PYENV_VERSION=pypy3.11-7.3.22 python microbench.py
Method Hasattr: 0.00768 seconds
Method Implementation: 0.00070 seconds
```
Refs: https://docs.python.org/3/library/sys.html#sys.implementation
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
psf-chronographer
Bot
added
the
bot:chronographer:provided
Contributor
Author
|
I found a faster implementation, and added a changelog entry. If there's a benchamrking/test suite somewhere I can hook into, I'd be happy to explore that, but I didn't find one, so pointers would be appreciated. |
Contributor
Author
|
CI results compared:
Table 1: Total Job Time (wall clock)Table 2: Test Step Only (excludes setup/checkout/install/upload)Table 3: Test Step Summary by PlatformKey takeaways:
The signal is clear and consistent on Linux where runners are stable. Comparison script (uses Details#!/usr/bin/env python3
"""Compare test step timings between two GitHub Actions workflow runs.
Usage:
python scripts/compare_ci_runs.py <upstream_run_id> <our_run_id> [--repo REPO]
Example:
python scripts/compare_ci_runs.py 26336401841 26344890356
python scripts/compare_ci_runs.py 26336401841 26344890356 --repo pytest-dev/pytest
Requires: gh CLI authenticated with repo read access.
"""
from __future__ import annotations
import argparse
import json
import subprocess
import sys
from datetime import datetime
def secs(a: str | None, b: str | None) -> int | None:
if not a or not b:
return None
fmt = "%Y-%m-%dT%H:%M:%SZ"
try:
return int((datetime.strptime(b, fmt) - datetime.strptime(a, fmt)).total_seconds())
except ValueError:
return None
def fmt_secs(s: int | None) -> str:
if s is None:
return "—"
m, sec = divmod(s, 60)
return f"{m}m{sec:02d}s"
def delta_fmt(before: int | None, after: int | None) -> str:
if before is None or after is None:
return "—"
d = after - before
prefix = "+" if d >= 0 else "-"
m, s = divmod(abs(d), 60)
return f"{prefix}{m}m{s:02d}s" if m else f"{prefix}{s}s"
def pct(before: int | None, after: int | None) -> str:
if not before or after is None:
return "—"
p = ((after - before) / before) * 100
return f"{'+' if p > 0 else ''}{p:.1f}%"
def get_jobs(run_id: int, repo: str) -> list[dict]:
result = subprocess.run(
["gh", "run", "view", str(run_id), "--repo", repo, "--json", "jobs"],
capture_output=True,
text=True,
check=True,
)
return json.loads(result.stdout)["jobs"]
def parse_jobs(jobs: list[dict]) -> dict[str, dict]:
parsed = {}
for job in jobs:
if not job["name"].startswith("build"):
continue
job_secs = secs(job["startedAt"], job["completedAt"])
test_secs_total = sum(
secs(s["startedAt"], s["completedAt"]) or 0
for s in job["steps"]
if s["name"].startswith("Test ")
and (secs(s["startedAt"], s["completedAt"]) or 0) > 0
)
parsed[job["name"]] = {
"job_secs": job_secs,
"test_secs": test_secs_total or None,
}
return parsed
PLATFORM_GROUPS = {
"Linux (full suite)": ["ubuntu-py310", "ubuntu-py311", "ubuntu-py312", "ubuntu-py313", "ubuntu-py314"],
"Linux (variants)": [
"ubuntu-py310-freeze", "ubuntu-py310-lsof-numpy-pexpect", "ubuntu-py310-pluggy",
"ubuntu-py310-unittest-asynctest", "ubuntu-py310-unittest-twisted24",
"ubuntu-py310-unittest-twisted25", "ubuntu-py310-xdist", "ubuntu-py313-pexpect",
"ubuntu-pypy3-xdist",
],
"macOS": ["macos-py310", "macos-py312", "macos-py313", "macos-py314"],
"Windows": [
"windows-py310-pluggy", "windows-py310-unittest-asynctest", "windows-py310-unittest-twisted24",
"windows-py310-unittest-twisted25", "windows-py310-xdist", "windows-py311",
"windows-py312", "windows-py313", "windows-py314",
],
}
def get_group(short_name: str) -> str:
for group, members in PLATFORM_GROUPS.items():
if short_name in members:
return group
return "Other"
def print_table(headers: list[str], rows: list[list[str]], footer: list[str] | None = None) -> None:
widths = [max(len(str(row[i])) for row in ([headers] + rows + ([footer] if footer else []))) for i in range(len(headers))]
sep = "-" * (sum(widths) + 3 * len(widths) + 1)
fmt = " " + " ".join(f"{{:<{widths[0]}}}" if i == 0 else f"{{:>{widths[i]}}}" for i in range(len(headers)))
print(fmt.format(*headers))
print(sep)
for row in rows:
print(fmt.format(*row))
if footer:
print(sep)
print(fmt.format(*footer))
def main() -> None:
parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
parser.add_argument("upstream_run_id", type=int)
parser.add_argument("our_run_id", type=int)
parser.add_argument("--repo", default="pytest-dev/pytest")
args = parser.parse_args()
repo = args.repo
upstream_url = f"https://github.com/{repo}/actions/runs/{args.upstream_run_id}"
our_url = f"https://github.com/{repo}/actions/runs/{args.our_run_id}"
print(f"Fetching upstream run {args.upstream_run_id}...")
upstream = parse_jobs(get_jobs(args.upstream_run_id, repo))
print(f"Fetching our run {args.our_run_id}...")
ours = parse_jobs(get_jobs(args.our_run_id, repo))
all_names = sorted(set(upstream) | set(ours))
print(f"\n{'=' * 90}")
print(f"UPSTREAM: {upstream_url}")
print(f"OURS: {our_url}")
print(f"{'=' * 90}")
# Table 1: total job time
print("\n### Table 1: Total Job Time (wall clock)\n")
rows, total_u, total_o = [], 0, 0
for name in all_names:
short = name.removeprefix("build (").removesuffix(")")
u, o = upstream.get(name, {}), ours.get(name, {})
uj, oj = u.get("job_secs"), o.get("job_secs")
if uj: total_u += uj
if oj: total_o += oj
note = " [PyPy]" if "pypy" in name else ""
rows.append([short + note, fmt_secs(uj), fmt_secs(oj), delta_fmt(uj, oj), pct(uj, oj)])
print_table(
["Job", "Upstream", "Ours", "Delta", "% Change"],
rows,
["TOTAL (sum of all jobs)", fmt_secs(total_u), fmt_secs(total_o), delta_fmt(total_u, total_o), pct(total_u, total_o)],
)
# Table 2: test step only
print("\n\n### Table 2: Test Step Duration Only (excludes setup/checkout/install/upload)\n")
rows, total_u, total_o = [], 0, 0
for name in all_names:
short = name.removeprefix("build (").removesuffix(")")
u, o = upstream.get(name, {}), ours.get(name, {})
ut, ot = u.get("test_secs"), o.get("test_secs")
if ut: total_u += ut
if ot: total_o += ot
note = " [PyPy]" if "pypy" in name else ""
rows.append([short + note, fmt_secs(ut), fmt_secs(ot), delta_fmt(ut, ot), pct(ut, ot)])
print_table(
["Job", "Upstream", "Ours", "Delta", "% Change"],
rows,
["TOTAL (sum of all test steps)", fmt_secs(total_u), fmt_secs(total_o), delta_fmt(total_u, total_o), pct(total_u, total_o)],
)
# Table 3: by platform
print("\n\n### Table 3: Test Step Summary by Platform\n")
group_u: dict[str, int] = {}
group_o: dict[str, int] = {}
for name in all_names:
short = name.removeprefix("build (").removesuffix(")")
g = get_group(short)
group_u[g] = group_u.get(g, 0) + (upstream.get(name, {}).get("test_secs") or 0)
group_o[g] = group_o.get(g, 0) + (ours.get(name, {}).get("test_secs") or 0)
rows, gtot_u, gtot_o = [], 0, 0
for g in [*PLATFORM_GROUPS, "Other"]:
gu, go = group_u.get(g, 0), group_o.get(g, 0)
gtot_u += gu
gtot_o += go
rows.append([g, fmt_secs(gu), fmt_secs(go), delta_fmt(gu, go), pct(gu, go)])
print_table(
["Group", "Upstream (test)", "Ours (test)", "Saved", "%"],
rows,
["TOTAL", fmt_secs(gtot_u), fmt_secs(gtot_o), delta_fmt(gtot_u, gtot_o), pct(gtot_u, gtot_o)],
)
if __name__ == "__main__":
main() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 5-iteration default was borrowed from the Trio project, where it was determined empirically to handle PyPy's object resurrection behavior: on PyPy, objects like coroutines can survive GC rounds because executing their del can resurrect them.
On CPython, reference counting frees most objects immediately. One GC pass is sufficient to handle reference cycles, as confirmed by all test_unraisableexception tests passing (including the refcycle variants).
Use 1 pass on CPython and retain 5 on PyPy.