Skip to content

/ cpython

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

Jump to bottom

bpo-44530: Add co_qualname field to PyCodeObject #26941

Merged
merged 5 commits into from Jul 7, 2021
Merged

bpo-44530: Add co_qualname field to PyCodeObject #26941

merged 5 commits into from Jul 7, 2021
+7,448 −7,435

Conversation

@P403n1x87
Copy link
Contributor

@P403n1x87 P403n1x87 commented Jun 28, 2021 •
edited

https://bugs.python.org/issue44530

Sample flame graph for the script

class Foo:
    def on_cpu(self, n):
        a = []
        for i in range(n):
            a.append(i)


class Bar:
    def on_cpu(self, n):
        a = []
        for i in range(n):
            a.append(i)


if __name__ == "__main__":
    f = Foo()
    b = Bar()

    f.on_cpu(1_000_000)
    b.on_cpu(5_000_000)

Python 3.9

py3_9

With proposed change

py_main
@P403n1x87 P403n1x87 requested review from markshannon and tiran as code owners Jun 28, 2021
@P403n1x87
feat(code): add co_qualname field
008cb5a
@P403n1x87 P403n1x87 force-pushed the P403n1x87:bpo-44530-code-qualname branch from 4548084 to 008cb5a Jun 28, 2021
@pablogsal
pablogsal requested changes Jun 29, 2021
View changes
Copy link
Member

@pablogsal pablogsal left a comment

Thanks for opening a PR @P403n1x87. Let's discuss this on the bpo issue, but at the very least this needs to:

  • Be exposed to Python side
  • Have tests
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jun 29, 2021

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
@P403n1x87
expose co_qualname to Python
a0252ab
@P403n1x87 P403n1x87 requested a review from 1st1 as a code owner Jul 4, 2021
@P403n1x87
P403n1x87 reviewed Jul 4, 2021
View changes
Copy link
Contributor Author

@P403n1x87 P403n1x87 left a comment

Just a couple of questions for the reviewers
Objects/codeobject.c Show resolved Hide resolved
Doc/c-api/function.rst Show resolved Hide resolved
@P403n1x87
Copy link
Contributor Author

@P403n1x87 P403n1x87 commented Jul 4, 2021

I have made the requested changes; please review again
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 4, 2021

Thanks for making the requested changes!

@pablogsal: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from pablogsal Jul 4, 2021
@pablogsal
pablogsal reviewed Jul 4, 2021
View changes
Include/funcobject.h Outdated
@@ -55,7 +55,6 @@ PyAPI_DATA(PyTypeObject) PyFunction_Type;
#define PyFunction_Check(op) Py_IS_TYPE(op, &PyFunction_Type)

PyAPI_FUNC(PyObject *) PyFunction_New(PyObject *, PyObject *);
PyAPI_FUNC(PyObject *) PyFunction_NewWithQualName(PyObject *, PyObject *, PyObject *);

This comment has been minimized.

Sign in to view
@pablogsal

pablogsal Jul 4, 2021
Member

Unfortunately removing this is not allowed per PEP 387

This comment has been minimized.

Sign in to view
@P403n1x87

P403n1x87 Jul 4, 2021
Author Contributor

👍 I have now restored it.
@P403n1x87
use co_qualname in function constructor
3f2c9e9 
The MAKE_FUNCTION opcode now needs to pop one less value from the TOS.
@P403n1x87 P403n1x87 force-pushed the P403n1x87:bpo-44530-code-qualname branch from 7a12d31 to 3f2c9e9 Jul 4, 2021
@P403n1x87
P403n1x87 reviewed Jul 4, 2021
View changes
Python/compile.c
@@ -7243,7 +7242,6 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist,
PyObject *consts = NULL;
PyObject *localsplusnames = NULL;
PyObject *localspluskinds = NULL;
PyObject *name = NULL;

This comment has been minimized.

Sign in to view
@P403n1x87

P403n1x87 Jul 4, 2021
Author Contributor

This was unused but was not reported by the compiler, probably because of the Py_XDECREF(name) below
@P403n1x87 P403n1x87 requested a review from pablogsal Jul 4, 2021
@pablogsal
pablogsal reviewed Jul 4, 2021
View changes
Misc/NEWS.d/next/C API/2021-06-28-23-44-47.bpo-44530.qij7YC.rst Outdated Show resolved Hide resolved
@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jul 4, 2021

Seems that you need to rebase and run make regen-all
@P403n1x87
Merge branch 'main' into bpo-44530-code-qualname
d5ec309
@P403n1x87
Copy link
Contributor Author

@P403n1x87 P403n1x87 commented Jul 4, 2021

Seems that you need to rebase and run make regen-all

yup, done! 🙂
@P403n1x87 P403n1x87 requested a review from pablogsal Jul 4, 2021
@pablogsal
pablogsal approved these changes Jul 5, 2021
View changes
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 5, 2021

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit d5ec309 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.
@pablogsal
pablogsal requested changes Jul 5, 2021
View changes
Copy link
Member

@pablogsal pablogsal left a comment •
edited

Hummm, seems that the Windows buildbots are failing with this PR:

======================================================================
ERROR: test_gprof (test.test_tools.test_gprof2html.Gprof2htmlTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\buildbot.python.org\pull_request.kloth-win64\build\lib\test\test_tools\test_gprof2html.py", line 16, in setUp
    self.gprof = import_tool('gprof2html')
                 ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\buildbot.python.org\pull_request.kloth-win64\build\lib\test\test_tools\__init__.py", line 35, in import_tool
    return importlib.import_module(toolname)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\buildbot.python.org\pull_request.kloth-win64\build\lib\importlib\__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 889, in exec_module
  File "<frozen importlib._bootstrap_external>", line 1022, in get_code
  File "<frozen importlib._bootstrap_external>", line 682, in _compile_bytecode
ValueError: bad marshal data (unknown type code)

Link to the builds:

https://buildbot.python.org/all/#/builders/577/builds/83
https://buildbot.python.org/all/#/builders/593/builds/87
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 5, 2021

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
@P403n1x87
Copy link
Contributor Author

@P403n1x87 P403n1x87 commented Jul 5, 2021

Hummm, seems that the Windows buildbots are failing with this PR:

Hmm is this related to the output of make regen-all? Is there anything special that needs to be regenerated for Windows?
@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jul 5, 2021 •
edited

Hmm is this related to the output of make regen-all? Is there anything special that needs to be regenerated for Windows?

Ah, the problem is that we are missing bumping the magic number to avoid picking up old pyc files:

cpython/Lib/importlib/_bootstrap_external.py

Lines 212 to 374 in f64de53

# Magic word to reject .pyc files generated by other Python versions.
# It should change for each incompatible change to the bytecode.
#
# The value of CR and LF is incorporated so if you ever read or write
# a .pyc file in text mode the magic number will be wrong; also, the
# Apple MPW compiler swaps their values, botching string constants.
#
# There were a variety of old schemes for setting the magic number.
# The current working scheme is to increment the previous value by
# 10.
#
# Starting with the adoption of PEP 3147 in Python 3.2, every bump in magic
# number also includes a new "magic tag", i.e. a human readable string used
# to represent the magic number in __pycache__ directories. When you change
# the magic number, you must also set a new unique magic tag. Generally this
# can be named after the Python major version of the magic number bump, but
# it can really be anything, as long as it's different than anything else
# that's come before. The tags are included in the following table, starting
# with Python 3.2a0.
#
# Known values:
# Python 1.5: 20121
# Python 1.5.1: 20121
# Python 1.5.2: 20121
# Python 1.6: 50428
# Python 2.0: 50823
# Python 2.0.1: 50823
# Python 2.1: 60202
# Python 2.1.1: 60202
# Python 2.1.2: 60202
# Python 2.2: 60717
# Python 2.3a0: 62011
# Python 2.3a0: 62021
# Python 2.3a0: 62011 (!)
# Python 2.4a0: 62041
# Python 2.4a3: 62051
# Python 2.4b1: 62061
# Python 2.5a0: 62071
# Python 2.5a0: 62081 (ast-branch)
# Python 2.5a0: 62091 (with)
# Python 2.5a0: 62092 (changed WITH_CLEANUP opcode)
# Python 2.5b3: 62101 (fix wrong code: for x, in ...)
# Python 2.5b3: 62111 (fix wrong code: x += yield)
# Python 2.5c1: 62121 (fix wrong lnotab with for loops and
# storing constants that should have been removed)
# Python 2.5c2: 62131 (fix wrong code: for x, in ... in listcomp/genexp)
# Python 2.6a0: 62151 (peephole optimizations and STORE_MAP opcode)
# Python 2.6a1: 62161 (WITH_CLEANUP optimization)
# Python 2.7a0: 62171 (optimize list comprehensions/change LIST_APPEND)
# Python 2.7a0: 62181 (optimize conditional branches:
# introduce POP_JUMP_IF_FALSE and POP_JUMP_IF_TRUE)
# Python 2.7a0 62191 (introduce SETUP_WITH)
# Python 2.7a0 62201 (introduce BUILD_SET)
# Python 2.7a0 62211 (introduce MAP_ADD and SET_ADD)
# Python 3000: 3000
# 3010 (removed UNARY_CONVERT)
# 3020 (added BUILD_SET)
# 3030 (added keyword-only parameters)
# 3040 (added signature annotations)
# 3050 (print becomes a function)
# 3060 (PEP 3115 metaclass syntax)
# 3061 (string literals become unicode)
# 3071 (PEP 3109 raise changes)
# 3081 (PEP 3137 make __file__ and __name__ unicode)
# 3091 (kill str8 interning)
# 3101 (merge from 2.6a0, see 62151)
# 3103 (__file__ points to source file)
# Python 3.0a4: 3111 (WITH_CLEANUP optimization).
# Python 3.0b1: 3131 (lexical exception stacking, including POP_EXCEPT
#3021)
# Python 3.1a1: 3141 (optimize list, set and dict comprehensions:
# change LIST_APPEND and SET_ADD, add MAP_ADD #2183)
# Python 3.1a1: 3151 (optimize conditional branches:
# introduce POP_JUMP_IF_FALSE and POP_JUMP_IF_TRUE
#4715)
# Python 3.2a1: 3160 (add SETUP_WITH #6101)
# tag: cpython-32
# Python 3.2a2: 3170 (add DUP_TOP_TWO, remove DUP_TOPX and ROT_FOUR #9225)
# tag: cpython-32
# Python 3.2a3 3180 (add DELETE_DEREF #4617)
# Python 3.3a1 3190 (__class__ super closure changed)
# Python 3.3a1 3200 (PEP 3155 __qualname__ added #13448)
# Python 3.3a1 3210 (added size modulo 2**32 to the pyc header #13645)
# Python 3.3a2 3220 (changed PEP 380 implementation #14230)
# Python 3.3a4 3230 (revert changes to implicit __class__ closure #14857)
# Python 3.4a1 3250 (evaluate positional default arguments before
# keyword-only defaults #16967)
# Python 3.4a1 3260 (add LOAD_CLASSDEREF; allow locals of class to override
# free vars #17853)
# Python 3.4a1 3270 (various tweaks to the __class__ closure #12370)
# Python 3.4a1 3280 (remove implicit class argument)
# Python 3.4a4 3290 (changes to __qualname__ computation #19301)
# Python 3.4a4 3300 (more changes to __qualname__ computation #19301)
# Python 3.4rc2 3310 (alter __qualname__ computation #20625)
# Python 3.5a1 3320 (PEP 465: Matrix multiplication operator #21176)
# Python 3.5b1 3330 (PEP 448: Additional Unpacking Generalizations #2292)
# Python 3.5b2 3340 (fix dictionary display evaluation order #11205)
# Python 3.5b3 3350 (add GET_YIELD_FROM_ITER opcode #24400)
# Python 3.5.2 3351 (fix BUILD_MAP_UNPACK_WITH_CALL opcode #27286)
# Python 3.6a0 3360 (add FORMAT_VALUE opcode #25483)
# Python 3.6a1 3361 (lineno delta of code.co_lnotab becomes signed #26107)
# Python 3.6a2 3370 (16 bit wordcode #26647)
# Python 3.6a2 3371 (add BUILD_CONST_KEY_MAP opcode #27140)
# Python 3.6a2 3372 (MAKE_FUNCTION simplification, remove MAKE_CLOSURE
# #27095)
# Python 3.6b1 3373 (add BUILD_STRING opcode #27078)
# Python 3.6b1 3375 (add SETUP_ANNOTATIONS and STORE_ANNOTATION opcodes
# #27985)
# Python 3.6b1 3376 (simplify CALL_FUNCTIONs & BUILD_MAP_UNPACK_WITH_CALL
#27213)
# Python 3.6b1 3377 (set __class__ cell from type.__new__ #23722)
# Python 3.6b2 3378 (add BUILD_TUPLE_UNPACK_WITH_CALL #28257)
# Python 3.6rc1 3379 (more thorough __class__ validation #23722)
# Python 3.7a1 3390 (add LOAD_METHOD and CALL_METHOD opcodes #26110)
# Python 3.7a2 3391 (update GET_AITER #31709)
# Python 3.7a4 3392 (PEP 552: Deterministic pycs #31650)
# Python 3.7b1 3393 (remove STORE_ANNOTATION opcode #32550)
# Python 3.7b5 3394 (restored docstring as the first stmt in the body;
# this might affected the first line number #32911)
# Python 3.8a1 3400 (move frame block handling to compiler #17611)
# Python 3.8a1 3401 (add END_ASYNC_FOR #33041)
# Python 3.8a1 3410 (PEP570 Python Positional-Only Parameters #36540)
# Python 3.8b2 3411 (Reverse evaluation order of key: value in dict
# comprehensions #35224)
# Python 3.8b2 3412 (Swap the position of positional args and positional
# only args in ast.arguments #37593)
# Python 3.8b4 3413 (Fix "break" and "continue" in "finally" #37830)
# Python 3.9a0 3420 (add LOAD_ASSERTION_ERROR #34880)
# Python 3.9a0 3421 (simplified bytecode for with blocks #32949)
# Python 3.9a0 3422 (remove BEGIN_FINALLY, END_FINALLY, CALL_FINALLY, POP_FINALLY bytecodes #33387)
# Python 3.9a2 3423 (add IS_OP, CONTAINS_OP and JUMP_IF_NOT_EXC_MATCH bytecodes #39156)
# Python 3.9a2 3424 (simplify bytecodes for *value unpacking)
# Python 3.9a2 3425 (simplify bytecodes for **value unpacking)
# Python 3.10a1 3430 (Make 'annotations' future by default)
# Python 3.10a1 3431 (New line number table format -- PEP 626)
# Python 3.10a2 3432 (Function annotation for MAKE_FUNCTION is changed from dict to tuple bpo-42202)
# Python 3.10a2 3433 (RERAISE restores f_lasti if oparg != 0)
# Python 3.10a6 3434 (PEP 634: Structural Pattern Matching)
# Python 3.10a7 3435 Use instruction offsets (as opposed to byte offsets).
# Python 3.10b1 3436 (Add GEN_START bytecode #43683)
# Python 3.10b1 3437 (Undo making 'annotations' future by default - We like to dance among core devs!)
# Python 3.10b1 3438 Safer line number table handling.
# Python 3.10b1 3439 (Add ROT_N)
# Python 3.11a1 3450 Use exception table for unwinding ("zero cost" exception handling)
# Python 3.11a1 3451 (Add CALL_METHOD_KW)
# Python 3.11a1 3452 (drop nlocals from marshaled code objects)
# Python 3.11a1 3453 (add co_fastlocalnames and co_fastlocalkinds)
# Python 3.11a1 3454 (compute cell offsets relative to locals bpo-43693)
# Python 3.11a1 3455 (add MAKE_CELL bpo-43693)
# Python 3.11a1 3456 (interleave cell args bpo-43693)
# Python 3.11a1 3457 (Change localsplus to a bytes object bpo-43693)
# Python 3.11a1 3458 (imported objects now don't use LOAD_METHOD/CALL_METHOD)
# Python 3.11a1 3459 (PEP 657: add end line numbers and column offsets for instructions)
#
# MAGIC must change whenever the bytecode emitted by the compiler may no
# longer be understood by older implementations of the eval loop (usually
# due to the addition of new opcodes).
#
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.
MAGIC_NUMBER = (3459).to_bytes(2, 'little') + b'\r\n'

@P403n1x87
bump MAGIC_NUMBER
ed26131
@P403n1x87
Copy link
Contributor Author

@P403n1x87 P403n1x87 commented Jul 6, 2021

I have made the requested changes; please review again
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 6, 2021

Thanks for making the requested changes!

@pablogsal: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from pablogsal Jul 6, 2021
@markshannon
markshannon approved these changes Jul 7, 2021
View changes
Copy link
Contributor

@markshannon markshannon left a comment

👍 Looks good.
I'll leave merging to @pablogsal
@pablogsal pablogsal merged commit 2f180ce into python:main Jul 7, 2021
13 checks passed
13 checks passed
@github-actions
Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
@github-actions
Address sanitizer Address sanitizer
Details
Azure Pipelines PR #20210706.4 succeeded
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@bedevere-bot
bedevere/issue-number Issue number 44530 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jul 7, 2021

Great job @P403n1x87! Happy to see Austin getting better :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon

@tiran tiran

@1st1 1st1

@pablogsal pablogsal

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
5 participants
@P403n1x87 @bedevere-bot @pablogsal @markshannon @the-knights-who-say-ni