From a75527289b0d2bbb3a13d34e24fbce1be42601f9 Mon Sep 17 00:00:00 2001 From: Christian Tismer Date: Thu, 5 Mar 2020 18:22:09 +0100 Subject: Avoid the GIL in SignalManager::retrieveMetaObject After massive GIL savings in the generated code, there still exists a place where a lot of repeated GIL acquirements are done. It was observed that up to 24 consecutive calls to retrieveMetaObject(self) were made, all with the same value for 'self'. The code in question was: (1) Shiboken::GilState gil; (2) MetaObjectBuilder *builder = metaBuilderFromDict( \ reinterpret_cast(self)->ob_dict); (3) if (!builder) (4) builder = &(retrieveTypeUserData(self)->mo); (5) (6) return builder->update(); An exact analysis of the code showed that the GIL usage (1) could be moved out of the function into a deeper function that does updates in a branch (6). Function retrieveTypeUserData does not involve the Python interpreter at all (4). It took some time until it was proven that access to some special Python dictionary cannot reach the Python interpreter and therefore does not need the GIL as well (2). This replaces the abandoned effort to write a "Lazy GIL". It worked great for the example program, but had problems with some never finishing tests. After all, this solution is much simpler and works perfectly well. More effort seems not to be necessary to handle the GIL. Task-number: PYSIDE-803 Change-Id: I439009ff933fc6f498beb0c7f1523b6f985afda8 Reviewed-by: Friedemann Kleint --- sources/pyside2/libpyside/dynamicqmetaobject.cpp | 4 ++++ sources/pyside2/libpyside/signalmanager.cpp | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/sources/pyside2/libpyside/dynamicqmetaobject.cpp b/sources/pyside2/libpyside/dynamicqmetaobject.cpp index 2c5a0ee05..51e6598b3 100644 --- a/sources/pyside2/libpyside/dynamicqmetaobject.cpp +++ b/sources/pyside2/libpyside/dynamicqmetaobject.cpp @@ -413,6 +413,10 @@ const QMetaObject *MetaObjectBuilderPrivate::update() if (!m_builder) return m_baseObject; if (m_cachedMetaObjects.empty() || m_dirty) { + // PYSIDE-803: The dirty branch needs to be protected by the GIL. + // This was moved from SignalManager::retrieveMetaObject to here, + // which is only the update in "return builder->update()". + Shiboken::GilState gil; m_cachedMetaObjects.push_back(m_builder->toMetaObject()); checkMethodOrder(m_cachedMetaObjects.back()); m_dirty = false; diff --git a/sources/pyside2/libpyside/signalmanager.cpp b/sources/pyside2/libpyside/signalmanager.cpp index c21a3e565..01b347a3d 100644 --- a/sources/pyside2/libpyside/signalmanager.cpp +++ b/sources/pyside2/libpyside/signalmanager.cpp @@ -550,6 +550,12 @@ bool SignalManager::registerMetaMethod(QObject *source, const char *signature, Q static MetaObjectBuilder *metaBuilderFromDict(PyObject *dict) { + // PYSIDE-803: The dict in this function is the ob_dict of an SbkObject. + // The "metaObjectAttr" entry is only handled in this file. There is no + // way in this function to involve the interpreter. Therefore, we need + // no GIL. + // Note that "SignalManager::registerMetaMethodGetIndex" has write actions + // that might involve the interpreter, but in that context the GIL is held. if (!dict || !PyDict_Contains(dict, metaObjectAttr)) return nullptr; @@ -605,7 +611,14 @@ int SignalManager::registerMetaMethodGetIndex(QObject *source, const char *signa const QMetaObject *SignalManager::retrieveMetaObject(PyObject *self) { - Shiboken::GilState gil; + // PYSIDE-803: Avoid the GIL in SignalManager::retrieveMetaObject + // This function had the GIL. We do not use the GIL unless we have to. + // metaBuilderFromDict accesses a Python dict, but in that context there + // is no way to reach the interpreter, see "metaBuilderFromDict". + // + // The update function is MetaObjectBuilderPrivate::update in + // dynamicmetaobject.c . That function now uses the GIL when the + // m_dirty flag is set. Q_ASSERT(self); MetaObjectBuilder *builder = metaBuilderFromDict(reinterpret_cast(self)->ob_dict); -- cgit v1.2.3