feature-select: optimize feature access to the feasible maximum

** fix: MSVC needs extra sign bit in basewrapper_p.h! Buglet? **

The new feature selection has some tiny overhead.
It is about two dict accesses plus a slot access for every
tp_(get|set)attro call.

The introduction of an explicit `__init_feature__` call allows to
optimize this overhead very nicely, because this init is done for
each __feature__ import:

First, we can remove that tiny overhead completely by not initializing
the feature_select module at all if no __feature__ import is used.

Second, we can optimize this access further by caching the current module
dict. If the dict is unchanged, then the last select_id can be used.
This reduces the overhead of frequent calls to a single slot access.

Third, we finally cache the select id in unused SbkObjectType bits.
That removes the last structure where repeated attribute lookup is used.

The overhead is therefore quite small when something is changed.
But typically these changes are infrequent. The majority of accesses
do change nothing, and this case is now quite much optimized.

Change-Id: I7d1a4611a1c19216fd9be8f04457bb18ebd52ab1
Reviewed-by: Christian Tismer <tismer@stackless.com>

10 files changed, 95 insertions, 22 deletions

diff --git a/sources/pyside2/PySide2/QtCore/typesystem_core_common.xml b/sources/pyside2/PySide2/QtCore/typesystem_core_common.xml
index 1fc0c8d23..62928f3cf 100644
--- a/sources/pyside2/PySide2/QtCore/typesystem_core_common.xml
+++ b/sources/pyside2/PySide2/QtCore/typesystem_core_common.xml
@@ -673,6 +673,10 @@
       <inject-code class="target" position="beginning" file="../glue/qtcore.cpp" snippet="qt-qflag"/>
   </add-function>
 
+  <add-function signature="__init_feature__()">
+      <inject-code class="target" position="beginning" file="../glue/qtcore.cpp" snippet="qt-init-feature"/>
+  </add-function>
+
   <add-function signature="qAbs(double)" return-type="double">
     <inject-code class="target" position="beginning" file="../glue/qtcore.cpp" snippet="qt-qabs"/>
   </add-function>
diff --git a/sources/pyside2/PySide2/glue/qtcore.cpp b/sources/pyside2/PySide2/glue/qtcore.cpp
index a0ca23662..6b23a9b79 100644
--- a/sources/pyside2/PySide2/glue/qtcore.cpp
+++ b/sources/pyside2/PySide2/glue/qtcore.cpp
@@ -595,6 +595,10 @@ PySide::runCleanupFunctions();
 %PYARG_0 = PySide::QEnum::QEnumMacro(%1, true);
 // @snippet qt-qflag
 
+// @snippet qt-init-feature
+PySide::Feature::init();
+// @snippet qt-init-feature
+
 // @snippet qt-pysideinit
 Shiboken::Conversions::registerConverterName(SbkPySide2_QtCoreTypeConverters[SBK_QSTRING_IDX], "unicode");
 Shiboken::Conversions::registerConverterName(SbkPySide2_QtCoreTypeConverters[SBK_QSTRING_IDX], "str");
diff --git a/sources/pyside2/libpyside/feature_select.cpp b/sources/pyside2/libpyside/feature_select.cpp
index bc4461366..c703d18c4 100644
--- a/sources/pyside2/libpyside/feature_select.cpp
+++ b/sources/pyside2/libpyside/feature_select.cpp
@@ -38,6 +38,7 @@
 ****************************************************************************/
 
 #include "feature_select.h"
+#include "pyside.h"
 
 #include <shiboken.h>
 #include <sbkstaticstrings.h>
@@ -124,21 +125,35 @@ namespace PySide { namespace Feature {
 
 using namespace Shiboken;
 
-static inline PyObject *getFeatureSelectID()
+typedef bool(*FeatureProc)(PyTypeObject *type, PyObject *prev_dict);
+
+static FeatureProc *featurePointer = nullptr;
+
+static PyObject *cached_globals = nullptr;
+static PyObject *last_select_id = nullptr;
+
+static PyObject *fast_id_array[256] = {};
+
+static inline PyObject *getFeatureSelectId()
 {
-    static PyObject *zero = PyInt_FromLong(0);
+    static PyObject *zero = fast_id_array[0];
     static PyObject *feature_dict = GetFeatureDict();
     // these things are all borrowed
     PyObject *globals = PyEval_GetGlobals();
     if (globals == nullptr)
         return zero;
+    if (globals == cached_globals)
+        return last_select_id;
+
     PyObject *modname = PyDict_GetItem(globals, PyMagicName::name());
     if (modname == nullptr)
         return zero;
-    PyObject *flag = PyDict_GetItem(feature_dict, modname);
-    if (flag == nullptr || !PyInt_Check(flag))  // int/long cheating
+    PyObject *select_id = PyDict_GetItem(feature_dict, modname);
+    if (select_id == nullptr || !PyInt_Check(select_id))  // int/long cheating
         return zero;
-    return flag;
+    cached_globals = globals;
+    last_select_id = select_id;
+    return select_id;
 }
 
 // Create a derived dict class
@@ -193,6 +208,21 @@ static inline PyObject *getSelectId(PyObject *dict)
     return select_id;
 }
 
+static inline void setCurrentSelectId(PyTypeObject *type, PyObject *select_id)
+{
+    SbkObjectType_SetReserved(type, PyInt_AsSsize_t(select_id));    // int/long cheating
+}
+
+static inline void setCurrentSelectId(PyTypeObject *type, int id)
+{
+    SbkObjectType_SetReserved(type, id);
+}
+
+static inline PyObject *getCurrentSelectId(PyTypeObject *type)
+{
+    return fast_id_array[SbkObjectType_GetReserved(type)];
+}
+
 static bool replaceClassDict(PyTypeObject *type)
 {
     /*
@@ -251,6 +281,7 @@ static bool moveToFeatureSet(PyTypeObject *type, PyObject *select_id)
         // This works because small numbers are singleton objects.
         if (current_id == select_id) {
             type->tp_dict = dict;
+            setCurrentSelectId(type, select_id);
             return true;
         }
     } while (dict != initial_dict);
@@ -258,10 +289,6 @@ static bool moveToFeatureSet(PyTypeObject *type, PyObject *select_id)
     return false;
 }
 
-typedef bool(*FeatureProc)(PyTypeObject *type, PyObject *prev_dict);
-
-static FeatureProc *featurePointer = nullptr;
-
 static bool createNewFeatureSet(PyTypeObject *type, PyObject *select_id)
 {
     /*
@@ -279,18 +306,19 @@ static bool createNewFeatureSet(PyTypeObject *type, PyObject *select_id)
     // make sure that small integers are cached
     assert(small_1 != nullptr && small_1 == small_2);
 
-    static auto zero = PyInt_FromLong(0);
+    static auto zero = fast_id_array[0];
     bool ok = moveToFeatureSet(type, zero);
     Q_UNUSED(ok);
     assert(ok);
 
     AutoDecRef prev_dict(type->tp_dict);
-    Py_INCREF(prev_dict);
+    Py_INCREF(prev_dict);   // keep the first ref unchanged
     if (!addNewDict(type, select_id))
         return false;
-    auto id = PyInt_AsSsize_t(select_id);
+    auto id = PyInt_AsSsize_t(select_id);   // int/long cheating
     if (id == -1)
         return false;
+    setCurrentSelectId(type, id);
     FeatureProc *proc = featurePointer;
     for (int idx = id; *proc != nullptr; ++proc, idx >>= 1) {
         if (idx & 1) {
@@ -348,8 +376,8 @@ static inline PyObject *SelectFeatureSet(PyTypeObject *type)
         if (!replaceClassDict(type))
             return nullptr;
     }
-    PyObject *select_id = getFeatureSelectID();     // borrowed
-    AutoDecRef current_id(getSelectId(type->tp_dict));
+    PyObject *select_id = getFeatureSelectId();         // borrowed
+    PyObject *current_id = getCurrentSelectId(type);    // borrowed
     if (select_id != current_id) {
         PyObject *mro = type->tp_mro;
         Py_ssize_t idx, n = PyTuple_GET_SIZE(mro);
@@ -367,6 +395,8 @@ static inline PyObject *SelectFeatureSet(PyTypeObject *type)
 // For cppgenerator:
 void Select(PyObject *obj)
 {
+    if (featurePointer == nullptr)
+        return;
     auto type = Py_TYPE(obj);
     type->tp_dict = SelectFeatureSet(type);
 }
@@ -392,10 +422,26 @@ static FeatureProc featureProcArray[] = {
     nullptr
 };
 
+void finalize()
+{
+    for (int idx = 0; idx < 256; ++idx)
+        Py_DECREF(fast_id_array[idx]);
+}
+
 void init()
 {
-    featurePointer = featureProcArray;
-    initSelectableFeature(SelectFeatureSet);
+    // This function can be called multiple times.
+    static bool is_initialized = false;
+    if (!is_initialized) {
+        for (int idx = 0; idx < 256; ++idx)
+            fast_id_array[idx] = PyInt_FromLong(idx);
+        featurePointer = featureProcArray;
+        initSelectableFeature(SelectFeatureSet);
+        registerCleanupFunction(finalize);
+        is_initialized = true;
+    }
+    // Reset the cache. This is called at any "from __feature__ import".
+    cached_globals = nullptr;
 }
 
 //////////////////////////////////////////////////////////////////////////////
diff --git a/sources/pyside2/libpyside/pyside.cpp b/sources/pyside2/libpyside/pyside.cpp
index c1ddfc278..66e931164 100644
--- a/sources/pyside2/libpyside/pyside.cpp
+++ b/sources/pyside2/libpyside/pyside.cpp
@@ -50,7 +50,6 @@
 #include "pysidemetafunction_p.h"
 #include "pysidemetafunction.h"
 #include "dynamicqmetaobject.h"
-#include "feature_select.h"
 
 #include <autodecref.h>
 #include <basewrapper.h>
@@ -94,7 +93,6 @@ void init(PyObject *module)
     MetaFunction::init(module);
     // Init signal manager, so it will register some meta types used by QVariant.
     SignalManager::instance();
-    Feature::init();
     initQApp();
 }
 
diff --git a/sources/pyside2/tests/QtCore/multiple_feature_test.py b/sources/pyside2/tests/QtCore/multiple_feature_test.py
index 26488326c..351090382 100644
--- a/sources/pyside2/tests/QtCore/multiple_feature_test.py
+++ b/sources/pyside2/tests/QtCore/multiple_feature_test.py
@@ -48,7 +48,8 @@ from init_paths import init_test_paths
 init_test_paths(False)
 
 from PySide2 import QtCore
-from PySide2.support.__feature__ import _really_all_feature_names
+from PySide2.support.__feature__ import (
+    _really_all_feature_names, pyside_feature_dict)
 from textwrap import dedent
 
 """
@@ -67,7 +68,6 @@ class FeaturesTest(unittest.TestCase):
         """
         Test for all 256 possible combinations of `__feature__` imports.
         """
-        global __name__
 
         def tst_bit0(flag, self):
             if flag == 0:
@@ -108,9 +108,10 @@ class FeaturesTest(unittest.TestCase):
                      tst_bit4, tst_bit5, tst_bit6, tst_bit7]
 
         for idx in range(0x100):
-            __name__ = "feature_{:02x}".format(idx)
+            pyside_feature_dict.clear()
+            config = "feature_{:02x}".format(idx)
             print()
-            print("--- Feature Test Module `{}` ---".format(__name__))
+            print("--- Feature Test Config `{}` ---".format(config))
             print("Imports:")
             for bit in range(8):
                 if idx & 1 << bit:
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index b10074f79..9182b76e2 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -5690,6 +5690,7 @@ bool CppGenerator::finishGeneration()
         s << includeQDebug;
         s << "#include <pyside.h>\n";
         s << "#include <pysideqenum.h>\n";
+        s << "#include <feature_select.h>\n";
         s << "#include <qapp_macro.h>\n";
     }
 
diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp
index cec2650b7..0d2a1975f 100644
--- a/sources/shiboken2/libshiboken/basewrapper.cpp
+++ b/sources/shiboken2/libshiboken/basewrapper.cpp
@@ -574,6 +574,17 @@ static int SbkObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *val
     return PyObject_GenericSetAttr(obj, name, value);
 }
 
+// Caching the select Id.
+int SbkObjectType_GetReserved(PyTypeObject *type)
+{
+    return PepType_SOTP(reinterpret_cast<SbkObjectType *>(type))->pyside_reserved_bits;
+}
+
+void SbkObjectType_SetReserved(PyTypeObject *type, int value)
+{
+    PepType_SOTP(reinterpret_cast<SbkObjectType *>(type))->pyside_reserved_bits = value;
+}
+
 //
 //////////////////////////////////////////////////////////////////////////////
 
diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h
index 47ea89577..b233c02b4 100644
--- a/sources/shiboken2/libshiboken/basewrapper.h
+++ b/sources/shiboken2/libshiboken/basewrapper.h
@@ -100,6 +100,10 @@ LIBSHIBOKEN_API void initSelectableFeature(SelectableFeatureHook func);
 // PYSIDE-1019: Publish the start of setattro.
 LIBSHIBOKEN_API void SbkObject_NotifySetAttr(PyObject *obj, PyObject *name, PyObject *value);
 
+// PYSIDE-1019: Get access to PySide reserved bits.
+LIBSHIBOKEN_API int SbkObjectType_GetReserved(PyTypeObject *type);
+LIBSHIBOKEN_API void SbkObjectType_SetReserved(PyTypeObject *type, int value);
+
 
 extern LIBSHIBOKEN_API PyTypeObject *SbkObjectType_TypeF(void);
 extern LIBSHIBOKEN_API SbkObjectType *SbkObject_TypeF(void);
diff --git a/sources/shiboken2/libshiboken/basewrapper_p.h b/sources/shiboken2/libshiboken/basewrapper_p.h
index 56a647b21..685b4fbe5 100644
--- a/sources/shiboken2/libshiboken/basewrapper_p.h
+++ b/sources/shiboken2/libshiboken/basewrapper_p.h
@@ -138,6 +138,8 @@ struct SbkObjectTypePrivate
     // TODO-CONVERTERS: to be deprecated/removed
     int type_behaviour : 2;
     int delete_in_main_thread : 1;
+    /// PYSIDE-1019: Caching the current select Id
+    int pyside_reserved_bits : 9;   // MSVC needs the sign bit! Buglet?
     /// C++ name
     char *original_name;
     /// Type user data
diff --git a/sources/shiboken2/shibokenmodule/files.dir/shibokensupport/__feature__.py b/sources/shiboken2/shibokenmodule/files.dir/shibokensupport/__feature__.py
index d088ea41b..ca712bd9b 100644
--- a/sources/shiboken2/shibokenmodule/files.dir/shibokensupport/__feature__.py
+++ b/sources/shiboken2/shibokenmodule/files.dir/shibokensupport/__feature__.py
@@ -94,6 +94,8 @@ Note: This are two imports.
 # XXX build an improved C version
 def _import(name, *args, **kwargs):
     if name == "__feature__" and args[2]:
+        # Initialize feature (multiple times allowed) and clear cache.
+        sys.modules["PySide2.QtCore"].__init_feature__()
         # This is an `import from` statement that corresponds to `IMPORT_NAME`.
         # The following `IMPORT_FROM` will handle errors. (Confusing, ofc.)
         flag = 0