Skip to content

gh-93911: Specialize LOAD_ATTR for custom __getattr__ and __getattribute__ #93988

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

Merged
merged 21 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions Include/internal/pycore_opcode.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ extern PyStatus _PyTypes_InitSlotDefs(void);
extern void _PyStaticType_Dealloc(PyTypeObject *type);


extern PyObject *_Py_slot_tp_getattro(PyObject *self, PyObject *name);
extern PyObject *_Py_slot_tp_getattr_hook(PyObject *self, PyObject *name);
extern PyObject *_Py_call_attribute(PyObject *self, PyObject *attr, PyObject *name);
#ifdef __cplusplus
}
#endif
Expand Down
59 changes: 30 additions & 29 deletions Include/opcode.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ def jabs_op(name, op):
"LOAD_ATTR_ADAPTIVE",
# These potentially push [NULL, bound method] onto the stack.
"LOAD_ATTR_CLASS",
"LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN",
"LOAD_ATTR_INSTANCE_VALUE",
"LOAD_ATTR_MODULE",
"LOAD_ATTR_PROPERTY",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Specialize ``LOAD_ATTR`` for objects with custom ``__getattr__`` and ``__getattribute__``.
38 changes: 19 additions & 19 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -7768,24 +7768,24 @@ slot_tp_call(PyObject *self, PyObject *args, PyObject *kwds)

/* There are two slot dispatch functions for tp_getattro.

- slot_tp_getattro() is used when __getattribute__ is overridden
- _Py_slot_tp_getattro() is used when __getattribute__ is overridden
but no __getattr__ hook is present;

- slot_tp_getattr_hook() is used when a __getattr__ hook is present.
- _Py_slot_tp_getattr_hook() is used when a __getattr__ hook is present.

The code in update_one_slot() always installs slot_tp_getattr_hook(); this
detects the absence of __getattr__ and then installs the simpler slot if
necessary. */
The code in update_one_slot() always installs _Py_slot_tp_getattr_hook();
this detects the absence of __getattr__ and then installs the simpler
slot if necessary. */

static PyObject *
slot_tp_getattro(PyObject *self, PyObject *name)
PyObject *
_Py_slot_tp_getattro(PyObject *self, PyObject *name)
{
PyObject *stack[2] = {self, name};
return vectorcall_method(&_Py_ID(__getattribute__), stack, 2);
}

static inline PyObject *
call_attribute(PyObject *self, PyObject *attr, PyObject *name)
PyObject *
_Py_call_attribute(PyObject *self, PyObject *attr, PyObject *name)
{
PyObject *res, *descr = NULL;

Expand All @@ -7809,8 +7809,8 @@ call_attribute(PyObject *self, PyObject *attr, PyObject *name)
return res;
}

static PyObject *
slot_tp_getattr_hook(PyObject *self, PyObject *name)
PyObject *
_Py_slot_tp_getattr_hook(PyObject *self, PyObject *name)
{
PyTypeObject *tp = Py_TYPE(self);
PyObject *getattr, *getattribute, *res;
Expand All @@ -7819,19 +7819,19 @@ slot_tp_getattr_hook(PyObject *self, PyObject *name)
method fully for each attribute lookup for classes with
__getattr__, even when the attribute is present. So we use
_PyType_Lookup and create the method only when needed, with
call_attribute. */
_Py_call_attribute. */
getattr = _PyType_Lookup(tp, &_Py_ID(__getattr__));
if (getattr == NULL) {
/* No __getattr__ hook: use a simpler dispatcher */
tp->tp_getattro = slot_tp_getattro;
return slot_tp_getattro(self, name);
tp->tp_getattro = _Py_slot_tp_getattro;
return _Py_slot_tp_getattro(self, name);
}
Py_INCREF(getattr);
/* speed hack: we could use lookup_maybe, but that would resolve the
method fully for each attribute lookup for classes with
__getattr__, even when self has the default __getattribute__
method. So we use _PyType_Lookup and create the method only when
needed, with call_attribute. */
needed, with _Py_call_attribute. */
getattribute = _PyType_Lookup(tp, &_Py_ID(__getattribute__));
if (getattribute == NULL ||
(Py_IS_TYPE(getattribute, &PyWrapperDescr_Type) &&
Expand All @@ -7840,12 +7840,12 @@ slot_tp_getattr_hook(PyObject *self, PyObject *name)
res = PyObject_GenericGetAttr(self, name);
else {
Py_INCREF(getattribute);
res = call_attribute(self, getattribute, name);
res = _Py_call_attribute(self, getattribute, name);
Py_DECREF(getattribute);
}
if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) {
PyErr_Clear();
res = call_attribute(self, getattr, name);
res = _Py_call_attribute(self, getattr, name);
}
Py_DECREF(getattr);
return res;
Expand Down Expand Up @@ -8184,10 +8184,10 @@ static slotdef slotdefs[] = {
PyWrapperFlag_KEYWORDS),
TPSLOT("__str__", tp_str, slot_tp_str, wrap_unaryfunc,
"__str__($self, /)\n--\n\nReturn str(self)."),
TPSLOT("__getattribute__", tp_getattro, slot_tp_getattr_hook,
TPSLOT("__getattribute__", tp_getattro, _Py_slot_tp_getattr_hook,
wrap_binaryfunc,
"__getattribute__($self, name, /)\n--\n\nReturn getattr(self, name)."),
TPSLOT("__getattr__", tp_getattro, slot_tp_getattr_hook, NULL, ""),
TPSLOT("__getattr__", tp_getattro, _Py_slot_tp_getattr_hook, NULL, ""),
TPSLOT("__setattr__", tp_setattro, slot_tp_setattro, wrap_setattr,
"__setattr__($self, name, value, /)\n--\n\nImplement setattr(self, name, value)."),
TPSLOT("__delattr__", tp_setattro, slot_tp_setattro, wrap_delattr,
Expand Down
42 changes: 42 additions & 0 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -3655,6 +3655,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
DEOPT_IF(cls->tp_version_tag != type_version, LOAD_ATTR);
assert(type_version != 0);
PyObject *fget = read_obj(cache->descr);
assert(Py_IS_TYPE(fget, &PyFunction_Type));
PyFunctionObject *f = (PyFunctionObject *)fget;
uint32_t func_version = read_u32(cache->keys_version);
assert(func_version != 0);
Expand All @@ -3681,6 +3682,47 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
goto start_frame;
}

TARGET(LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN) {
assert(cframe.use_tracing == 0);
DEOPT_IF(tstate->interp->eval_frame, LOAD_ATTR);
_PyLoadMethodCache *cache = (_PyLoadMethodCache *)next_instr;
PyObject *owner = TOP();
PyTypeObject *cls = Py_TYPE(owner);
uint32_t type_version = read_u32(cache->type_version);
DEOPT_IF(cls->tp_version_tag != type_version, LOAD_ATTR);
assert(type_version != 0);
PyObject *getattribute = read_obj(cache->descr);
assert(Py_IS_TYPE(getattribute, &PyFunction_Type));
PyFunctionObject *f = (PyFunctionObject *)getattribute;
uint32_t func_version = read_u32(cache->keys_version);
assert(func_version != 0);
DEOPT_IF(f->func_version != func_version, LOAD_ATTR);
PyCodeObject *code = (PyCodeObject *)f->func_code;
assert(code->co_argcount == 2);
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL);
STAT_INC(LOAD_ATTR, hit);

PyObject *name = GETITEM(names, oparg >> 1);
Py_INCREF(f);
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f);
SET_TOP(NULL);
int push_null = !(oparg & 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We push NULL if oparg & 1 (and the line below should be STACK_SHRINK(1-push_null))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

int shrink_stack = !(oparg & 1);
STACK_SHRINKG(shrink_stack);

The behaviour is equivalent but it does save a subtract operation.

STACK_SHRINK(push_null);
Py_INCREF(name);
new_frame->localsplus[0] = owner;
new_frame->localsplus[1] = name;
for (int i = 2; i < code->co_nlocalsplus; i++) {
new_frame->localsplus[i] = NULL;
}
_PyFrame_SetStackPointer(frame, stack_pointer);
JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
frame->prev_instr = next_instr - 1;
new_frame->previous = frame;
frame = cframe.current_frame = new_frame;
CALL_STAT_INC(inlined_py_calls);
goto start_frame;
}

TARGET(STORE_ATTR_ADAPTIVE) {
assert(cframe.use_tracing == 0);
_PyAttrCache *cache = (_PyAttrCache *)next_instr;
Expand Down
Loading