Skip to content

Generator objects are defined using PyObject_HEAD rather than PyObject_VAR_HEAD #126596

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

Open
jbower-fb opened this issue Nov 8, 2024 · 4 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@jbower-fb
Copy link
Contributor

jbower-fb commented Nov 8, 2024

Bug report

Bug description:

#29891 converted the generator objects to be variable in size so they could incorporate interpreter frames. However, the object layout is still defined using PyObject_HEAD. Technically I think this is incorrect. In practice probably nobody notices because the generator type is not a base class and there are other ways to work out the variable sized part where needed, so ob_size is never read after it is written as part of make_gen().

Long story short, I want to fix this because I would like to use ob_size to find the size of generator objects as part of work on Meta's JIT.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@jbower-fb jbower-fb added the type-bug An unexpected behavior, bug, or error label Nov 8, 2024
@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 8, 2024
@jbower-fb
Copy link
Contributor Author

cc @markshannon who authored #29891.

@markshannon
Copy link
Member

The PyVarObject docs state that "This is only used for objects that have some notion of length".
Code objects have no notion of length.

What advantage would there be to changing to PyVarObject?
It wouldn't provide any new capabilities, but it would use an additional 8 bytes per generator.

@jbower-fb
Copy link
Contributor Author

jbower-fb commented Dec 6, 2024

Primarily it would allow me to stuff JIT-related data at the end of the generator object and not have to a bunch of work each time to calculate where that starts. I do appreciate that may not be so as useful to anyone else though so I'll see if I can find another way to mitigate the cost.

I am still left wondering though, if these objects don't have a notion of length then why don't we set tp_itemsize to 0 in the generator types?

cc @DinoV @markshannon

@jbower-fb jbower-fb reopened this Dec 6, 2024
@markshannon
Copy link
Member

why don't we set tp_itemsize to 0 in the generator types?

Because tp_itemsize is to tell the allocation function(s) how big the object is. Everything has a size, not everything has a length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants