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

[Docs] Possibly Add More Information About Immortal Objects #120426

Closed
ericsnowcurrently opened this issue Jun 12, 2024 · 8 comments
Closed

[Docs] Possibly Add More Information About Immortal Objects #120426

ericsnowcurrently opened this issue Jun 12, 2024 · 8 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes docs Documentation in the Doc dir topic-C-API

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jun 12, 2024

From @ncoghlan in python/peps#3817 (comment):

I'm wondering if it might be worth adding more detail on immortal objects to a new subheading in https://docs.python.org/3/c-api/refcounting.html though, as the best canonical doc reference I could find in the reference docs is to the term reference count which really just mentions immortal objects rather than explaining them. A glossary entry referencing that new subsection would also be helpful.

The two explanatory links in the C API docs go to the PEP, which is about to be marked as no longer to be trusted as living documentation.

Linked PRs

@ericsnowcurrently ericsnowcurrently added docs Documentation in the Doc dir topic-C-API 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Jun 12, 2024
@ericsnowcurrently
Copy link
Member Author

FWIW, I'm hesitant to make immortal objects more prominent in the docs. Presently they are an internal implementation detail (which daring users may use at their own risk). If anything, I'd rather they were less prominent in the docs. The only reason to mention them in the docs at all is for the benefit of people that notice objects with a really big refcount (which always stays the same).

All that would change, of course, if we were to expose immortal objects via public API--which PEP 683 purposefully did not do (other than the slight change in semantics for a few refcount operations). However, I'm not sure the existing internal API should ever be made public. It would be a genuine foot gun in its current unconstrained and unmanaged form, inviting substantial memory leaks. At best we might eventually choose to expose some more-carefully-crafted variant that's either better constrained in scope, less generic, or better managed by the runtime (or all of those). Until then, I think we should avoid drawing people's attention to the concept/term.

CC @eduardo-elizondo

@ncoghlan
Copy link
Contributor

It's mainly conceptual documentation that I was suggesting would be helpful, rather than getting into the exact mechanics.

At the very least, a glossary entry for other documentation like https://docs.python.org/3/library/sys.html#sys.getrefcount and https://docs.python.org/3/glossary.html#term-reference-count to point to rather than each mention having to provide its own short definition.

I don't think the actual definition needs to be particularly in depth, just give a bit more info about why immortal objects are a thing:

immortal object
  To reduce runtime overhead (especially in free-threaded builds and when sharing objects between
  subinterpreters), the CPython interpreter internally marks some objects (such as :const:`None`,
  :const:`True`, and :const:`False`) as "immortal", and excludes them from the interpreter's normal
  reference count tracking. This provides several practical benefits (as described in :pep:`683`) but
  also means the immortal objects do not report accurate reference count information at runtime.

  .. impl-detail::
     Interpreters other than CPython may not need or define immortal objects.

And then tweak the "historical document" notice on the PEP to make it clear that while it may no longer be an accurate description of the technical implementation details, it will indefinitely provide an accurate description of the benefits of including immortal objects in the CPython implementation.

@picnixz
Copy link
Contributor

picnixz commented Jun 17, 2024

I'm in favor of this definition, but would say "may not report" instead of "does not report".

@eduardo-elizondo
Copy link
Contributor

I agree with @ericsnowcurrently's proposal of keeping this mostly as an implementation detail since that was the original intent of the PEP. At the same time, @ncoghlan is very explicitly stating that it's an implementation detail so I wouldn't be opposed to that. I'll leave it to Eric to ultimately decide

@picnixz picnixz closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2024
@picnixz picnixz reopened this Aug 19, 2024
@picnixz
Copy link
Contributor

picnixz commented Aug 19, 2024

(Sorry, I closed the wrong issue)

@encukou
Copy link
Member

encukou commented Aug 19, 2024

I think we can all agree the current entry for “immortal” is not OK.

IMO, it should be kept in some form -- users do run into immortal objects and end up surprised. But it should be marked as a CPython implementation detail.

@encukou
Copy link
Member

encukou commented Aug 20, 2024

When wording this, I wanted to add something like the following, to answer an obvious question users might have:

There is no mechanism to make arbitrary objects immortal.

But, I see that for free-threading builds, Py_SET_REFCNT is documented as a way for users to immortalize objects.

@colesbury, is that intended to be a public API contract? Is _Py_SetImmortal intended to immortalize any arbitrary object? I thought that we do not provide this, and I've changed _Py_SetImmortal to reject strings (which need to go through _PyUnicode_InternImmortal to set some extra state).

I'd expect a note for Py_SET_REFCNT to be something like:

Setting the reference count to something other than the total number of references may lead to memory leaks or crashes. In particular, Py_SET_REFCNT is not a safe way to make an object immortal.

cc @vstinner

@encukou
Copy link
Member

encukou commented Aug 26, 2024

I sent a PR without a “no mechanism” line. I plan to merge it ~ Friday if there are no objections.

encukou added a commit that referenced this issue Aug 27, 2024
Reword the glossary term "immortal", mark it as an implementation detail
encukou added a commit to encukou/cpython that referenced this issue Aug 30, 2024
Reword the glossary term "immortal", mark it as an implementation detail

(cherry picked from commit 6754566)
Yhg1s pushed a commit that referenced this issue Sep 2, 2024
…23491)

gh-120426: Reword the glossary term "immortal" (GH-123191)

Reword the glossary term "immortal", mark it as an implementation detail

(cherry picked from commit 6754566)
encukou added a commit to encukou/cpython that referenced this issue Sep 3, 2024
…3191) (python#123491)

Add the glossary term "immortal", mark it as an implementation detail

(cherry picked from commit 6754566)
(cherry picked from commit 1af74fa)

(Unlike the original commits, this adds the entire entry.)
ambv pushed a commit that referenced this issue Sep 6, 2024
…3491) (#123636)

Add the glossary term "immortal", mark it as an implementation detail

(cherry picked from commit 6754566)
(cherry picked from commit 1af74fa)

(Unlike the original commits, this adds the entire entry.)
@ambv ambv closed this as completed Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes docs Documentation in the Doc dir topic-C-API
Projects
None yet
Development

No branches or pull requests

6 participants