PEP 683: Add a PEP for immortal objects #2320
Conversation
|
You can take 683 |
ericsnowcurrently
changed the title
ericsnowcurrently
changed the title
| memory usage. Several enterprise Python users (e.g. Instagram, | ||
| YouTube) have taken advantage of this. However, the above |
There was a problem hiding this comment.
It might be useful have a citation/link for this.
There was a problem hiding this comment.
I don't really have any. However, it's fairly common knowledge on the core team.
There was a problem hiding this comment.
Yeah, I don't think we've made a public facing post about this, but I added this to Instagram and can confirm that we are still using this 🙂
|
@ericsnowcurrently please update |
|
@eduardo-elizondo, unless you have any objections, I'm going to merge this first thing tomorrow morning and post to python-dev. |
AA-Turner
changed the title
| Several promising mitigation strategies will be pursued in the effort | ||
| to bring it closer to performance-neutral. |
There was a problem hiding this comment.
Probably not needed to include here but these are four different strategies that I'll be pursuing:
- Immortalized interned strings (including Py_IDENTIFIERs)
- Immortalizing heap after startup
- Immortalizing module-level globals at import time
Also the following but it probably won't impact current benchmarks:
- GC improvements for large applications
Mental note to expand the benchmark suite to include something that overflows the last GC generation
There was a problem hiding this comment.
- Immortalized interned strings (including Py_IDENTIFIERs)
Note that I've replaced all core uses of _Py_IDENTIFIER() with direct access of statically allocated/initialized objects under _PyRuntimeState.global_objects.
pep-0683.rst
Outdated
|
|
||
| * (public) ``Py_REFCNT()`` | ||
| * (public) ``sys.getrefcount()`` | ||
| * (public) ``sys.gettotalrefcount()`` |
There was a problem hiding this comment.
This won't change! Or rather, it will change but not in the expected way.
This is something that I really looked into to make sure that the refleak tooling wouldn't break from this change. gettotalrefcount relies on _Py_RefTotal which is updated independently from the object's refcount field. Thus, an object's immortality will not make the value of gettotalrefcount be something really large.
Currently, I've placed the _Py_RefTotal update after the immortalization check. Thus, this value now reflects all the increfs/decrefs that did take place. If we move the check to before the immortalization check, it will now reflect all the attempts to incref and decrefs that took place (but they could have been a no-op). I chose the former since it's a bit unintuitive to have a returned value that's larger than the actual effective operations. This is enough to make the refcount leak tooling to work as is. However, if we choose the latter, it will work even in cases where we can upgrade a once mortal object to be immortal (which is beyond of the scope of any of the current changes in the PR). In this case, _Py_RefTotal will always be updated regardless of the the immortalization status of the instance, but the returned value of gettotalrefcount might make less sense as a whole.
The final consideration is that we need to make sure that _PySet_Dummy is not set to be immortal, otherwise, it will affect the implementation of _Py_GetRefTotal. Perhaps we should just remove the use of _PySet_Dummy and fix the gdb plugin in another way?
There was a problem hiding this comment.
I've mentioned that sys.gettotalrefcount() is unaffected but didn't include any of your explanation. If you think any of it would be helpful to PEP readers then we can add it in a follow-up.
There was a problem hiding this comment.
@ericsnowcurrently I've added a bunch of comments. Up to you if you think we need to update the PEP to reflect what I mention there. Otherwise, looks good to me!
|
@eduardo-elizondo Great! I'll make some updates and merge it. Thanks for the help. |
CAM-Gerlach
changed the title
|
Oops, too late on that change—glad I didn't start a full review. Just remember for the future, though—titles don't have periods in English, and neither do commit message summary lines (which is what the PR title maps to). |
| PEP: 683 | ||
| Title: Immortal Objects, Using a Fixed Refcount | ||
| Author: Eric Snow <ericsnowcurrently@gmail.com>, Eddie Elizondo <eduardo.elizondorueda@gmail.com> | ||
| Discussions-To: python-dev@python.org |
There was a problem hiding this comment.
Please update the PEP with a link to the actual thread once you create it, so it is easy for others to find without a lot of guessing and Googling.
Also, there are a smattering of relatively non-critical editing issues (aside from the title containing a comma, which is a bit irregular), but as this was merged just as I got to it, I'm a bit late to the party now so if needed, I can make a separate PR fixing those (though they aren't terribly critical).
I plan on merging/posting this as soon as I get a review from my co-author.