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

[subinterpreters] Make the type attribute lookup cache per-interpreter #86911

Closed
vstinner opened this issue Dec 25, 2020 · 5 comments
Closed
Labels
3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters

Comments

@vstinner
Copy link
Member

BPO 42745
Nosy @vstinner, @pablogsal
PRs
  • bpo-42745: Make the type cache per-interpreter #23947
  • bpo-42745: finalize_interp_types() calls _PyType_Fini() #23953
  • 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 = None
    closed_at = <Date 2020-12-26.00:46:02.836>
    created_at = <Date 2020-12-25.22:50:34.708>
    labels = ['expert-subinterpreters', 'interpreter-core', '3.10']
    title = '[subinterpreters] Make the type attribute lookup cache per-interpreter'
    updated_at = <Date 2020-12-26.19:26:19.730>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-12-26.19:26:19.730>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-12-26.00:46:02.836>
    closer = 'vstinner'
    components = ['Interpreter Core', 'Subinterpreters']
    creation = <Date 2020-12-25.22:50:34.708>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 42745
    keywords = ['patch']
    message_count = 5.0
    messages = ['383777', '383786', '383816', '383820', '383822']
    nosy_count = 2.0
    nosy_names = ['vstinner', 'pablogsal']
    pr_nums = ['23947', '23953']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue42745'
    versions = ['Python 3.10']

    @vstinner
    Copy link
    Member Author

    Currently, the type lookup cache is shared by all interpreter which causes multiple issues:

    • The version tag is currently protected by the GIL, but it would require a new lock if the GIL is made per interpreter (bpo-40512)

    • Clearing the cache in an interpreter clears the cache in all interpreters

    • The cache has a fixed size of 4096 entries. The cache misses increase with the number of interpreters, since each interpreter has its own types.

    I propose to make the type lookup cache per interpreter.

    @vstinner vstinner added 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters labels Dec 25, 2020
    @vstinner vstinner changed the title [subinterpreters] Make the type lookup cache per-interpreter [subinterpreters] Make the type attribute lookup cache per-interpreter Dec 25, 2020
    @vstinner vstinner changed the title [subinterpreters] Make the type lookup cache per-interpreter [subinterpreters] Make the type attribute lookup cache per-interpreter Dec 25, 2020
    @vstinner
    Copy link
    Member Author

    New changeset 4101018 by Victor Stinner in branch 'master':
    bpo-42745: Make the type cache per-interpreter (GH-23947)
    4101018

    @pablogsal
    Copy link
    Member

    4101018 introduced some reference leaks:

    https://buildbot.python.org/all/#/builders/384/builds/136

    4101018 is the first bad commit
    commit 4101018
    Author: Victor Stinner <vstinner@python.org>
    Date: Sat Dec 26 01:45:43 2020 +0100

    bpo-42745: Make the type cache per-interpreter (GH-23947)
    
    Make the type attribute lookup cache per-interpreter.
    
    Add private _PyType_InitCache() function, called by PyInterpreterState_New().
    
    Continue to share next_version_tag between interpreters, since static
    types are still shared by interpreters.
    
    Remove MCACHE macro: the cache is no longer disabled if the
    EXPERIMENTAL_ISOLATED_SUBINTERPRETERS macro is defined.
    

    Include/internal/pycore_interp.h | 22 +++
    Include/internal/pycore_object.h | 3 +
    Include/internal/pycore_pylifecycle.h | 2 +-
    .../2020-12-25-23-30-58.bpo-42745.XsFoHS.rst | 1 +
    Objects/typeobject.c | 178 ++++++++++++---------
    Python/pylifecycle.c | 2 +-
    Python/pystate.c | 2 +
    7 files changed, 128 insertions(+), 82 deletions(-)
    create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-12-25-23-30-58.bpo-42745.XsFoHS.rst
    bisect run success

    I assume PR 23953 fixes those? If that is so, could we land it before more buildbots start to fail?

    @vstinner
    Copy link
    Member Author

    I assume PR 23953 fixes those?

    Yes, it fix the 7 tests which leak:

    ./python -m test -R 3:3 -j0 test__xxsubinterpreters test_ast test_atexit test_capi test_interpreters test_threading

    @vstinner
    Copy link
    Member Author

    New changeset f450723 by Victor Stinner in branch 'master':
    bpo-42745: finalize_interp_types() calls _PyType_Fini() (GH-23953)
    f450723

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

    No branches or pull requests

    2 participants