From 53c7901078f0b28a768dd25202cc348f30f01d90 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 1 Aug 2022 16:11:20 -0600 Subject: [PATCH 1/5] Factor out _PyObject_GET_BASIC_WEAKREFS_LISTPTR(). --- Include/internal/pycore_object.h | 10 ++++++++-- Objects/typeobject.c | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 9f061d8b08772c..ccad0d67591943 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -217,6 +217,13 @@ extern void _Py_PrintReferences(FILE *); extern void _Py_PrintReferenceAddresses(FILE *); #endif +static inline PyObject ** +_PyObject_GET_BASIC_WEAKREFS_LISTPTR(PyObject *op) +{ + Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; + return (PyObject **)((char *)op + offset); +} + static inline PyObject ** _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) { @@ -226,8 +233,7 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) (PyTypeObject *)op); return _PyStaticType_GET_WEAKREFS_LISTPTR(state); } - Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; - return (PyObject **)((char *)op + offset); + return _PyObject_GET_BASIC_WEAKREFS_LISTPTR(op); } // Fast inlined version of PyObject_IS_GC() diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e4adf1c4e12f8c..1e5ec917cf5a2b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1509,7 +1509,7 @@ subtype_dealloc(PyObject *self) if (type->tp_weaklistoffset && !base->tp_weaklistoffset) { /* Modeled after GET_WEAKREFS_LISTPTR() */ PyWeakReference **list = (PyWeakReference **) \ - _PyObject_GET_WEAKREFS_LISTPTR(self); + _PyObject_GET_BASIC_WEAKREFS_LISTPTR(self); while (*list) _PyWeakref_ClearRef(*list); } From a02c1aeaa1b42503bcf51ce47b91c9bd818e38db Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 1 Aug 2022 16:12:42 -0600 Subject: [PATCH 2/5] Use _PyObject_GET_BASIC_WEAKREFS_LISTPTR() in performance-sensitive locations. --- Modules/gcmodule.c | 9 ++++++--- Objects/typeobject.c | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index dcd46feff0cc48..4cb77d53e098be 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 = (PyWeakReference **)_PyObject_GET_BASIC_WEAKREFS_LISTPTR(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 1e5ec917cf5a2b..a6658b5ea583b2 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1507,7 +1507,10 @@ 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() */ + /* 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 = (PyWeakReference **) \ _PyObject_GET_BASIC_WEAKREFS_LISTPTR(self); while (*list) From 23defa4b08293dc6fc4c239bcfe8f5e2f7c55262 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 2 Aug 2022 13:50:18 -0600 Subject: [PATCH 3/5] Add some comments. --- Include/internal/pycore_object.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index ccad0d67591943..52ea8f138f5ee7 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. + * + * For static builtin types this 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 such types is actually needed then use + * _PyObject_GET_WEAKREFS_LISTPTR(). + */ static inline PyObject ** _PyObject_GET_BASIC_WEAKREFS_LISTPTR(PyObject *op) { @@ -224,6 +234,7 @@ _PyObject_GET_BASIC_WEAKREFS_LISTPTR(PyObject *op) return (PyObject **)((char *)op + offset); } +/* Return the *address* of the object's weaklist. */ static inline PyObject ** _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) { From 13d047586a0594be806c0ab41b316b10170c3b1b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 2 Aug 2022 18:14:06 -0600 Subject: [PATCH 4/5] Update the comments. --- Include/internal/pycore_object.h | 50 +++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 52ea8f138f5ee7..eccecdee08ce59 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -217,25 +217,17 @@ extern void _Py_PrintReferences(FILE *); extern void _Py_PrintReferenceAddresses(FILE *); #endif -/* Return the *address* of the object's weaklist. - * - * For static builtin types this 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. + +/* 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). * - * If the weaklist for such types is actually needed then use - * _PyObject_GET_WEAKREFS_LISTPTR(). + * 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_BASIC_WEAKREFS_LISTPTR(PyObject *op) -{ - Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; - return (PyObject **)((char *)op + offset); -} - -/* Return the *address* of the object's weaklist. */ -static inline PyObject ** _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) { if (PyType_Check(op) && @@ -244,9 +236,33 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) (PyTypeObject *)op); return _PyStaticType_GET_WEAKREFS_LISTPTR(state); } - return _PyObject_GET_BASIC_WEAKREFS_LISTPTR(op); + // Essentially _PyObject_GET_BASIC_WEAKREFS_LISTPTR(): + 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 PyObject ** +_PyObject_GET_BASIC_WEAKREFS_LISTPTR(PyObject *op) +{ + assert(!PyType_Check(op) || + ((PyTypeObject *)op)->tp_flags & Py_TPFLAGS_HEAPTYPE); + Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; + return (PyObject **)((char *)op + offset); +} + + // Fast inlined version of PyObject_IS_GC() static inline int _PyObject_IS_GC(PyObject *obj) From 579604dd7d4a7b90a2e5b2ec996aa940cbf66309 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 4 Aug 2022 10:40:48 -0600 Subject: [PATCH 5/5] _PyObject_GET_BASIC_WEAKREFS_LISTPTR() -> _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(). --- Include/internal/pycore_object.h | 8 ++++---- Modules/gcmodule.c | 2 +- Objects/typeobject.c | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index eccecdee08ce59..a37d5d76827e22 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -236,7 +236,7 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) (PyTypeObject *)op); return _PyStaticType_GET_WEAKREFS_LISTPTR(state); } - // Essentially _PyObject_GET_BASIC_WEAKREFS_LISTPTR(): + // Essentially _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(): Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; return (PyObject **)((char *)op + offset); } @@ -253,13 +253,13 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) * If the weaklist for static types is actually needed then use * _PyObject_GET_WEAKREFS_LISTPTR(). */ -static inline PyObject ** -_PyObject_GET_BASIC_WEAKREFS_LISTPTR(PyObject *op) +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 (PyObject **)((char *)op + offset); + return (PyWeakReference **)((char *)op + offset); } diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 4cb77d53e098be..97cb6e6e1efb1f 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -799,7 +799,7 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old) * This is never triggered for static types so we can avoid the * (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR(). */ - wrlist = (PyWeakReference **)_PyObject_GET_BASIC_WEAKREFS_LISTPTR(op); + 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 a6658b5ea583b2..bfc3541432f7fa 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1511,10 +1511,11 @@ subtype_dealloc(PyObject *self) This is never triggered for static types so we can avoid the (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR(). */ - PyWeakReference **list = (PyWeakReference **) \ - _PyObject_GET_BASIC_WEAKREFS_LISTPTR(self); - while (*list) + PyWeakReference **list = \ + _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(self); + while (*list) { _PyWeakref_ClearRef(*list); + } } }