-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Drop support for PyPy < 7.3.1 and clean up old PyPy workarounds #2456
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
Conversation
This is a peculiar piece of code... https://github.com/pybind/pybind11/blob/master/include/pybind11/pytypes.h#L730-L736 Add to that #996 |
5b0f9e2
to
dfb30f8
Compare
|
Good catch, @bstaletic! I was going to tell you your can just push to this branch if you want, but I guess you can't. I gave you permissions on my fork now, though. So feel free to... (Feel free to not do that as well, btw! I can get to it later this weekend.) |
Can't all maintainers edit your branch, since it's a PR here? Or did you uncheck that option? |
No, it's checked, but @bstaletic is (officially) not a maintainer. |
dfb30f8
to
8aa1682
Compare
PyPy (at least version 5.9) does not garbage collect objects when the | ||
interpreter exits. An alternative approach (which also works on CPython) is to use | ||
the :py:mod:`atexit` module [#f7]_, for example: | ||
PyPy does not garbage collect objects when the interpreter exits. An alternative |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to re-confirm, this, though. I did not do that, yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have tests for that. Some tests explicitly call gc.collect()
more than once because of PyPy, so it shouldn't be hard to verify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give it a quick try, but it's actually documented: https://doc.pypy.org/en/latest/cpython_differences.html#differences-related-to-garbage-collection-strategies
Last note: CPython tries to do a
gc.collect()
automatically when the program finishes; not PyPy. (It is possible in both CPython and PyPy to design a case where severalgc.collect()
are needed before all objects die. This makes CPython’s approach only work “most of the time” anyway.)
Should we add this to the docs, or are we fine as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I actually can't find any tests on the garbage collector passing by when the interpreter exits, though. So not sure how to apply @bstaletic's suggestion.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about calling PyGC_Collect()
? Would that work across the board?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good plan, but seems PyPy doesn't have PyGC_Collect
:-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than gc.collect()
, which isn't C, there's PyGC_Collect()
and a bunch of "didn't mean to make it public" functions that got removed in python 3.9. On the other hand PyGC_Collect()
was never documented.
Conclusion: CPython is a horrible mess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PyPy devs, struggling to keep up with massive amounts of C API every release, are the last ones you need to convince of that statement, I'm afraid ;-)
Thanks, @bstaletic! Fixed those extra PyPy reference, and rebased so that we can take advantage of #2462. |
Reminder to ourselves: #2436 (comment) |
8aa1682
to
10af980
Compare
10af980
to
9d346cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was included in Google-global testing: no issues found.
Commit by commit, let's test which parts can go and make tests succeed, and which preprocessor branches we'll need to keep.
Suggestions/commits welcome.
Closes #2436.