bpo-42927: Inline cache for attributes defined with '__slots__' (#24216)

2 files changed, 118 insertions, 64 deletions

diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-01-15-20-05-56.bpo-42927.GI-l-7.rst b/Misc/NEWS.d/next/Core and Builtins/2021-01-15-20-05-56.bpo-42927.GI-l-7.rst
new file mode 100644
index 00000000000..8ee578e8162
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2021-01-15-20-05-56.bpo-42927.GI-l-7.rst
@@ -0,0 +1,2 @@
+The inline cache for ``LOAD_ATTR`` now also optimizes access to attributes defined by ``__slots__``.

+This makes reading such attribute up to 30% faster.
\ No newline at end of file
diff --git a/Python/ceval.c b/Python/ceval.c
index be9ea244547..3aa2aa2c9bc 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -29,6 +29,7 @@
 #include "opcode.h"
 #include "pydtrace.h"
 #include "setobject.h"
+#include "structmember.h"         // struct PyMemberDef, T_OFFSET_EX
 
 #include <ctype.h>
 
@@ -3169,106 +3170,157 @@ main_loop:
             if (co_opcache != NULL && PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG))
             {
                 if (co_opcache->optimized > 0) {
-                    /* Fast path -- cache hit makes LOAD_ATTR ~30% faster */
+                    // Fast path -- cache hit makes LOAD_ATTR ~30% faster.
                     la = &co_opcache->u.la;
                     if (la->type == type && la->tp_version_tag == type->tp_version_tag)
                     {
-                        assert(type->tp_dict != NULL);
-                        assert(type->tp_dictoffset > 0);
-
-                        dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
-                        dict = *dictptr;
-                        if (dict != NULL && PyDict_CheckExact(dict)) {
-                            Py_ssize_t hint = la->hint;
-                            Py_INCREF(dict);
-                            res = NULL;
-                            la->hint = _PyDict_GetItemHint((PyDictObject*)dict, name, hint, &res);
-
+                        // Hint >= 0 is a dict index; hint == -1 is a dict miss.
+                        // Hint < -1 is an inverted slot offset: offset is strictly > 0,
+                        // so ~offset is strictly < -1 (assuming 2's complement).
+                        if (la->hint < -1) {
+                            // Even faster path -- slot hint.
+                            Py_ssize_t offset = ~la->hint;
+                            // fprintf(stderr, "Using hint for offset %zd\n", offset);
+                            char *addr = (char *)owner + offset;
+                            res = *(PyObject **)addr;
                             if (res != NULL) {
-                                if (la->hint == hint && hint >= 0) {
-                                    /* Our hint has helped -- cache hit. */
-                                    OPCACHE_STAT_ATTR_HIT();
-                                } else {
-                                    /* The hint we provided didn't work.
-                                       Maybe next time? */
-                                    OPCACHE_MAYBE_DEOPT_LOAD_ATTR();
-                                }
-
                                 Py_INCREF(res);
                                 SET_TOP(res);
                                 Py_DECREF(owner);
-                                Py_DECREF(dict);
                                 DISPATCH();
+                            }
+                            // Else slot is NULL.  Fall through to slow path to raise AttributeError(name).
+                            // Don't DEOPT, since the slot is still there.
+                        } else {
+                            // Fast path for dict.
+                            assert(type->tp_dict != NULL);
+                            assert(type->tp_dictoffset > 0);
+
+                            dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
+                            dict = *dictptr;
+                            if (dict != NULL && PyDict_CheckExact(dict)) {
+                                Py_ssize_t hint = la->hint;
+                                Py_INCREF(dict);
+                                res = NULL;
+                                la->hint = _PyDict_GetItemHint((PyDictObject*)dict, name, hint, &res);
+
+                                if (res != NULL) {
+                                    if (la->hint == hint && hint >= 0) {
+                                        // Our hint has helped -- cache hit.
+                                        OPCACHE_STAT_ATTR_HIT();
+                                    } else {
+                                        // The hint we provided didn't work.
+                                        // Maybe next time?
+                                        OPCACHE_MAYBE_DEOPT_LOAD_ATTR();
+                                    }
+
+                                    Py_INCREF(res);
+                                    SET_TOP(res);
+                                    Py_DECREF(owner);
+                                    Py_DECREF(dict);
+                                    DISPATCH();
+                                } else {
+                                    // This attribute can be missing sometimes;
+                                    // we don't want to optimize this lookup.
+                                    OPCACHE_DEOPT_LOAD_ATTR();
+                                    Py_DECREF(dict);
+                                }
                             } else {
-                                // This attribute can be missing sometimes -- we
-                                // don't want to optimize this lookup.
+                                // There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact.
                                 OPCACHE_DEOPT_LOAD_ATTR();
-                                Py_DECREF(dict);
                             }
-                        } else {
-                            // There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact
-                            OPCACHE_DEOPT_LOAD_ATTR();
                         }
                     } else {
                         // The type of the object has either been updated,
                         // or is different.  Maybe it will stabilize?
                         OPCACHE_MAYBE_DEOPT_LOAD_ATTR();
                     }
-
                     OPCACHE_STAT_ATTR_MISS();
                 }
 
-                if (co_opcache != NULL && /* co_opcache can be NULL after a DEOPT() call. */
+                if (co_opcache != NULL && // co_opcache can be NULL after a DEOPT() call.
                     type->tp_getattro == PyObject_GenericGetAttr)
                 {
-                    Py_ssize_t ret;
-
-                    if (type->tp_dictoffset > 0) {
-                        if (type->tp_dict == NULL) {
-                            if (PyType_Ready(type) < 0) {
-                                Py_DECREF(owner);
-                                SET_TOP(NULL);
-                                goto error;
-                            }
+                    if (type->tp_dict == NULL) {
+                        if (PyType_Ready(type) < 0) {
+                            Py_DECREF(owner);
+                            SET_TOP(NULL);
+                            goto error;
                         }
-                        if (_PyType_Lookup(type, name) == NULL) {
-                            dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
-                            dict = *dictptr;
+                    }
+                    PyObject *descr = _PyType_Lookup(type, name);
+                    if (descr != NULL) {
+                        // We found an attribute with a data-like descriptor.
+                        PyTypeObject *dtype = Py_TYPE(descr);
+                        if (dtype == &PyMemberDescr_Type) {  // It's a slot
+                            PyMemberDescrObject *member = (PyMemberDescrObject *)descr;
+                            struct PyMemberDef *dmem = member->d_member;
+                            if (dmem->type == T_OBJECT_EX) {
+                                Py_ssize_t offset = dmem->offset;
+                                assert(offset > 0);  // 0 would be confused with dict hint == -1 (miss).
+
+                                if (co_opcache->optimized == 0) {
+                                    // First time we optimize this opcode.
+                                    OPCACHE_STAT_ATTR_OPT();
+                                    co_opcache->optimized = OPCODE_CACHE_MAX_TRIES;
+                                    // fprintf(stderr, "Setting hint for %s, offset %zd\n", dmem->name, offset);
+                                }
 
-                            if (dict != NULL && PyDict_CheckExact(dict)) {
-                                Py_INCREF(dict);
-                                res = NULL;
-                                ret = _PyDict_GetItemHint((PyDictObject*)dict, name, -1, &res);
+                                la = &co_opcache->u.la;
+                                la->type = type;
+                                la->tp_version_tag = type->tp_version_tag;
+                                la->hint = ~offset;
+
+                                char *addr = (char *)owner + offset;
+                                res = *(PyObject **)addr;
                                 if (res != NULL) {
                                     Py_INCREF(res);
-                                    Py_DECREF(dict);
                                     Py_DECREF(owner);
                                     SET_TOP(res);
 
-                                    if (co_opcache->optimized == 0) {
-                                        // First time we optimize this opcode. */
-                                        OPCACHE_STAT_ATTR_OPT();
-                                        co_opcache->optimized = OPCODE_CACHE_MAX_TRIES;
-                                    }
-
-                                    la = &co_opcache->u.la;
-                                    la->type = type;
-                                    la->tp_version_tag = type->tp_version_tag;
-                                    la->hint = ret;
-
                                     DISPATCH();
                                 }
+                                // Else slot is NULL.  Fall through to slow path to raise AttributeError(name).
+                            }
+                            // Else it's a slot of a different type.  We don't handle those.
+                        }
+                        // Else it's some other kind of descriptor that we don't handle.
+                        OPCACHE_DEOPT_LOAD_ATTR();
+                    } else if (type->tp_dictoffset > 0) {
+                        // We found an instance with a __dict__.
+                        dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
+                        dict = *dictptr;
+
+                        if (dict != NULL && PyDict_CheckExact(dict)) {
+                            Py_INCREF(dict);
+                            res = NULL;
+                            Py_ssize_t hint = _PyDict_GetItemHint((PyDictObject*)dict, name, -1, &res);
+                            if (res != NULL) {
+                                Py_INCREF(res);
                                 Py_DECREF(dict);
-                            } else {
-                                // There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact
-                                OPCACHE_DEOPT_LOAD_ATTR();
+                                Py_DECREF(owner);
+                                SET_TOP(res);
+
+                                if (co_opcache->optimized == 0) {
+                                    // First time we optimize this opcode.
+                                    OPCACHE_STAT_ATTR_OPT();
+                                    co_opcache->optimized = OPCODE_CACHE_MAX_TRIES;
+                                }
+
+                                la = &co_opcache->u.la;
+                                la->type = type;
+                                la->tp_version_tag = type->tp_version_tag;
+                                la->hint = hint;
+
+                                DISPATCH();
                             }
+                            Py_DECREF(dict);
                         } else {
-                            // We failed to find an attribute without a data-like descriptor
+                            // There is no dict, or __dict__ doesn't satisfy PyDict_CheckExact.
                             OPCACHE_DEOPT_LOAD_ATTR();
                         }
                     } else {
-                        // The object's class does not have a tp_dictoffset we can use
+                        // The object's class does not have a tp_dictoffset we can use.
                         OPCACHE_DEOPT_LOAD_ATTR();
                     }
                 } else if (type->tp_getattro != PyObject_GenericGetAttr) {
@@ -3276,7 +3328,7 @@ main_loop:
                 }
             }
 
-            /* slow path */
+            // Slow path.
             res = PyObject_GetAttr(owner, name);
             Py_DECREF(owner);
             SET_TOP(res);