-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
Changes from all commits
1e02be1
d891208
4fe05f1
82eb42d
4fbaa99
1573e3d
85bdaf9
51f2e49
File filter
Filter by extension
Conversations
Jump to
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
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 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 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 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. 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. 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? 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 resolved
Hide resolved
|
||
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 resolved
Hide resolved
|
||
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.
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.