Skip to content

Logically merge cell and locals array. They are already contiguous in memory #87859

Closed
Closed
Logically merge cell and locals array. They are already contiguous in memory#87859
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

Description

@markshannon
markshannon
opened on Apr 1, 2021
BPO 43693
Nosy @gvanrossum, @rhettinger, @ncoghlan, @markshannon, @ericsnowcurrently, @serhiy-storchaka, @gvanrossum, @pablogsal
PRs
  • bpo-43693: Compute deref offsets in compiler #25152
  • bpo-43693: Group the code in codeobject.c logically. #26216
  • bpo-43693: Add _PyCode_New() and do some related cleanup. #26258
  • bpo-43693: Clean up the PyCodeObject fields. #26364
  • bpo-43693: Add _PyCode_New(). #26375
  • bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. #26388
  • bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. #26396
  • bpo-43693: Revert commits 2c1e2583fdc4db6b43d163239ea42b0e8394171f and b2bf2bc1ece673d387341e06c8d3c2bc6e259747  #26530
  • bpo-43693: Un-revert commits 2c1e258 and b2bf2bc. #26577
  • bpo-43693: Eliminate unused "fast locals". #26587
  • bpo-43693: Silence some compiler warnings. #26588
  • Revert "bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. (gh-26396)" #26597
  • bpo-43693: Un-revert commit f3fa63e. #26609
  • bpo-43693: Do not check co_cell2arg if a non-cell offset. #26626
  • bpo-43693: Turn localspluskinds into an object #26749
  • bpo-43693 Get rid of CO_NOFREE -- it's unused #26839
    		•

    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 = None
created_at = <Date 2021-04-01.11:33:50.457>
labels = ['interpreter-core', 'performance']
title = 'Logically merge cell and locals array. They are already contiguous in memory'
updated_at = <Date 2021-07-21.18:07:49.033>
user = 'https://github.com/markshannon'

    bugs.python.org fields:

    activity = <Date 2021-07-21.18:07:49.033>
actor = 'gvanrossum'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2021-04-01.11:33:50.457>
creator = 'Mark.Shannon'
dependencies = []
files = []
hgrepos = []
issue_num = 43693
keywords = ['patch']
message_count = 32.0
messages = ['389974', '390069', '390247', '390253', '394455', '394556', '395014', '395059', '395097', '395098', '395102', '395105', '395137', '395273', '395294', '395297', '395317', '395318', '395319', '395321', '395326', '395329', '395331', '395332', '395338', '395365', '395448', '395899', '396015', '396288', '396433', '397956']
nosy_count = 8.0
nosy_names = ['gvanrossum', 'rhettinger', 'ncoghlan', 'Mark.Shannon', 'eric.snow', 'serhiy.storchaka', 'Guido.van.Rossum', 'pablogsal']
pr_nums = ['25152', '26216', '26258', '26364', '26375', '26388', '26396', '26530', '26577', '26587', '26588', '26597', '26609', '26626', '26749', '26839']
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue43693'
versions = []

    Linked PRs

    Activity

    1. markshannon commented on Apr 1, 2021

      @markshannon
      markshannon
      on Apr 1, 2021
      MemberAuthor

      In the interpreter and compiler, the "fast" locals array and cells array are treated separately. By merging them in the compiler, the interpreter can be simplified a bit.

    2. markshannon
      self-assigned this
      on Apr 1, 2021
    3. markshannon
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      performancePerformance or resource usage
      on Apr 1, 2021
    4. markshannon
      self-assigned this
      on Apr 1, 2021
    5. markshannon
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      performancePerformance or resource usage
      on Apr 1, 2021

    6. rhettinger commented on Apr 2, 2021

      @rhettinger
      rhettinger
      on Apr 2, 2021
      Contributor

      This doesn't look simpler to me. We will forever lose mental clock cycles disentangling the locals and cells. So, I don't think they should be commingled.

    7. gvanrossum commented on Apr 5, 2021

      @gvanrossum
      gvanrossum
      on Apr 5, 2021
      Member

      To me it looks simpler though. The locals and cells are already stored in a single array, f_localsplus (which also contains the evaluation stack). There are various complications in ceval.c to translate cell indexes to indexes in this array (ein particular the extra local variable 'freevars', which weighs down the stack frame).

      Making the interpreter simpler by moving things to the compiler also makes it easier for the C compiler to optimize the code of the interpreter better.

    8. serhiy-storchaka commented on Apr 5, 2021

      @serhiy-storchaka
      serhiy-storchaka
      on Apr 5, 2021
      Member

      LGTM.

    9. markshannon commented on May 26, 2021

      @markshannon
      markshannon
      on May 26, 2021
      MemberAuthor

      New changeset 6cc800d by Eric Snow in branch 'main':
      bpo-43693: Clean up the PyCodeObject fields. (GH-26364)
      6cc800d

    10. ericsnowcurrently commented on May 27, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on May 27, 2021
      Member

      New changeset 9f494d4 by Eric Snow in branch 'main':
      bpo-43693: Add _PyCode_New(). (gh-26375)
      9f494d4

    11. ericsnowcurrently commented on Jun 3, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on Jun 3, 2021
      Member

      New changeset 2c1e258 by Eric Snow in branch 'main':
      bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. (gh-26388)
      2c1e258

    12. ericsnowcurrently commented on Jun 4, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on Jun 4, 2021
      Member

      New changeset b2bf2bc by Mark Shannon in branch 'main':
      bpo-43693: Compute deref offsets in compiler (gh-25152)
      b2bf2bc

    13. pablogsal commented on Jun 4, 2021

      @pablogsal
      pablogsal
      on Jun 4, 2021
      Member

      Unfortunately, commit 2c1e258 has broken all the refleak buildbots. Example failure:

      BISECTION:

      c1e2583fdc4db6b43d163239ea42b0e8394171f is the first bad commit
      commit 2c1e258
      Author: Eric Snow <ericsnowcurrently@gmail.com>
      Date: Thu Jun 3 10:28:27 2021 -0600

      bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. (gh-26388)

      https://buildbot.python.org/all/#/builders/384/builds/50/steps/5/logs/stdio

      As this is affecting all test, I am proceeding with a revert of commit 2c1e258 directly to not mask other issues.

    14. 18 remaining items

    15. ericsnowcurrently commented on Jun 15, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on Jun 15, 2021
      Member

      New changeset ac38a9f by Eric Snow in branch 'main':
      bpo-43693: Eliminate unused "fast locals". (gh-26587)
      ac38a9f

    16. markshannon
      assigned
      ericsnowcurrently
      and unassigned
      markshannon
      on Jun 17, 2021

    17. ericsnowcurrently commented on Jun 17, 2021

      @ericsnowcurrently
      ericsnowcurrently
      on Jun 17, 2021
      Member

      FWIW, I've wrapped up the key parts that I wanted to get done here
      (co_localplusnames/kinds, MAKE_CELL, eliminate unused fast local
      for arg cells). I'm leaving this open for now as there are a few
      things I didn't do that seem part of the original intention of this
      issue:

      • fully interleave the cells with the locals in their "natural" order
        (rather than only interleaving arg cells)
      • update the compiler to track names/kinds rather than computing
        them after the fact during the assembler step
        (this will allow us to remove a decent amount of code)
      • track the specific arg kinds in localspluskinds
        (this should allow us to make _PyEval_MakeFrameVector() simpler
        and a bit more efficient)

    18. gvanrossum commented on Jun 21, 2021

      @gvanrossum
      gvanrossum
      on Jun 21, 2021
      Member

      New changeset 355f5dd by Guido van Rossum in branch 'main':
      bpo-43693: Turn localspluskinds into an object (GH-26749)
      355f5dd

    19. gvanrossum commented on Jun 23, 2021

      @gvanrossum
      gvanrossum
      on Jun 23, 2021
      Member

      New changeset 769d7d0 by Guido van Rossum in branch 'main':
      bpo-43693 Get rid of CO_NOFREE -- it's unused (GH-26839)
      769d7d0

    20. ericsnowcurrently
      unassigned
      ericsnowcurrently
      on Jul 21, 2021

    21. gvanrossum commented on Jul 21, 2021

      @gvanrossum
      gvanrossum
      on Jul 21, 2021
      Member

      Another task seems to have appeared, Nick Coghlan would like the typedef to be renamed to PyLocal_VarKind, and the getter/setter similarly.

    22. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    23. markshannon commented on May 2, 2022

      @markshannon
      markshannon
      on May 2, 2022
      MemberAuthor

      I don't think there is anything left to do here, so closing.

    24. markshannon
      closed this as completedon May 2, 2022
    25. ericsnowcurrently
      added a commit that references this issue on Apr 29, 2025

      gh-87859: Track Code Object Local Kinds For Arguments (gh-132980)

      219d8d2
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      interpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions