aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian Tismer <tismer@stackless.com>2020-02-11 10:12:32 +0100
committerChristian Tismer <tismer@stackless.com>2020-03-03 10:12:46 +0000
commit50f382579d1323817165d85bf88a394328a4e9a0 (patch)
treeb1f610626d0a6d17aacb3aa86372b44c10b2272c
parent4a8a2713433154aee21f1f83ef3e9e7dc9466275 (diff)
Avoid the GIL in non-overridden Methods
In order to get better performance, we try to avoid allocating the GIL when methods have no override with a Python function. Every class gets an associated bool array that records functions where no override was found. On second call of a function, the GIL is avoided by this flag. Update 2020-02-06: The result is very promising when combined with a drastic reduction of Py_(BEGIN|END)_ALLOW_THREAD macro calls. So this could become the solution when combined with a good reduction! The Python 2 bug was circumvented by not generating the additional Py_tp_setattro functions for class QQuickItem. Task-number: PYSIDE-803 Change-Id: I0fe36edc5610b2d51bbb05f1b7507beee5088c83 Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
-rw-r--r--sources/shiboken2/generator/shiboken2/cppgenerator.cpp65
-rw-r--r--sources/shiboken2/generator/shiboken2/cppgenerator.h8
-rw-r--r--sources/shiboken2/generator/shiboken2/headergenerator.cpp13
-rw-r--r--sources/shiboken2/generator/shiboken2/shibokengenerator.cpp27
-rw-r--r--sources/shiboken2/generator/shiboken2/shibokengenerator.h6
-rw-r--r--sources/shiboken2/libshiboken/bindingmanager.cpp8
6 files changed, 109 insertions, 18 deletions
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 64a574f9..b4991060 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -415,18 +415,17 @@ void CppGenerator::generateClass(QTextStream &s, GeneratorContext &classContext)
}
const AbstractMetaFunctionList &funcs = filterFunctions(metaClass);
+ int maxOverrides = 0;
+ writeCacheResetNative(s, metaClass);
for (const AbstractMetaFunction *func : funcs) {
const bool notAbstract = !func->isAbstract();
if ((func->isPrivate() && notAbstract && !visibilityModifiedToPrivate(func))
|| (func->isModifiedRemoved() && notAbstract))
continue;
- if (func->functionType() == AbstractMetaFunction::ConstructorFunction && !func->isUserAdded()) {
+ if (func->functionType() == AbstractMetaFunction::ConstructorFunction && !func->isUserAdded())
writeConstructorNative(s, func);
- } else if ((!avoidProtectedHack() || !metaClass->hasPrivateDestructor())
- && ((func->isVirtual() || func->isAbstract())
- && (func->attributes() & AbstractMetaAttributes::FinalCppMethod) == 0)) {
- writeVirtualMethodNative(s, func);
- }
+ else if (shouldWriteVirtualMethodNative(func))
+ writeVirtualMethodNative(s, func, maxOverrides++);
}
if (!avoidProtectedHack() || !metaClass->hasPrivateDestructor()) {
@@ -696,6 +695,14 @@ void CppGenerator::generateClass(QTextStream &s, GeneratorContext &classContext)
}
}
+void CppGenerator::writeCacheResetNative(QTextStream &s, const AbstractMetaClass *metaClass)
+{
+ Indentation indentation(INDENT);
+ s << "void " << wrapperName(metaClass) << "::resetPyMethodCache()\n{\n";
+ s << INDENT << "std::fill_n(m_PyMethodCache, sizeof(m_PyMethodCache) / sizeof(m_PyMethodCache[0]), false);\n";
+ s << "}\n\n";
+}
+
void CppGenerator::writeConstructorNative(QTextStream &s, const AbstractMetaFunction *func)
{
Indentation indentation(INDENT);
@@ -705,6 +712,7 @@ void CppGenerator::writeConstructorNative(QTextStream &s, const AbstractMetaFunc
writeFunctionCall(s, func);
s << "\n{\n";
const AbstractMetaArgument *lastArg = func->arguments().isEmpty() ? nullptr : func->arguments().constLast();
+ s << INDENT << "resetPyMethodCache();\n";
writeCodeSnips(s, func->injectedCodeSnips(), TypeSystem::CodeSnipPositionBeginning, TypeSystem::NativeCode, func, lastArg);
s << INDENT << "// ... middle\n";
writeCodeSnips(s, func->injectedCodeSnips(), TypeSystem::CodeSnipPositionEnd, TypeSystem::NativeCode, func, lastArg);
@@ -760,7 +768,9 @@ QString CppGenerator::getVirtualFunctionReturnTypeName(const AbstractMetaFunctio
return QString::fromLatin1("reinterpret_cast<PyTypeObject *>(Shiboken::SbkType< %1 >())->tp_name").arg(func->type()->typeEntry()->qualifiedCppName());
}
-void CppGenerator::writeVirtualMethodNative(QTextStream &s, const AbstractMetaFunction *func)
+void CppGenerator::writeVirtualMethodNative(QTextStream &s,
+ const AbstractMetaFunction *func,
+ int cacheIndex)
{
//skip metaObject function, this will be written manually ahead
if (usePySideExtensions() && func->ownerClass() && func->ownerClass()->isQObject() &&
@@ -835,6 +845,28 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s, const AbstractMetaFu
s << endl;
}
+ // PYSIDE-803: Build a boolean cache for unused overrides.
+ bool multi_line = retType == nullptr; // set to true when using instrumentation
+ s << INDENT << "if (m_PyMethodCache[" << cacheIndex << "])" << (multi_line ? " {\n" : "\n");
+ {
+ Indentation indentation(INDENT);
+ s << INDENT;
+ if (retType)
+ s << "return ";
+ if (!func->isAbstract()) {
+ s << "this->::" << func->implementingClass()->qualifiedCppName() << "::";
+ writeFunctionCall(s, func, Generator::VirtualCall);
+ } else {
+ if (retType)
+ s << ' ' << defaultReturnExpr.returnValue();
+ }
+ if (!retType)
+ s << ";\n" << INDENT << "return";
+ s << ";\n";
+ }
+ if (multi_line)
+ s << INDENT << "}\n";
+
s << INDENT << "Shiboken::GilState gil;\n";
// Get out of virtual method call if someone already threw an error.
@@ -867,7 +899,10 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s, const AbstractMetaFu
s << ' ' << defaultReturnExpr.returnValue();
} else {
s << INDENT << "gil.release();\n";
- s << INDENT;
+ if (useOverrideCaching(func->ownerClass())) {
+ s << INDENT << "m_PyMethodCache[" << cacheIndex << "] = true;\n";
+ s << INDENT;
+ }
if (retType)
s << "return ";
s << "this->::" << func->implementingClass()->qualifiedCppName() << "::";
@@ -877,7 +912,7 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s, const AbstractMetaFu
}
}
s << ";\n";
- s << INDENT<< "}\n\n";
+ s << INDENT << "}\n\n"; //WS
writeConversionRule(s, func, TypeSystem::TargetLangCode);
@@ -5235,6 +5270,16 @@ void CppGenerator::writeSetattroFunction(QTextStream &s, AttroCheck attroCheck,
Q_ASSERT(!context.forSmartPointer());
const AbstractMetaClass *metaClass = context.metaClass();
writeSetattroDefinition(s, metaClass);
+ // PYSIDE-803: Detect duck-punching; clear cache if a method is set.
+ if (attroCheck.testFlag(AttroCheckFlag::SetattroMethodOverride)
+ && ShibokenGenerator::shouldGenerateCppWrapper(metaClass)) {
+ s << INDENT << "if (value && PyCallable_Check(value)) {\n";
+ s << INDENT << " auto plain_inst = " << cpythonWrapperCPtr(metaClass, QLatin1String("self")) << ";\n";
+ s << INDENT << " auto inst = dynamic_cast<" << wrapperName(metaClass) << " *>(plain_inst);\n";
+ s << INDENT << " if (inst)\n";
+ s << INDENT << " inst->resetPyMethodCache();\n";
+ s << INDENT << "}\n";
+ }
if (attroCheck.testFlag(AttroCheckFlag::SetattroQObject)) {
s << INDENT << "Shiboken::AutoDecRef pp(reinterpret_cast<PyObject *>(PySide::Property::getObject(self, name)));\n";
s << INDENT << "if (!pp.isNull())\n";
@@ -5897,7 +5942,7 @@ void CppGenerator::writeParentChildManagement(QTextStream &s, const AbstractMeta
writeReturnValueHeuristics(s, func);
}
-void CppGenerator::writeReturnValueHeuristics(QTextStream &s, const AbstractMetaFunction *func, const QString &self)
+void CppGenerator::writeReturnValueHeuristics(QTextStream &s, const AbstractMetaFunction *func)
{
AbstractMetaType *type = func->type();
if (!useReturnValueHeuristic()
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.h b/sources/shiboken2/generator/shiboken2/cppgenerator.h
index e3bb2c9c..aee1fb7d 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.h
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.h
@@ -50,11 +50,15 @@ protected:
bool finishGeneration() override;
private:
+ void writeInitFunc(QTextStream &declStr, QTextStream &callStr,
+ const Indentor &indent, const QString &initFunctionName,
+ const TypeEntry *enclosingEntry = nullptr);
+ void writeCacheResetNative(QTextStream &s, const AbstractMetaClass *metaClass);
void writeConstructorNative(QTextStream &s, const AbstractMetaFunction *func);
void writeDestructorNative(QTextStream &s, const AbstractMetaClass *metaClass);
QString getVirtualFunctionReturnTypeName(const AbstractMetaFunction *func);
- void writeVirtualMethodNative(QTextStream &s, const AbstractMetaFunction *func);
+ void writeVirtualMethodNative(QTextStream &s, const AbstractMetaFunction *func, int cacheIndex);
void writeMetaObjectMethod(QTextStream &s, const AbstractMetaClass *metaClass);
void writeMetaCast(QTextStream &s, const AbstractMetaClass *metaClass);
@@ -309,7 +313,7 @@ private:
void writeParentChildManagement(QTextStream &s, const AbstractMetaFunction *func, bool userHeuristicForReturn);
bool writeParentChildManagement(QTextStream &s, const AbstractMetaFunction *func, int argIndex, bool userHeuristicPolicy);
- void writeReturnValueHeuristics(QTextStream &s, const AbstractMetaFunction *func, const QString &self = QLatin1String("self"));
+ void writeReturnValueHeuristics(QTextStream &s, const AbstractMetaFunction *func);
void writeInitQtMetaTypeFunctionBody(QTextStream &s, GeneratorContext &context) const;
/**
diff --git a/sources/shiboken2/generator/shiboken2/headergenerator.cpp b/sources/shiboken2/generator/shiboken2/headergenerator.cpp
index 6ae00f75..34bba408 100644
--- a/sources/shiboken2/generator/shiboken2/headergenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/headergenerator.cpp
@@ -139,10 +139,17 @@ void HeaderGenerator::generateClass(QTextStream &s, GeneratorContext &classConte
s << "\n{\npublic:\n";
const AbstractMetaFunctionList &funcs = filterFunctions(metaClass);
+ int maxOverrides = 0;
for (AbstractMetaFunction *func : funcs) {
- if ((func->attributes() & AbstractMetaAttributes::FinalCppMethod) == 0)
+ if ((func->attributes() & AbstractMetaAttributes::FinalCppMethod) == 0) {
writeFunction(s, func);
+ // PYSIDE-803: Build a boolean cache for unused overrides.
+ if (shouldWriteVirtualMethodNative(func))
+ maxOverrides++;
+ }
}
+ if (!maxOverrides)
+ maxOverrides = 1;
if (avoidProtectedHack() && metaClass->hasProtectedFields()) {
const AbstractMetaFieldList &fields = metaClass->fields();
@@ -182,6 +189,10 @@ void HeaderGenerator::generateClass(QTextStream &s, GeneratorContext &classConte
if (usePySideExtensions())
s << INDENT << "static void pysideInitQtMetaTypes();\n";
+ s << INDENT << "void resetPyMethodCache();\n";
+ s << "private:\n";
+ s << INDENT << "mutable bool m_PyMethodCache[" << maxOverrides << "];\n";
+
s << "};\n\n";
if (!innerHeaderGuard.isEmpty())
s << "# endif // SBK_" << innerHeaderGuard << "_H\n\n";
diff --git a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
index 86fca9c5..56904e44 100644
--- a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
@@ -322,6 +322,15 @@ bool ShibokenGenerator::shouldGenerateCppWrapper(const AbstractMetaClass *metaCl
return result;
}
+bool ShibokenGenerator::shouldWriteVirtualMethodNative(const AbstractMetaFunction *func)
+{
+ // PYSIDE-803: Extracted this because it is used multiple times.
+ const AbstractMetaClass *metaClass = func->ownerClass();
+ return (!avoidProtectedHack() || !metaClass->hasPrivateDestructor())
+ && ((func->isVirtual() || func->isAbstract())
+ && (func->attributes() & AbstractMetaAttributes::FinalCppMethod) == 0);
+}
+
void ShibokenGenerator::lookForEnumsInClassesNotToBeGenerated(AbstractMetaEnumList &enumList, const AbstractMetaClass *metaClass)
{
Q_ASSERT(metaClass);
@@ -371,6 +380,15 @@ QString ShibokenGenerator::wrapperName(const AbstractMetaType *metaType) const
return metaType->cppSignature();
}
+QString ShibokenGenerator::wrapperName(const TypeEntry *type) const
+{
+ QString name = type->name();
+ int pos = name.lastIndexOf(QLatin1String("::"));
+ if (pos >= 0)
+ name = name.remove(0, pos + 2);
+ return name + QLatin1String("Wrapper");
+}
+
QString ShibokenGenerator::fullPythonClassName(const AbstractMetaClass *metaClass)
{
QString fullClassName = metaClass->name();
@@ -2182,6 +2200,13 @@ bool ShibokenGenerator::injectedCodeUsesArgument(const AbstractMetaFunction *fun
return false;
}
+bool ShibokenGenerator::useOverrideCaching(const AbstractMetaClass *metaClass)
+{
+ return metaClass->isPolymorphic()
+ && !metaClass->typeEntry()->typeFlags().testFlag(ComplexTypeEntry::NoOverrideCaching);
+
+}
+
ShibokenGenerator::AttroCheck ShibokenGenerator::checkAttroFunctionNeeds(const AbstractMetaClass *metaClass) const
{
AttroCheck result;
@@ -2192,6 +2217,8 @@ ShibokenGenerator::AttroCheck ShibokenGenerator::checkAttroFunctionNeeds(const A
result |= AttroCheckFlag::GetattroOverloads;
if (usePySideExtensions() && metaClass->qualifiedCppName() == QLatin1String("QObject"))
result |= AttroCheckFlag::SetattroQObject;
+ if (useOverrideCaching(metaClass))
+ result |= AttroCheckFlag::SetattroMethodOverride;
}
return result;
}
diff --git a/sources/shiboken2/generator/shiboken2/shibokengenerator.h b/sources/shiboken2/generator/shiboken2/shibokengenerator.h
index 13419b79..24c1374a 100644
--- a/sources/shiboken2/generator/shiboken2/shibokengenerator.h
+++ b/sources/shiboken2/generator/shiboken2/shibokengenerator.h
@@ -72,6 +72,7 @@ public:
GetattroMask = 0x0F,
SetattroQObject = 0x10,
SetattroSmartPointer = 0x20,
+ SetattroMethodOverride = 0x40,
SetattroMask = 0xF0,
};
Q_DECLARE_FLAGS(AttroCheck, AttroCheckFlag);
@@ -193,6 +194,7 @@ protected:
/// Returns the top-most class that has multiple inheritance in the ancestry.
static const AbstractMetaClass *getMultipleInheritingClass(const AbstractMetaClass *metaClass);
+ static bool useOverrideCaching(const AbstractMetaClass *metaClass);
AttroCheck checkAttroFunctionNeeds(const AbstractMetaClass *metaClass) const;
/// Returns a list of methods of the given class where each one is part of a different overload with both static and non-static method.
@@ -212,6 +214,9 @@ protected:
/// Verifies if the class should have a C++ wrapper generated for it, instead of only a Python wrapper.
bool shouldGenerateCppWrapper(const AbstractMetaClass *metaClass) const;
+ /// Condition to call WriteVirtualMethodNative. Was extracted because also used to count these calls.
+ bool shouldWriteVirtualMethodNative(const AbstractMetaFunction *func);
+
/// Adds enums eligible for generation from classes/namespaces marked not to be generated.
static void lookForEnumsInClassesNotToBeGenerated(AbstractMetaEnumList &enumList, const AbstractMetaClass *metaClass);
/// Returns the enclosing class for an enum, or nullptr if it should be global.
@@ -219,6 +224,7 @@ protected:
QString wrapperName(const AbstractMetaClass *metaClass) const;
QString wrapperName(const AbstractMetaType *metaType) const;
+ QString wrapperName(const TypeEntry *type) const;
QString fullPythonClassName(const AbstractMetaClass *metaClass);
QString fullPythonFunctionName(const AbstractMetaFunction *func);
diff --git a/sources/shiboken2/libshiboken/bindingmanager.cpp b/sources/shiboken2/libshiboken/bindingmanager.cpp
index 725150e8..1f18ed60 100644
--- a/sources/shiboken2/libshiboken/bindingmanager.cpp
+++ b/sources/shiboken2/libshiboken/bindingmanager.cpp
@@ -37,6 +37,7 @@
**
****************************************************************************/
+#include "autodecref.h"
#include "basewrapper.h"
#include "basewrapper_p.h"
#include "bindingmanager.h"
@@ -288,7 +289,7 @@ PyObject *BindingManager::getOverride(const void *cptr, const char *methodName)
}
}
- PyObject *pyMethodName = Shiboken::String::fromCString(methodName);
+ Shiboken::AutoDecRef pyMethodName(Shiboken::String::fromCString(methodName));
PyObject *method = PyObject_GetAttr(reinterpret_cast<PyObject *>(wrapper), pyMethodName);
if (method && PyMethod_Check(method)
@@ -302,16 +303,13 @@ PyObject *BindingManager::getOverride(const void *cptr, const char *methodName)
auto *parent = reinterpret_cast<PyTypeObject *>(PyTuple_GET_ITEM(mro, i));
if (parent->tp_dict) {
defaultMethod = PyDict_GetItem(parent->tp_dict, pyMethodName);
- if (defaultMethod && PyMethod_GET_FUNCTION(method) != defaultMethod) {
- Py_DECREF(pyMethodName);
+ if (defaultMethod && PyMethod_GET_FUNCTION(method) != defaultMethod)
return method;
- }
}
}
}
Py_XDECREF(method);
- Py_DECREF(pyMethodName);
return nullptr;
}