[Python-checkins] gh-94673: Recover Weaklist Lookup Performance (gh-95544)

[ericsnowcurrently]

https://github.com/python/cpython/commit/bdbadb905ae638b67a6c9a6767be396e18839dd6
commit: bdbadb905ae638b67a6c9a6767be396e18839dd6
branch: main
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: ericsnowcurrently <ericsnowcurrently at gmail.com>
date: 2022-08-04T11:28:15-06:00
summary:

gh-94673: Recover Weaklist Lookup Performance (gh-95544)

gh-95302 seems to have introduced a small performance regression. Here we make some minor changes to recover that lost performance.

files:
M Include/internal/pycore_object.h
M Modules/gcmodule.c
M Objects/typeobject.c

diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
index 173d36784cf1..b3b0b8464808 100644
--- a/Include/internal/pycore_object.h
+++ b/Include/internal/pycore_object.h
@@ -217,6 +217,16 @@ extern void _Py_PrintReferences(FILE *);
 extern void _Py_PrintReferenceAddresses(FILE *);
 #endif
 
+
+/* Return the *address* of the object's weaklist.  The address may be
+ * dereferenced to get the current head of the weaklist.  This is useful
+ * for iterating over the linked list of weakrefs, especially when the
+ * list is being modified externally (e.g. refs getting removed).
+ *
+ * The returned pointer should not be used to change the head of the list
+ * nor should it be used to add, remove, or swap any refs in the list.
+ * That is the sole responsibility of the code in weakrefobject.c.
+ */
 static inline PyObject **
 _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op)
 {
@@ -226,10 +236,33 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op)
                                                         (PyTypeObject *)op);
         return _PyStaticType_GET_WEAKREFS_LISTPTR(state);
     }
+    // Essentially _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET():
     Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
     return (PyObject **)((char *)op + offset);
 }
 
+/* This is a special case of _PyObject_GET_WEAKREFS_LISTPTR().
+ * Only the most fundamental lookup path is used.
+ * Consequently, static types should not be used.
+ *
+ * For static builtin types the returned pointer will always point
+ * to a NULL tp_weaklist.  This is fine for any deallocation cases,
+ * since static types are never deallocated and static builtin types
+ * are only finalized at the end of runtime finalization.
+ *
+ * If the weaklist for static types is actually needed then use
+ * _PyObject_GET_WEAKREFS_LISTPTR().
+ */
+static inline PyWeakReference **
+_PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(PyObject *op)
+{
+    assert(!PyType_Check(op) ||
+            ((PyTypeObject *)op)->tp_flags & Py_TPFLAGS_HEAPTYPE);
+    Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
+    return (PyWeakReference **)((char *)op + offset);
+}
+
+
 // Fast inlined version of PyObject_IS_GC()
 static inline int
 _PyObject_IS_GC(PyObject *obj)
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index dcd46feff0cc..97cb6e6e1efb 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -794,9 +794,12 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
         if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op)))
             continue;
 
-        /* It supports weakrefs.  Does it have any? */
-        wrlist = (PyWeakReference **)
-                                _PyObject_GET_WEAKREFS_LISTPTR(op);
+        /* It supports weakrefs.  Does it have any?
+         *
+         * This is never triggered for static types so we can avoid the
+         * (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR().
+         */
+        wrlist = _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(op);
 
         /* `op` may have some weakrefs.  March over the list, clear
          * all the weakrefs, and move the weakrefs with callbacks
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 1c3f8a9b775d..1f56a5866e3d 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -1505,11 +1505,15 @@ subtype_dealloc(PyObject *self)
            finalizers since they might rely on part of the object
            being finalized that has already been destroyed. */
         if (type->tp_weaklistoffset && !base->tp_weaklistoffset) {
-            /* Modeled after GET_WEAKREFS_LISTPTR() */
-            PyWeakReference **list = (PyWeakReference **) \
-                _PyObject_GET_WEAKREFS_LISTPTR(self);
-            while (*list)
+            /* Modeled after GET_WEAKREFS_LISTPTR().
+
+               This is never triggered for static types so we can avoid the
+               (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR(). */
+            PyWeakReference **list = \
+                _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(self);
+            while (*list) {
                 _PyWeakref_ClearRef(*list);
+            }
         }
     }