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

Statically allocate interpreter states as much as possible. #90111

Open
ericsnowcurrently opened this issue Dec 1, 2021 · 19 comments
Open

Statically allocate interpreter states as much as possible. #90111

ericsnowcurrently opened this issue Dec 1, 2021 · 19 comments
Assignees
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Dec 1, 2021

BPO 45953
Nosy @gvanrossum, @vstinner, @markshannon, @ericsnowcurrently, @JulienPalard, @miss-islington, @brandtbucher
PRs
  • bpo-45953: Statically allocate the main interpreter (and initial thread state). #29883
  • bpo-45953: Statically initialize the small ints. #30092
  • bpo-45953: Statically allocate and initialize global bytes objects. #30096
  • bpo-45953: Statically initialize all the _PyRuntimeState fields we can. #30588
  • bpo-45953: Statically initialize all the non-object PyInterpreterState fields we can. #30589
  • bpo-45953: Statically initialize all the PyThreadState fields we can. #30590
  • bpo-45953: Preserve backward compatibility on some PyThreadState field names. #31038
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ericsnowcurrently'
    closed_at = None
    created_at = <Date 2021-12-01.18:50:39.120>
    labels = ['interpreter-core', '3.11']
    title = 'Statically allocate interpreter states as much as possible.'
    updated_at = <Date 2022-03-29.07:04:09.685>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2022-03-29.07:04:09.685>
    actor = 'mdk'
    assignee = 'eric.snow'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2021-12-01.18:50:39.120>
    creator = 'eric.snow'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45953
    keywords = ['patch']
    message_count = 15.0
    messages = ['407476', '408507', '410315', '410441', '410536', '410537', '412046', '412213', '412291', '416159', '416161', '416170', '416208', '416210', '416245']
    nosy_count = 7.0
    nosy_names = ['gvanrossum', 'vstinner', 'Mark.Shannon', 'eric.snow', 'mdk', 'miss-islington', 'brandtbucher']
    pr_nums = ['29883', '30092', '30096', '30588', '30589', '30590', '31038']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue45953'
    versions = ['Python 3.11']

    Linked PRs

    @ericsnowcurrently
    Copy link
    Member Author

    ericsnowcurrently commented Dec 1, 2021

    Currently we allocate each new PyInterpreterState in PyInterpreterState_New(). Furthermore, PyInterpreterState is full of pointers which are almost all allocated on the heap during runtime init. We can statically allocate (and initialize?) most of what goes into PyInterpreterState, as well as the main interpreter itself (as part of _PyRuntimeState). This includes each interpreter's initial PyThreadState.

    TODO:

    • add PyInterpreterState main; to _PyRuntimeState.interpreters and use it
    • change references from the pointer to the value
    • add PyThreadState _main; to PyInterpreterState.threads and use it
    • change references from the pointer to the value
    • change PyInterpreterState pointer fields to values (as much as possible)
    • change PyThreadState pointer fields to values (as much as possible)

    benefits:

    • fewer possible failures (no memory) during runtime/interpreter/thread init
    • improved memory locality for pointers looked up relative to interpreter/thread state

    There is one non-trivial bit: embedding the various PyObject values in PyInterpreterState and PyThreadState means hard-coding the various pieces of the object there (e.g. for dict, its keys/values; for list, its array), as well as adding necessary init code to PyInterpreterState_New() and PyThreadState_New(). The resulting added complexity can be mitigated somewhat with macros or even code generation. (In fact, there is probably significant overlap with Guido's deepfreeze tool.) Regardless, we'll probably need to factor out init funcs for a number of object types, where currently there are only "Py*_New()" funcs that combine allocation and init.

    @ericsnowcurrently ericsnowcurrently added the 3.11 only security fixes label Dec 1, 2021
    @ericsnowcurrently ericsnowcurrently self-assigned this Dec 1, 2021
    @ericsnowcurrently ericsnowcurrently added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.11 only security fixes labels Dec 1, 2021
    @ericsnowcurrently ericsnowcurrently self-assigned this Dec 1, 2021
    @ericsnowcurrently ericsnowcurrently added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Dec 1, 2021
    @ericsnowcurrently
    Copy link
    Member Author

    New changeset 121f1f8 by Eric Snow in branch 'main':
    bpo-45953: Statically initialize the small ints. (gh-30092)
    121f1f8

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset cf496d6 by Eric Snow in branch 'main':
    bpo-45953: Statically allocate and initialize global bytes objects. (gh-30096)
    cf496d6

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset ed57b36 by Eric Snow in branch 'main':
    bpo-45953: Statically allocate the main interpreter (and initial thread state). (gh-29883)
    ed57b36

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset 324908b by Eric Snow in branch 'main':
    bpo-45953: Statically initialize all the PyThreadState fields we can. (gh-30590)
    324908b

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset 322f962 by Eric Snow in branch 'main':
    bpo-45953: Statically initialize all the non-object PyInterpreterState fields we can. (gh-30589)
    322f962

    @brandtbucher
    Copy link
    Member

    Any chance we could revert the recent renaming of tstate.exc_state and tstate.root_cframe in #30590? It broke Greenlet again:

    python-greenlet/greenlet#288

    If it's only a name change (and the members themselves are the same), I think reverting it is preferable to burying Greenlet in more compatibility macros and bugging them to put out another new release.

    @ericsnowcurrently
    Copy link
    Member Author

    Any chance we could revert the recent renaming of tstate.exc_state and tstate.root_cframe

    Yeah, I'll sort this out. Sorry for that.

    @miss-islington
    Copy link
    Contributor

    New changeset f78be59 by Eric Snow in branch 'main':
    bpo-45953: Preserve backward compatibility on some PyThreadState field names. (GH-31038)
    f78be59

    @JulienPalard
    Copy link
    Member

    Since 121f1f8, sys.getrefcount(1) is surprising:

        >>> __import__("sys").getrefcount(1)
        1000000210

    Should sys.getrefcount try to "fix" the value like by returning PyREFCNT(object) % 999999999? (But using a define to avoid the "magic, copy/pasted value").

    @vstinner
    Copy link
    Member

    Should sys.getrefcount try to "fix" the value (...)

    https://peps.python.org/pep-0683/ would make it possible. Right now, I don't think that it's possible.

    Right now, a refcount of 1000000210 can be a real value, or it can be an immortal object.

    @gvanrossum
    Copy link
    Member

    Please don’t try to “fix” anything. The value is only useful if you
    understand the implementation. It should map straightforwardly to what’s in
    memory.

    On Mon, Mar 28, 2022 at 05:16 STINNER Victor <report@bugs.python.org> wrote:

    STINNER Victor <vstinner@python.org> added the comment:

    > Should sys.getrefcount try to "fix" the value (...)

    https://peps.python.org/pep-0683/ would make it possible. Right now, I
    don't think that it's possible.

    Right now, a refcount of 1000000210 can be a real value, or it can be an
    immortal object.

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue45953\>


    --
    --Guido (mobile)

    @JulienPalard
    Copy link
    Member

    Hum, and why 999999999? I am probably missing something obvious but 1 should be enough to ensure the value never hits 0. Except for refcount bugs obviously, but I don't think this is the right reason?

    @gvanrossum
    Copy link
    Member

    I used 999999999 in deepfreeze.py to signify "immortal object". It has been copied by others (small integers are essentially immortal too). I wasn't too sure that the refcount wouldn't go below zero if the interpreter is repeatedly finalized and reinitialized. Once we have official immortal objects (see PEP-683) we should switch to that.

    Since you seem to be challenging the value of 999999999, my question to you is, why do you care what the refcount of 1 is?

    @JulienPalard
    Copy link
    Member

    Since you seem to be challenging the value of 999999999, my question to you is, why do you care what the refcount of 1 is?

    Yesterday I was teaching Python, and we were speaking of integer immutability, names being "labels to objects" and so on, and I was showing the memory layout of all of this by hand on a whiteboard while "prooving" my drawings using an interpreter.

    While doing so came a question like "So, many modules can use the object int(1)?" So I answered yes, told that I expected many reuse of 1, and went importing sys.getrefcount to show them.

    And boom, it printed 1000000209 so I bugged for a few seconds, the value was obviously not the real refcount, and was also obviously bumped by a constant like 100000000, so I went inspecting why and found this commit.

    I have nothing against keeping 999999999, but in the other hand it could surprise other people, maybe we should at least document it near sys.getrefcount.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @kumaraditya303
    Copy link
    Contributor

    This looks completed, @ericsnowcurrently can this be closed now?

    @ericsnowcurrently
    Copy link
    Member Author

    We should add a note in the docs, as @JulienPalard recommended. Arguably, that should be part of #98154.

    Otherwise, I'll take a look soon to see if there's anything else left to do.

    ericsnowcurrently added a commit that referenced this issue Dec 14, 2022
    * move _PyRuntime.global_objects.interned to _PyRuntime.cached_objects.interned_strings (and use _Py_CACHED_OBJECT())
    * rename _PyRuntime.global_objects to _PyRuntime.static_objects
    
    (This also relates to gh-96075.)
    
    #90111
    carljm added a commit to carljm/cpython that referenced this issue Dec 16, 2022
    * main:
      Improve stats presentation for calls. (pythonGH-100274)
      Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295)
      pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277)
      Document that zipfile's pwd parameter is a `bytes` object (python#100209)
      pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271)
      Fix typo in introduction.rst (python#100266)
      pythongh-78997: AttributeError if loading fails in LibraryLoader.__getattr__
      pythonGH-100234: Set a default value for random.expovariate() (pythonGH-100235)
      Remove uninformative itertools recipe (pythonGH-100253)
      pythonGH-99767: update PyTypeObject docs for type watchers (pythonGH-99928)
      Move stats for the method cache into the `Py_STAT` machinery (pythonGH-100255)
      pythonGH-100222: fix typo _py_set_opocde -> _py_set_opcode (pythonGH-100259)
      pythonGH-100000: Cleanup and polish various watchers code (pythonGH-99998)
      pythongh-90111: Minor Cleanup for Runtime-Global Objects (pythongh-100254)
    @arnaudlegout
    Copy link

    Following @JulienPalard comments, I also faced the same issue during a Python course. Whatever is the meaning of refcount, the documentation of sys.getrefcount should be fixed. The doc currently says that

    Return the reference count of object.
    
    The count returned is generally one higher than you might expect,
    because it includes the (temporary) reference as an argument to
    getrefcount().
    

    For immortal objects, it is no more the case, the returned number is not the reference count.

    @ericsnowcurrently
    Copy link
    Member Author

    See gh-98154.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants