gh-95245: Document Py_TPFLAGS_PREHEADER

[sergey-miryanov]

I see following usages of Py_TPFLAGS_PREHEADER:

1. Calculate size of preheader (but preheader also contains GC info)
2. Check if types are support managed dict or managed weakref

I checked all linked PR from #95245 and can't find any explanation. I also checked https://github.com/python/cpython/blob/main/Objects/object_layout.md.

@encukou Please take a look.

• Issue: Reduce the size of plain Python objects from 8 to 6 machine words. #95245

[ZeroIntensity]

We should include documentation on the type object page too.

[sergey-miryanov]

@ZeroIntensity Thanks for review! Fixed (I hope :)

[ZeroIntensity]

I think that if this is internal-only, it would be better to completely drop it from public headers rather than document it as such.

[ZeroIntensity]

I think that if this is internal-only, it would be better to completely drop it from public headers rather than document it as such.

(Don't do this yet, by the way. Let's come to a consensus in the issue.)

[sergey-miryanov]

As no comments from @iritkatriel or @markshannon I propose to make it internal as @ZeroIntensity suggested and rename it to _Py_TPFLAGS_PREHEADER.

@encukou WDYT?

[encukou]

Per PEP-387, removing (or renaming) it probablyneeds a deprecation cycle. Or is there evidence that no existing code uses it?

[markshannon]

Since the tp_flags flags must be public, it seemed sensible to put Py_TPFLAGS_PREHEADER in the same header.
I have no objection to moving it.

It might be better to rename it to Py_TPFLAGS_VM_MANAGED_FIELDS to focus on the semantics, not the layout, and keep it public?

[sergey-miryanov]

Per PEP-387, removing (or renaming) it probablyneeds a deprecation cycle. Or is there evidence that no existing code uses it?

https://grep.app/search?q=Py_TPFLAGS_PREHEADER shows close to nothing of usage. Also, I'm downloading top-pypi packages to checks usage (as described here https://hugovk.dev/blog/2022/how-to-search-5000-python-projects/), will report as it finished.

[sergey-miryanov]

It might be better to rename it to Py_TPFLAGS_VM_MANAGED_FIELDS to focus on the semantics, not the layout, and keep it public?

@markshannon WDYT about wordings in this PR? Can you suggest some improvements?

[sergey-miryanov]

will report as it finished.

Searched over 13k+ packages:

➜ .\python.exe ../misc/cpython/search_pypi_top.py -q ..\top-pypi\output\ "Py_TPFLAGS_PREHEADER"
..\top-pypi\output\recordclass-0.23.1.tar.gz: recordclass-0.23.1/lib/recordclass/_dataobject.c: tp->tp_flags &= ~Py_TPFLAGS_PREHEADER;
Time: 0:01:10.586519
Found 1 matching lines in 1 projects
➜ ( Get-ChildItem ..\top-pypi\output\ | Measure-Object ).Count;
13961

[encukou]

It might be better to rename it to Py_TPFLAGS_VM_MANAGED_FIELDS to focus on the semantics, not the layout, and keep it public?

If it's public, it's best not to rename it.

[ZeroIntensity]

If it's public, it's best not to rename it.

I think it's probably safe to rename it, at least for 3.14. Based on a code search, we're the only ones who use it (all the non-CPython mentions are people copying our headers).

[sergey-miryanov]

If it's public, it's best not to rename it.

I think it's probably safe to rename it, at least for 3.14. Based on a code search, we're the only ones who use it (all the non-CPython mentions are people copying our headers).

Only recordclass uses it - #135861 (comment)

[sergey-miryanov]

@ZeroIntensity Please take a look.

[ZeroIntensity]

Thanks, this mostly looks good.

[ZeroIntensity]

LGTM.

I'm not a huge fan of documenting things that should be private, but I think it's too late to move this to internal headers or rename it. We should consider deprecating this in 3.15.

@encukou Would you like to finish up your review?

[encukou]

It's hard to deprecate a macro, so adding warnings in docs look best.
Thank you for sticking to this, @sergey-miryanov!

[sergey-miryanov]

Thanks everyone!