Skip to content
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

GH-125174: Make immortal objects more robust, following design from PEP 683 #125251

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Oct 10, 2024

@markshannon
Copy link
Member Author

This is not a complete fix for #125174, just enough to unblock other work

@markshannon markshannon added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Oct 10, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 94f8fd0 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Oct 10, 2024
@markshannon
Copy link
Member Author

The two "failing" refleak buildbots seem to be hanging in test.test_concurrent_futures.test_shutdown. There are no reported refleaks.

@markshannon
Copy link
Member Author

Performance impact:

Overall a small slowdown, with a large 7-10% slowdown on nbody.
This is almost certainly due to the regression in stack spilling around _Py_IsImmortal_Loose fixed here.

ARM 64 darwin
x86-64 linux

I'll re-run the linux benchmarks with the fix as a sanity check.

@markshannon
Copy link
Member Author

Windows shows an over 2% improvement. With the regression fixed, this I would expect this to be more than 3%.

This reason for this is that the MSVC does not convert unsigned int x; x > (1 << 31) into signed x; x < 0 as do clang and gcc.
This PR uses the signed x; x < 0 form. Hence the speedup.

@markshannon
Copy link
Member Author

x86-64 linux performance numbers are a bit noisy, but show no significant change.

@markshannon markshannon merged commit c901437 into python:main Oct 10, 2024
62 of 67 checks passed
@markshannon markshannon deleted the more-robust-immortality branch October 10, 2024 17:20
tacaswell added a commit to tacaswell/msgspec that referenced this pull request Oct 24, 2024
python/cpython#125251 renamed
_PY_IMMORTAL_REFCNT -> _PY_IMMORTAL_INITIAL_REFCNT
jcrist added a commit to jcrist/msgspec that referenced this pull request Oct 24, 2024
* MNT: account for CPython 314 changes

python/cpython#125251 renamed
_PY_IMMORTAL_REFCNT -> _PY_IMMORTAL_INITIAL_REFCNT

* Define new macro to reduce duplication

---------

Co-authored-by: Jim Crist-Harif <jcristharif@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants