Make LOAD_ATTR deferred

Author: Fidget-Spinner

Include/internal/pycore_dict.h

@@ -118,6 +118,7 @@ PyAPI_FUNC(int) _PyDict_SetItem_KnownHash_LockHeld(PyDictObject *mp, PyObject *k
 PyAPI_FUNC(int) _PyDict_GetItemRef_KnownHash_LockHeld(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result);
 extern int _PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result);
 extern int _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result);
+extern int _PyDict_GetItem_KnownHash_StackRef(PyDictObject *op, PyObject *key, Py_hash_t hash, _PyStackRef *result);
 extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr, PyObject *name, PyObject *value);
 
 extern int _PyDict_Pop_KnownHash(

Include/internal/pycore_object.h

@@ -837,6 +837,7 @@ PyAPI_FUNC(PyObject*) _PyObject_LookupSpecialMethod(PyObject *self, PyObject *at
 extern int _PyObject_IsAbstract(PyObject *);
 
 PyAPI_FUNC(int) _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method);
+PyAPI_FUNC(int) _PyObject_GetMethodStackRef(PyObject *obj, PyObject *name, _PyStackRef *method);
 extern PyObject* _PyObject_NextNotImplemented(PyObject *);
 
 // Pickle support.
@@ -874,6 +875,8 @@ PyAPI_DATA(int) _Py_SwappedOp[];
 
 extern void _Py_GetConstant_Init(void);
 
+extern void _PyType_LookupStackRef(PyTypeObject *type, PyObject *name, _PyStackRef *result);
+
 #ifdef __cplusplus
 }
 #endif

Include/internal/pycore_stackref.h

@@ -149,8 +149,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj)
 {
     // Make sure we don't take an already tagged value.
     assert(((uintptr_t)obj & Py_TAG_BITS) == 0);
-    assert(obj != NULL);
-    if (_Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj)) {
+    if (obj == NULL || _Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj)) {
         return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED };
     }
     else {

Objects/dictobject.c

@@ -2324,6 +2324,27 @@ _PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, Py
     return 1;  // key is present
 }
 
+int
+_PyDict_GetItem_KnownHash_StackRef(PyDictObject *op, PyObject *key, Py_hash_t hash, _PyStackRef *result)
+{
+#ifdef Py_GIL_DISABLED
+    Py_ssize_t ix = _Py_dict_lookup_threadsafe_stackref(op, key, hash, result);
+#else
+    PyObject *value;
+    Py_ssize_t ix = _Py_dict_lookup(op, key, hash, &value);
+    *result = PyStackRef_FromPyObjectSteal(value);
+#endif
+    assert(ix >= 0 || PyStackRef_IsNull(*result));
+    if (ix == DKIX_ERROR) {
+        *result = PyStackRef_NULL;
+        return -1;
+    }
+    if (PyStackRef_IsNull(*result)) {
+        return 0;  // missing key
+    }
+    return 1;  // key is present
+}
+
 int
 PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **result)
 {

Objects/object.c

@@ -1581,6 +1581,106 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
     return 0;
 }
 
+int
+_PyObject_GetMethodStackRef(PyObject *obj, PyObject *name, _PyStackRef *method)
+{
+
+    int meth_found = 0;
+
+    assert(PyStackRef_IsNull(*method));
+
+    PyTypeObject *tp = Py_TYPE(obj);
+    if (!_PyType_IsReady(tp)) {
+        if (PyType_Ready(tp) < 0) {
+            return 0;
+        }
+    }
+
+    if (tp->tp_getattro != PyObject_GenericGetAttr || !PyUnicode_CheckExact(name)) {
+        *method = PyStackRef_FromPyObjectSteal(PyObject_GetAttr(obj, name));
+        return 0;
+    }
+
+    // Directly set it to that if a GC cycle happens, the descriptor doesn't get
+    // evaporated.
+    // This is why we no longer need a strong reference for this if it's
+    // deferred.
+    _PyType_LookupStackRef(tp, name, method);
+    _PyStackRef descr_st = *method;
+    descrgetfunc f = NULL;
+    if (!PyStackRef_IsNull(descr_st)) {
+        if (_PyType_HasFeature(PyStackRef_TYPE(descr_st), Py_TPFLAGS_METHOD_DESCRIPTOR)) {
+            meth_found = 1;
+        } else {
+            f = PyStackRef_TYPE(descr_st)->tp_descr_get;
+            if (f != NULL && PyDescr_IsData(PyStackRef_AsPyObjectBorrow(descr_st))) {
+                *method = PyStackRef_FromPyObjectSteal(f(PyStackRef_AsPyObjectBorrow(descr_st), obj, (PyObject *)Py_TYPE(obj)));
+                PyStackRef_CLOSE(descr_st);
+                return 0;
+            }
+        }
+    }
+    PyObject *dict, *attr;
+    if ((tp->tp_flags & Py_TPFLAGS_INLINE_VALUES) &&
+        _PyObject_TryGetInstanceAttribute(obj, name, &attr)) {
+        if (attr != NULL) {
+            *method = PyStackRef_FromPyObjectSteal(attr);
+            PyStackRef_XCLOSE(descr_st);
+            return 0;
+        }
+        dict = NULL;
+    }
+    else if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT)) {
+        dict = (PyObject *)_PyObject_GetManagedDict(obj);
+    }
+    else {
+        PyObject **dictptr = _PyObject_ComputedDictPointer(obj);
+        if (dictptr != NULL) {
+            dict = *dictptr;
+        }
+        else {
+            dict = NULL;
+        }
+    }
+    if (dict != NULL) {
+        Py_INCREF(dict);
+        PyObject *item;
+        if (PyDict_GetItemRef(dict, name, &item) != 0) {
+            *method = PyStackRef_FromPyObjectSteal(item);
+            // found or error
+            Py_DECREF(dict);
+            PyStackRef_CLOSE(descr_st);
+            return 0;
+        }
+        // not found
+        Py_DECREF(dict);
+    }
+
+    if (meth_found) {
+        *method = descr_st;
+        return 1;
+    }
+
+    if (f != NULL) {
+        *method = PyStackRef_FromPyObjectSteal(f(PyStackRef_AsPyObjectBorrow(descr_st), obj, (PyObject *)Py_TYPE(obj)));
+        PyStackRef_CLOSE(descr_st);
+        return 0;
+    }
+
+    if (!PyStackRef_IsNull(descr_st)) {
+        *method = descr_st;
+        return 0;
+    }
+
+    *method = PyStackRef_NULL;
+    PyErr_Format(PyExc_AttributeError,
+                 "'%.100s' object has no attribute '%U'",
+                 tp->tp_name, name);
+
+    _PyObject_SetAttributeErrorContext(obj, name);
+    return 0;
+}
+
 /* Generic GetAttr functions - put these in your tp_[gs]etattro slot. */
 
 PyObject *

Objects/typeobject.c

@@ -5356,6 +5356,58 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error)
     return res;
 }
 
+
+static void
+find_name_in_mro_stackref(PyTypeObject *type, PyObject *name, int *error, _PyStackRef *result)
+{
+    ASSERT_TYPE_LOCK_HELD();
+
+    Py_hash_t hash = _PyObject_HashFast(name);
+    if (hash == -1) {
+        *error = -1;
+        *result = PyStackRef_NULL;
+        return;
+    }
+
+    /* Look in tp_dict of types in MRO */
+    PyObject *mro = lookup_tp_mro(type);
+    if (mro == NULL) {
+        if (!is_readying(type)) {
+            if (PyType_Ready(type) < 0) {
+                *error = -1;
+                *result = PyStackRef_NULL;
+                return;
+            }
+            mro = lookup_tp_mro(type);
+        }
+        if (mro == NULL) {
+            *error = 1;
+            *result = PyStackRef_NULL;
+            return;
+        }
+    }
+
+    /* Keep a strong reference to mro because type->tp_mro can be replaced
+       during dict lookup, e.g. when comparing to non-string keys. */
+    Py_INCREF(mro);
+    Py_ssize_t n = PyTuple_GET_SIZE(mro);
+    for (Py_ssize_t i = 0; i < n; i++) {
+        PyObject *base = PyTuple_GET_ITEM(mro, i);
+        PyObject *dict = lookup_tp_dict(_PyType_CAST(base));
+        assert(dict && PyDict_Check(dict));
+        if (_PyDict_GetItem_KnownHash_StackRef((PyDictObject *)dict, name, hash, result) < 0) {
+            *error = -1;
+            goto done;
+        }
+        if (!PyStackRef_IsNull(*result)) {
+            break;
+        }
+    }
+    *error = 0;
+done:
+    Py_DECREF(mro);
+}
+
 /* Check if the "readied" PyUnicode name
    is a double-underscore special name. */
 static int
@@ -5528,6 +5580,95 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
     return res;
 }
 
+void
+_PyType_LookupStackRef(PyTypeObject *type, PyObject *name, _PyStackRef *result)
+{
+    int error;
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+
+    unsigned int h = MCACHE_HASH_METHOD(type, name);
+    struct type_cache *cache = get_type_cache();
+    struct type_cache_entry *entry = &cache->hashtable[h];
+#ifdef Py_GIL_DISABLED
+    // synchronize-with other writing threads by doing an acquire load on the sequence
+    while (1) {
+        int sequence = _PySeqLock_BeginRead(&entry->sequence);
+        uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
+        uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
+        if (entry_version == type_version &&
+            _Py_atomic_load_ptr_relaxed(&entry->name) == name) {
+            OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name));
+            OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name));
+            PyObject *value = _Py_atomic_load_ptr_relaxed(&entry->value);
+            *result = PyStackRef_FromPyObjectNew(value);
+            // If the sequence is still valid then we're done
+            if (_PySeqLock_EndRead(&entry->sequence, sequence)) {
+                return;
+            }
+            *result = PyStackRef_NULL;
+        }
+        else {
+            // cache miss
+            break;
+        }
+    }
+#else
+    if (entry->version == type->tp_version_tag &&
+        entry->name == name) {
+        assert(type->tp_version_tag);
+        OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name));
+        OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name));
+        Py_XINCREF(entry->value);
+        return entry->value;
+    }
+#endif
+    OBJECT_STAT_INC_COND(type_cache_misses, !is_dunder_name(name));
+    OBJECT_STAT_INC_COND(type_cache_dunder_misses, is_dunder_name(name));
+
+    /* We may end up clearing live exceptions below, so make sure it's ours. */
+    assert(!PyErr_Occurred());
+
+    // We need to atomically do the lookup and capture the version before
+    // anyone else can modify our mro or mutate the type.
+
+    int has_version = 0;
+    int version = 0;
+    BEGIN_TYPE_LOCK();
+    find_name_in_mro_stackref(type, name, &error, result);
+    if (MCACHE_CACHEABLE_NAME(name)) {
+        has_version = assign_version_tag(interp, type);
+        version = type->tp_version_tag;
+    }
+    END_TYPE_LOCK();
+
+    /* Only put NULL results into cache if there was no error. */
+    if (error) {
+        /* It's not ideal to clear the error condition,
+           but this function is documented as not setting
+           an exception, and I don't want to change that.
+           E.g., when PyType_Ready() can't proceed, it won't
+           set the "ready" flag, so future attempts to ready
+           the same type will call it again -- hopefully
+           in a context that propagates the exception out.
+        */
+        if (error == -1) {
+            PyErr_Clear();
+        }
+        *result = PyStackRef_NULL;
+        return;
+    }
+
+    if (has_version) {
+#if Py_GIL_DISABLED
+        update_cache_gil_disabled(entry, name, version,
+                                  PyStackRef_AsPyObjectBorrow(*result));
+#else
+        PyObject *old_value = update_cache(entry, name, version, PyStackRef_AsPyObjectBorrow(*result));
+        Py_DECREF(old_value);
+#endif
+    }
+}
+
 PyObject *
 _PyType_Lookup(PyTypeObject *type, PyObject *name)
 {

Python/bytecodes.c

@@ -1977,19 +1977,17 @@ dummy_func(
             #endif  /* ENABLE_SPECIALIZATION */
         }
 
-        op(_LOAD_ATTR, (owner -- attr, self_or_null if (oparg & 1))) {
+        op(_LOAD_ATTR, (owner -- attr: _PyStackRef *, self_or_null if (oparg & 1))) {
             PyObject *name = GETITEM(FRAME_CO_NAMES, oparg >> 1);
-            PyObject *attr_o;
             if (oparg & 1) {
                 /* Designed to work in tandem with CALL, pushes two values. */
-                attr_o = NULL;
-                int is_meth = _PyObject_GetMethod(PyStackRef_AsPyObjectBorrow(owner), name, &attr_o);
-                if (is_meth) {
+                *attr = PyStackRef_NULL;
+                if (_PyObject_GetMethodStackRef(PyStackRef_AsPyObjectBorrow(owner), name, attr)) {
                     /* We can bypass temporary bound method object.
                        meth is unbound method and obj is self.
                        meth | self | arg1 | ... | argN
                      */
-                    assert(attr_o != NULL);  // No errors on this branch
+                    assert(!PyStackRef_IsNull(*attr));  // No errors on this branch
                     self_or_null = owner;  // Transfer ownership
                 }
                 else {
@@ -2000,17 +1998,16 @@ dummy_func(
                        meth | NULL | arg1 | ... | argN
                     */
                     DECREF_INPUTS();
-                    ERROR_IF(attr_o == NULL, error);
+                    ERROR_IF(PyStackRef_IsNull(*attr), error);
                     self_or_null = PyStackRef_NULL;
                 }
             }
             else {
                 /* Classic, pushes one value. */
-                attr_o = PyObject_GetAttr(PyStackRef_AsPyObjectBorrow(owner), name);
+                *attr = PyStackRef_FromPyObjectSteal(PyObject_GetAttr(PyStackRef_AsPyObjectBorrow(owner), name));
                 DECREF_INPUTS();
-                ERROR_IF(attr_o == NULL, error);
+                ERROR_IF(PyStackRef_IsNull(*attr), error);
             }
-            attr = PyStackRef_FromPyObjectSteal(attr_o);
         }
 
         macro(LOAD_ATTR) =