specialize builtins too

python · markshannon · Aug 17, 2021 · Aug 6, 2021 · Aug 6, 2021 · Aug 7, 2021
commit 2b014a22c0bda554be2769187a18e29ef2cd9f3f
diff --git a/Include/opcode.h b/Include/opcode.h
diff --git a/Lib/opcode.py b/Lib/opcode.py
@@ -234,7 +234,6 @@ def jabs_op(name, op):
     "LOAD_GLOBAL_MODULE",
     "LOAD_GLOBAL_BUILTIN",
     "LOAD_METHOD_ADAPTIVE",
-    "LOAD_METHOD_SPLIT_KEYS",
     "LOAD_METHOD_WITH_HINT",
 ]
 

@@ -4128,31 +4128,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             }            
         }
 
-        //case TARGET(LOAD_METHOD_SPLIT_KEYS): {
-        //    assert(cframe.use_tracing == 0);
-        //    PyObject *owner = TOP();
-        //    PyObject *res;
-        //    PyTypeObject *tp = Py_TYPE(owner);
-        //    SpecializedCacheEntry *caches = GET_CACHE();
-        //    _PyAdaptiveEntry *cache0 = &caches[0].adaptive;
-        //    _PyLoadAttrCache *cache1 = &caches[-1].load_attr;
-        //    assert(cache1->tp_version != 0);
-        //    DEOPT_IF(tp->tp_version_tag != cache1->tp_version, LOAD_METHOD);
-        //    assert(tp->tp_dictoffset > 0);
-        //    PyDictObject *dict = *(PyDictObject **)(((char *)owner) + tp->tp_dictoffset);
-        //    DEOPT_IF(dict == NULL, LOAD_METHOD);
-        //    assert(PyDict_CheckExact((PyObject *)dict));
-        //    DEOPT_IF(dict->ma_keys->dk_version != cache1->dk_version_or_hint, LOAD_METHOD);
-        //    res = dict->ma_values[cache0->index];
-        //    DEOPT_IF(res == NULL, LOAD_METHOD);
-        //    STAT_INC(LOAD_METHOD, hit);
-        //    record_cache_hit(cache0);
-        //    Py_INCREF(res);
-        //    SET_TOP(res);
-        //    Py_DECREF(owner);
-        //    DISPATCH();
-        //}
-
         case TARGET(LOAD_METHOD_WITH_HINT): {
             assert(cframe.use_tracing == 0);
             PyObject *self = TOP();
@@ -4165,13 +4140,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             assert(cache1->dk_version_or_hint != 0);
             assert(cache1->tp_version != 0);
             DEOPT_IF(self_cls->tp_version_tag != cache1->tp_version, LOAD_METHOD);
-            assert(self_cls->tp_dictoffset > 0);
+            assert(self_cls->tp_dictoffset >= 0);
 
             // ensure self.__dict__ didn't modify keys
             PyObject **dictptr = _PyObject_GetDictPtr(self);
             PyDictObject *dict = NULL;
-            // DEOPT_IF(dictptr == NULL || *dictptr == NULL, LOAD_METHOD);
 
+            // maybe todo: don't need this branch if
+            // LOAD_METHOD_WITH_HINT_NO_DICT exists
             if (dictptr != NULL && *dictptr != NULL) {
                 dict = (PyDictObject *)*dictptr;
                 assert(PyDict_CheckExact(dict));

diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h
diff --git a/Python/specialize.c b/Python/specialize.c
@@ -700,17 +700,18 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name,
         SPECIALIZATION_FAIL(LOAD_METHOD, owner_cls, name, "not a method");
         goto fail;
     }
-    if (owner_cls->tp_dictoffset > 0) {
-        PyObject **cls_dictptr = _PyObject_GetDictPtr((PyObject *)owner_cls);
-        if (cls_dictptr == NULL || *cls_dictptr == NULL ||
-            !PyDict_CheckExact(*cls_dictptr)) {
-            // Maybe a builtin type
-            SPECIALIZATION_FAIL(LOAD_METHOD, owner_cls, name,
-                "cls no dict or not a dict");
-            goto fail;
-        }
+    PyObject **cls_dictptr = _PyObject_GetDictPtr((PyObject *)owner_cls);
+    int cls_has_dict = (cls_dictptr != NULL &&
+                        *cls_dictptr != NULL &&
+                        PyDict_CheckExact(*cls_dictptr));
+    PyObject **owner_dictptr = _PyObject_GetDictPtr(owner);
+    int owner_has_dict = (owner_dictptr != NULL &&
+                          *owner_dictptr != NULL &&
+                          !PyDict_CheckExact(*owner_dictptr));
+    PyDictObject *cls_dict = cls_has_dict ? (PyDictObject *)*cls_dictptr : NULL;
+    PyDictObject *owner_dict = owner_has_dict ? (PyDictObject *)*owner_dictptr : NULL;
+    if (owner_cls->tp_dictoffset >= 0 && cls_has_dict) {
         // We found a type with a __dict__.
-        PyDictObject *cls_dict = (PyDictObject *)*cls_dictptr;
         if ((owner_cls->tp_flags & Py_TPFLAGS_HEAPTYPE)
             && cls_dict->ma_keys == ((PyHeapTypeObject*)owner_cls)->ht_cached_keys) {
             // Keys are shared. Rare for LOAD_METHOD.
@@ -719,12 +720,9 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name,
         }
 
         // if o.__dict__ changes, the method might be found in o.__dict__
-        // instead of type lookup. So record o.__dict__.
-        PyObject **owner_dictptr = _PyObject_GetDictPtr(owner);
+        // instead of old type lookup. So record o.__dict__.
         uint32_t keys_version = UINT32_MAX;
-        // o.__dict__ exists
-        if (*owner_dictptr != NULL && !PyDict_CheckExact(*owner_dictptr)) {
-            PyDictObject *owner_dict = (PyDictObject *)*owner_dictptr;
+        if (owner_has_dict) {
             // check if its an attr
             int is_attr = PyDict_Contains((PyObject *)owner_dict, name);
             if (is_attr < 0) {
@@ -757,19 +755,6 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name,
         goto success;
 
     }
-#if SPECIALIZATION_STATS
-    else if (owner_cls->tp_dictoffset == 0) {
-        PyObject **owner_dictptr = _PyObject_GetDictPtr(owner);
-        if (owner_dictptr == NULL ||
-            *owner_dictptr == NULL ||
-            !PyDict_CheckExact(*owner_dictptr)) {
-            SPECIALIZATION_FAIL(LOAD_METHOD, owner_cls, name, "builtin no dict");
-            goto fail;
-        }
-        SPECIALIZATION_FAIL(LOAD_METHOD, owner_cls, name, "builtin with dict");
-        goto fail;
-    }
-#endif
 fail:
     STAT_INC(LOAD_METHOD, specialization_failure);
     assert(!PyErr_Occurred());