Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-44525: Copy freevars in bytecode to allow calls to inner functions to be specialized#29595
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
1e02be1d8912084fe05f182eb42d4fbaa991573e3d85bdaf951f2e49File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -20,13 +20,13 @@ enum _framestate{ | ||
| typedef signed char PyFrameState; | ||
| typedef struct _interpreter_frame{ | ||
| PyObject *f_globals; | ||
| PyObject *f_builtins; | ||
| PyObject *f_locals; | ||
| PyCodeObject *f_code; | ||
| PyFrameObject *frame_obj; | ||
| /* Borrowed reference to a generator, or NULL */ | ||
| PyObject *generator; | ||
| PyFunctionObject *f_func; /* Strong reference */ | ||
| PyObject *f_globals; /* Borrowed reference */ | ||
| PyObject *f_builtins; /* Borrowed reference */ | ||
Comment on lines +24 to +25 Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, we avoid extra refcount operations for these, since The only problem I see is that the function's Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having these on However, that only matters in cases where these are used relatively frequently (so the cost of the indirection adds up), i.e. in the eval loop (via Note that then we would have a different possible race where the function's Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, why don't we similarly add MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We might want to avoid creating a closure object at all. We could put the cells at the end of the function object, rather than in a separate tuple. but that's future work. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you skipped over the two other comments here. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The frame has a strong reference to the function, which has a strong reference to both the globals and builtins.
And where would you store those references across calls? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we keep | ||
| PyObject *f_locals; /* Strong reference, may be NULL */ | ||
| PyCodeObject *f_code; /* Strong reference */ | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| PyFrameObject *frame_obj; /* Strong reference, may be NULL */ | ||
| PyObject *generator; /* Borrowed reference, may be NULL */ | ||
| struct _interpreter_frame *previous; | ||
| int f_lasti; /* Last instruction if called */ | ||
| int stacktop; /* Offset of TOS from localsplus */ | ||
| @@ -70,16 +70,18 @@ static inline void _PyFrame_StackPush(InterpreterFrame *f, PyObject *value){ | ||
| #define FRAME_SPECIALS_SIZE ((sizeof(InterpreterFrame)-1)/sizeof(PyObject *)) | ||
| InterpreterFrame * | ||
| _PyInterpreterFrame_HeapAlloc(PyFrameConstructor *con, PyObject *locals); | ||
| _PyInterpreterFrame_HeapAlloc(PyFunctionObject *func, PyObject *locals); | ||
| static inline void | ||
| _PyFrame_InitializeSpecials( | ||
| InterpreterFrame *frame, PyFrameConstructor *con, | ||
| InterpreterFrame *frame, PyFunctionObject *func, | ||
| PyObject *locals, int nlocalsplus) | ||
| { | ||
| frame->f_code = (PyCodeObject *)Py_NewRef(con->fc_code); | ||
| frame->f_builtins = Py_NewRef(con->fc_builtins); | ||
| frame->f_globals = Py_NewRef(con->fc_globals); | ||
| Py_INCREF(func); | ||
| frame->f_func = func; | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| frame->f_code = (PyCodeObject *)Py_NewRef(func->func_code); | ||
| frame->f_builtins = func->func_builtins; | ||
| frame->f_globals = func->func_globals; | ||
| frame->f_locals = Py_XNewRef(locals); | ||
| frame->stacktop = nlocalsplus; | ||
| frame->frame_obj = NULL; | ||
| @@ -150,7 +152,7 @@ void | ||
| _PyFrame_LocalsToFast(InterpreterFrame *frame, int clear); | ||
| InterpreterFrame *_PyThreadState_PushFrame( | ||
| PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals); | ||
| PyThreadState *tstate, PyFunctionObject *func, PyObject *locals); | ||
| extern InterpreterFrame * | ||
| _PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| #ifndef Py_INTERNAL_FUNCTION_H | ||
| #define Py_INTERNAL_FUNCTION_H | ||
| #include "Python.h" | ||
| PyFunctionObject * | ||
| _PyFunction_FromConstructor(PyFrameConstructor *constr); | ||
| #endif /* !Py_INTERNAL_FUNCTION_H */ |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -141,6 +141,8 @@ | ||
| check_impl_detail, requires_debug_ranges, | ||
| gc_collect) | ||
| from test.support.script_helper import assert_python_ok | ||
| from opcode import opmap | ||
| COPY_FREE_VARS = opmap['COPY_FREE_VARS'] | ||
| def consts(t): | ||
| @@ -185,7 +187,7 @@ def create_closure(__class__): | ||
| def new_code(c): | ||
| '''A new code object with a __class__ cell added to freevars''' | ||
| return c.replace(co_freevars=c.co_freevars + ('__class__',)) | ||
| return c.replace(co_freevars=c.co_freevars + ('__class__',), co_code=bytes([COPY_FREE_VARS, 1])+c.co_code) | ||
ericsnowcurrently marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| def add_foreign_method(cls, name, f): | ||
| code = new_code(f.__code__) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -459,16 +459,17 @@ def foo(x): | ||
| dis_nested_1 = """%s | ||
| Disassembly of <code object foo at 0x..., file "%s", line %d>: | ||
| 0 MAKE_CELL 0 (x) | ||
| %3d 2 LOAD_CLOSURE 0 (x) | ||
| 4 BUILD_TUPLE 1 | ||
| 6 LOAD_CONST 1 (<code object <listcomp> at 0x..., file "%s", line %d>) | ||
| 8 MAKE_FUNCTION 8 (closure) | ||
| 10 LOAD_DEREF 1 (y) | ||
| 12 GET_ITER | ||
| 14 CALL_FUNCTION 1 | ||
| 16 RETURN_VALUE | ||
| 0 COPY_FREE_VARS 1 | ||
| 2 MAKE_CELL 0 (x) | ||
| %3d 4 LOAD_CLOSURE 0 (x) | ||
| 6 BUILD_TUPLE 1 | ||
| 8 LOAD_CONST 1 (<code object <listcomp> at 0x..., file "%s", line %d>) | ||
| 10 MAKE_FUNCTION 8 (closure) | ||
| 12 LOAD_DEREF 1 (y) | ||
| 14 GET_ITER | ||
| 16 CALL_FUNCTION 1 | ||
| 18 RETURN_VALUE | ||
| """ % (dis_nested_0, | ||
| __file__, | ||
| _h.__code__.co_firstlineno + 1, | ||
| @@ -479,16 +480,18 @@ def foo(x): | ||
| dis_nested_2 = """%s | ||
| Disassembly of <code object <listcomp> at 0x..., file "%s", line %d>: | ||
| %3d 0 BUILD_LIST 0 | ||
| 2 LOAD_FAST 0 (.0) | ||
| >> 4 FOR_ITER 6 (to 18) | ||
| 6 STORE_FAST 1 (z) | ||
| 8 LOAD_DEREF 2 (x) | ||
| 10 LOAD_FAST 1 (z) | ||
| 12 BINARY_OP 0 (+) | ||
| 14 LIST_APPEND 2 | ||
| 16 JUMP_ABSOLUTE 2 (to 4) | ||
| >> 18 RETURN_VALUE | ||
| 0 COPY_FREE_VARS 1 | ||
| %3d 2 BUILD_LIST 0 | ||
| 4 LOAD_FAST 0 (.0) | ||
| >> 6 FOR_ITER 6 (to 20) | ||
| 8 STORE_FAST 1 (z) | ||
| 10 LOAD_DEREF 2 (x) | ||
| 12 LOAD_FAST 1 (z) | ||
| 14 BINARY_OP 0 (+) | ||
| 16 LIST_APPEND 2 | ||
| 18 JUMP_ABSOLUTE 3 (to 6) | ||
| >> 20 RETURN_VALUE | ||
| """ % (dis_nested_1, | ||
| __file__, | ||
| _h.__code__.co_firstlineno + 3, | ||
| @@ -1007,42 +1010,43 @@ def _prepare_test_cases(): | ||
| Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=40, starts_line=None, is_jump_target=False, positions=None), | ||
| ] | ||
| expected_opinfo_f = [ | ||
| Instruction(opname='MAKE_CELL', opcode=135, arg=0, argval='c', argrepr='c', offset=0, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='MAKE_CELL', opcode=135, arg=1, argval='d', argrepr='d', offset=2, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=4, argval=(5, 6), argrepr='(5, 6)', offset=4, starts_line=3, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=3, argval='a', argrepr='a', offset=6, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=4, argval='b', argrepr='b', offset=8, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=0, argval='c', argrepr='c', offset=10, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=1, argval='d', argrepr='d', offset=12, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='BUILD_TUPLE', opcode=102, arg=4, argval=4, argrepr='', offset=14, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=3, argval=code_object_inner, argrepr=repr(code_object_inner), offset=16, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='MAKE_FUNCTION', opcode=132, arg=9, argval=9, argrepr='defaults, closure', offset=18, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='STORE_FAST', opcode=125, arg=2, argval='inner', argrepr='inner', offset=20, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_GLOBAL', opcode=116, arg=0, argval='print', argrepr='print', offset=22, starts_line=5, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=3, argval='a', argrepr='a', offset=24, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=4, argval='b', argrepr='b', offset=26, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=0, argval='c', argrepr='c', offset=28, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=1, argval='d', argrepr='d', offset=30, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='CALL_FUNCTION', opcode=131, arg=4, argval=4, argrepr='', offset=32, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=34, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=2, argval='inner', argrepr='inner', offset=36, starts_line=6, is_jump_target=False, positions=None), | ||
| Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=38, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='COPY_FREE_VARS', opcode=149, arg=2, argval=2, argrepr='', offset=0, starts_line=None, is_jump_target=False, positions=None), | ||
markshannon marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| Instruction(opname='MAKE_CELL', opcode=135, arg=0, argval='c', argrepr='c', offset=2, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='MAKE_CELL', opcode=135, arg=1, argval='d', argrepr='d', offset=4, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=4, argval=(5, 6), argrepr='(5, 6)', offset=6, starts_line=3, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=3, argval='a', argrepr='a', offset=8, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=4, argval='b', argrepr='b', offset=10, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=0, argval='c', argrepr='c', offset=12, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CLOSURE', opcode=136, arg=1, argval='d', argrepr='d', offset=14, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='BUILD_TUPLE', opcode=102, arg=4, argval=4, argrepr='', offset=16, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=3, argval=code_object_inner, argrepr=repr(code_object_inner), offset=18, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='MAKE_FUNCTION', opcode=132, arg=9, argval=9, argrepr='defaults, closure', offset=20, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='STORE_FAST', opcode=125, arg=2, argval='inner', argrepr='inner', offset=22, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_GLOBAL', opcode=116, arg=0, argval='print', argrepr='print', offset=24, starts_line=5, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=3, argval='a', argrepr='a', offset=26, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=4, argval='b', argrepr='b', offset=28, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=0, argval='c', argrepr='c', offset=30, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=1, argval='d', argrepr='d', offset=32, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='CALL_FUNCTION', opcode=131, arg=4, argval=4, argrepr='', offset=34, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=36, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=2, argval='inner', argrepr='inner', offset=38, starts_line=6, is_jump_target=False, positions=None), | ||
| Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=40, starts_line=None, is_jump_target=False, positions=None), | ||
| ] | ||
| expected_opinfo_inner = [ | ||
| Instruction(opname='LOAD_GLOBAL', opcode=116, arg=0, argval='print', argrepr='print', offset=0, starts_line=4, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=2, argval='a', argrepr='a', offset=2, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=3, argval='b', argrepr='b', offset=4, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=4, argval='c', argrepr='c', offset=6, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=5, argval='d', argrepr='d', offset=8, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='e', argrepr='e', offset=10, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=1, argval='f', argrepr='f', offset=12, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='CALL_FUNCTION', opcode=131, arg=6, argval=6, argrepr='', offset=14, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=16, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=18, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=20, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='COPY_FREE_VARS', opcode=149, arg=4, argval=4, argrepr='', offset=0, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_GLOBAL', opcode=116, arg=0, argval='print', argrepr='print', offset=2, starts_line=4, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=2, argval='a', argrepr='a', offset=4, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=3, argval='b', argrepr='b', offset=6, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=4, argval='c', argrepr='c', offset=8, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_DEREF', opcode=137, arg=5, argval='d', argrepr='d', offset=10, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='e', argrepr='e', offset=12, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_FAST', opcode=124, arg=1, argval='f', argrepr='f', offset=14, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='CALL_FUNCTION', opcode=131, arg=6, argval=6, argrepr='', offset=16, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=18, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=20, starts_line=None, is_jump_target=False, positions=None), | ||
| Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=22, starts_line=None, is_jump_target=False, positions=None), | ||
| ] | ||
| expected_opinfo_jumpy = [ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Adds new :opcode:`COPY_FREE_VARS` opcode, to make copying of free variables | ||
| from function to frame explicit. Helps optimization of calls to Python | ||
| function. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.