Merge pull request #745 from kmod/perf2

Improve isinstance speed

from_cpython/Include/descrobject.h

@@ -36,7 +36,8 @@ struct wrapperbase {
 
 /* Flags for above struct */
 #define PyWrapperFlag_KEYWORDS 1 /* wrapper function takes keyword args */
-#define PyWrapperFlag_PYSTON 2   /* wrapper function is a Pyston function*/
+#define PyWrapperFlag_PYSTON   2 /* wrapper function is a Pyston function */
+#define PyWrapperFlag_BOOL     4 /* not really a wrapper, just set a bool field */
 
 /* Various kinds of descriptor objects */
 

from_cpython/Include/object.h

@@ -457,7 +457,7 @@ struct _typeobject {
     char _ics[32];
     void* _gcvisit_func;
     int _attrs_offset;
-    bool _flags[4];
+    bool _flags[6];
     void* _tpp_descr_get;
     void* _tpp_hasnext;
     void* _tpp_call;

microbenchmarks/bm_django.py

+"""
+This benchmark was adapted from
+https://bitbucket.org/pypy/benchmarks/src/34f06618ef7f29d72d9c19f1e3894607587edc76/unladen_swallow/performance/bm_django.py?at=default
+"""
+
+__author__ = "collinwinter@google.com (Collin Winter)"
+
+import os
+import sys
+sys.path.append(os.path.join(os.path.dirname(__file__), "../test/integration/django"))
+
+from django.conf import settings
+settings.configure()
+from django.template import Context, Template
+
+import time
+
+DJANGO_TMPL = Template("""<table>
+{% for row in table %}
+<tr>{% for col in row %}<td>{{ col|escape }}</td>{% endfor %}</tr>
+{% endfor %}
+</table>
+""")
+
+def test_django(count):
+    table = [xrange(50) for _ in xrange(50)]
+    context = Context({"table": table})
+
+    times = []
+    for _ in xrange(count):
+        t0 = time.time()
+        data = DJANGO_TMPL.render(context)
+        t1 = time.time()
+        times.append(t1 - t0)
+    return times
+
+test_django(15)

src/capi/abstract.cpp

@@ -404,10 +404,14 @@ static int recursive_isinstance(PyObject* inst, PyObject* cls) noexcept {
     } else if (PyType_Check(cls)) {
         retval = PyObject_TypeCheck(inst, (PyTypeObject*)cls);
         if (retval == 0) {
-            PyObject* c = PyObject_GetAttr(inst, __class__);
-            if (c == NULL) {
-                PyErr_Clear();
-            } else {
+            PyObject* c = NULL;
+            if (inst->cls->has___class__ || inst->cls->tp_getattr != object_cls->tp_getattr
+                || inst->cls->tp_getattro != object_cls->tp_getattro) {
+                c = PyObject_GetAttr(inst, __class__);
+                if (!c)
+                    PyErr_Clear();
+            }
+            if (c) {
                 if (c != (PyObject*)(inst->cls) && PyType_Check(c))
                     retval = PyType_IsSubtype((PyTypeObject*)c, (PyTypeObject*)cls);
                 Py_DECREF(c);
@@ -435,6 +439,8 @@ extern "C" int _PyObject_RealIsInstance(PyObject* inst, PyObject* cls) noexcept
 }
 
 extern "C" int PyObject_IsInstance(PyObject* inst, PyObject* cls) noexcept {
+    STAT_TIMER(t0, "us_timer_pyobject_isinstance", 20);
+
     static PyObject* name = NULL;
 
     /* Quick test for an exact match */
@@ -461,8 +467,15 @@ extern "C" int PyObject_IsInstance(PyObject* inst, PyObject* cls) noexcept {
     }
 
     if (!(PyClass_Check(cls) || PyInstance_Check(cls))) {
-        PyObject* checker;
-        checker = _PyObject_LookupSpecial(cls, "__instancecheck__", &name);
+        PyObject* checker = NULL;
+        if (cls->cls->has_instancecheck) {
+            checker = _PyObject_LookupSpecial(cls, "__instancecheck__", &name);
+            if (!checker && PyErr_Occurred())
+                return -1;
+
+            assert(checker);
+        }
+
         if (checker != NULL) {
             PyObject* res;
             int ok = -1;
@@ -478,8 +491,7 @@ extern "C" int PyObject_IsInstance(PyObject* inst, PyObject* cls) noexcept {
                 Py_DECREF(res);
             }
             return ok;
-        } else if (PyErr_Occurred())
-            return -1;
+        }
     }
     return recursive_isinstance(inst, cls);
 }

src/capi/typeobject.cpp

@@ -1488,6 +1488,8 @@ static slotdef slotdefs[]
                PyWrapperFlag_KEYWORDS),
         TPSLOT("__new__", tp_new, slot_tp_new, NULL, ""),
         TPSLOT("__del__", tp_del, slot_tp_del, NULL, ""),
+        FLSLOT("__class__", has___class__, NULL, NULL, "", PyWrapperFlag_BOOL),
+        FLSLOT("__instancecheck__", has_instancecheck, NULL, NULL, "", PyWrapperFlag_BOOL),
         TPPSLOT("__hasnext__", tpp_hasnext, slotTppHasnext, wrapInquirypred, "hasnext"),
 
         BINSLOT("__add__", nb_add, slot_nb_add, "+"),                             // [force clang-format to line break]
@@ -1668,6 +1670,22 @@ static const slotdef* update_one_slot(BoxedClass* type, const slotdef* p) noexce
 
     do {
         descr = typeLookup(type, p->name_strobj, NULL);
+
+        if (p->flags & PyWrapperFlag_BOOL) {
+            // We are supposed to iterate over each slotdef; for now just assert that
+            // there was only one:
+            assert((p + 1)->offset > p->offset);
+
+            static BoxedString* class_str = internStringImmortal("__class__");
+            if (p->name_strobj == class_str) {
+                if (descr == object_cls->getattr(class_str))
+                    descr = NULL;
+            }
+
+            *(bool*)ptr = (bool)descr;
+            return p + 1;
+        }
+
         if (descr == NULL) {
             if (ptr == (void**)&type->tp_iternext) {
                 specific = (void*)_PyObject_NextNotImplemented;

src/runtime/builtin_modules/builtins.cpp

@@ -361,11 +361,9 @@ Box* sorted(Box* obj, Box* cmp, Box* key, Box** args) {
 }
 
 Box* isinstance_func(Box* obj, Box* cls) {
-    STAT_TIMER(t0, "us_timer_isinstance_func", 10);
-
     int rtn = PyObject_IsInstance(obj, cls);
     if (rtn < 0)
-        checkAndThrowCAPIException();
+        throwCAPIException();
     return boxBool(rtn);
 }
 

src/runtime/inline/boxing.h

@@ -31,18 +31,6 @@ extern "C" inline Box* boxComplex(double r, double i) {
     return new BoxedComplex(r, i);
 }
 
-extern "C" inline Box* boxBool(bool b) __attribute__((visibility("default")));
-extern "C" inline Box* boxBool(bool b) {
-    Box* rtn = b ? True : False;
-    return rtn;
-}
-
-extern "C" inline Box* boxBoolNegated(bool b) __attribute__((visibility("default")));
-extern "C" inline Box* boxBoolNegated(bool b) {
-    Box* rtn = b ? False : True;
-    return rtn;
-}
-
 extern "C" inline bool unboxBool(Box* b) __attribute__((visibility("default")));
 extern "C" inline bool unboxBool(Box* b) {
     assert(b->cls == bool_cls);

src/runtime/objmodel.cpp

@@ -409,7 +409,9 @@ BoxedClass::BoxedClass(BoxedClass* base, gcvisit_func gc_visit, int attrs_offset
       attrs_offset(attrs_offset),
       is_constant(false),
       is_user_defined(is_user_defined),
-      is_pyston_class(true) {
+      is_pyston_class(true),
+      has___class__(false),
+      has_instancecheck(false) {
 
     // Zero out the CPython tp_* slots:
     memset(&tp_name, 0, (char*)(&tp_version_tag + 1) - (char*)(&tp_name));

src/runtime/types.h

@@ -134,8 +134,14 @@ extern "C" {
 extern BoxedModule* sys_module, *builtins_module, *math_module, *time_module, *thread_module;
 }
 
-extern "C" Box* boxBool(bool);
-extern "C" Box* boxBoolNegated(bool);
+extern "C" inline Box* boxBool(bool b) __attribute__((visibility("default")));
+extern "C" inline Box* boxBool(bool b) {
+    return b ? True : False;
+}
+extern "C" inline Box* boxBoolNegated(bool b) __attribute__((visibility("default")));
+extern "C" inline Box* boxBoolNegated(bool b) {
+    return b ? False : True;
+}
 extern "C" Box* boxInt(i64) __attribute__((visibility("default")));
 extern "C" i64 unboxInt(Box*);
 extern "C" Box* boxFloat(double d);
@@ -224,6 +230,9 @@ public:
     // that we can't rely on for extension classes.
     bool is_pyston_class;
 
+    bool has___class__; // Has a custom __class__ attribute (ie different from object's __class__ descriptor)
+    bool has_instancecheck;
+
     typedef bool (*pyston_inquiry)(Box*);
 
     // tpp_descr_get is currently just a cache only for the use of tp_descr_get, and shouldn't

test/tests/isinstance.py

+class M(type):
+    pass
+
+class C(object):
+    __metaclass__ = M
+
+class D(C):
+    pass
+
+c = C()
+d = D()
+i = 0
+
+print "Testing base case:"
+print isinstance(i, C), isinstance(c, C), isinstance(d, C), isinstance(c, int)
+print
+
+print "Testing custom metaclass instancecheck:"
+def m_instancecheck(self, obj):
+    print "m_instancecheck", type(self), type(obj)
+    return False
+M.__instancecheck__ = m_instancecheck
+print isinstance(i, C), isinstance(c, C), isinstance(d, C), isinstance(c, int)
+print
+
+print "Testing custom class instancecheck:"
+def c_instancecheck(obj):
+    print "c_instancecheck", type(obj)
+    return False
+C.__instancecheck__ = c_instancecheck
+print isinstance(i, C), isinstance(c, C), isinstance(d, C), isinstance(c, int)
+print
+
+del M.__instancecheck__
+del C.__instancecheck__
+
+print "Testing custom class getattribute:"
+def c_getattribute(self, attr):
+    print "c_getattribute", type(self), attr
+C.__getattribute__ = c_getattribute
+print isinstance(i, C), isinstance(c, C), isinstance(d, C), isinstance(c, int)
+print
+
+print "Testing custom metaclass getattribute:"
+def m_getattribute(self, attr):
+    print "m_getattribute", type(self), attr
+M.__getattribute__ = m_getattribute
+print isinstance(i, C), isinstance(c, C), isinstance(d, C), isinstance(c, int)
+print
+
+del C.__getattribute__
+del M.__getattribute__
+
+print "Testing custom instance __class__"
+c2 = C()
+c2.__dict__['__class__'] = int
+print type(c2), c2.__class__ # should be the same; __class__ is a data decriptor
+print isinstance(c2, int), isinstance(c, int)
+print
+
+print "Testing custom class __class__:"
+class E(object):
+    __class__ = int
+e = E()
+print type(e), e.__class__ # should be different!
+print isinstance(e, int), isinstance(e, float), isinstance(e, E), isinstance(E, object)
+print
+
+print "Testing custom instance __class__ with custom class __class__:"
+# Unlike the non-custom-class version, a custom instance __class__ will now
+# take effect since the class __class__ is no longer a data descriptor.
+e.__dict__['__class__'] = float
+print type(e), e.__class__ # __class__ is now float!
+print isinstance(e, int), isinstance(e, float), isinstance(e, E), isinstance(E, object)
+print
+
+class M(type):
+    pass
+
+class M2(M):
+    pass
+
+class C(object):
+    __metaclass__ = M2
+
+class D(object):
+    pass
+
+# Setting instancecheck on a superclass better update the subclasses:
+print "checking superclass instancecheck:"
+print isinstance(1, C)
+M.__instancecheck__ =  m_instancecheck
+print isinstance(1, C)