Apply Mark's suggestions, fix up comments

python · markshannon · Aug 17, 2021 · Aug 6, 2021 · Aug 6, 2021 · Aug 7, 2021
commit 2f5cc63e3333eb346ede69b34125899ffda88872
diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
@@ -30,10 +30,9 @@ typedef struct {
     uint32_t builtin_keys_version;
 } _PyLoadGlobalCache;
 
-typedef union {
-    _PyAttrCache attr;
-    PyObject *meth;  /* borrowed */
-} _PyLoadMethodCache;
+typedef struct {
+    PyObject *obj;  /* borrowed */
+} _PyObjectCache;
 
 /* Add specialized versions of entries to this union.
  *
@@ -50,7 +49,7 @@ typedef union {
     _PyAdaptiveEntry adaptive;
     _PyAttrCache attr;
     _PyLoadGlobalCache load_global;
-    _PyLoadMethodCache load_method;
+    _PyObjectCache obj;
 } SpecializedCacheEntry;
 
 #define INSTRUCTIONS_PER_ENTRY (sizeof(SpecializedCacheEntry)/sizeof(_Py_CODEUNIT))

@@ -4254,34 +4254,30 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
    <
8000
span class='blob-code-inner blob-code-marker ' data-code-marker=" ">            PyObject *res = NULL;
             SpecializedCacheEntry *caches = GET_CACHE();
             _PyAdaptiveEntry *cache0 = &caches[0].adaptive;
-            _PyLoadMethodCache *cache1 = &caches[-1].load_method;
-            _PyLoadMethodCache *cache2 = &caches[-2].load_method;
+            _PyAttrCache *cache1 = &caches[-1].attr;
+            _PyObjectCache *cache2 = &caches[-2].obj;
 
-            DEOPT_IF(self_cls->tp_version_tag != cache1->attr.tp_version, LOAD_METHOD);
-            assert(cache1->attr.dk_version_or_hint != 0);
-            assert(cache1->attr.tp_version != 0);
+            DEOPT_IF(self_cls->tp_version_tag != cache1->tp_version, LOAD_METHOD);
+            assert(cache1->dk_version_or_hint != 0);
+            assert(cache1->tp_version != 0);
             assert(self_cls->tp_dictoffset >= 0);
             assert(Py_TYPE(self_cls)->tp_dictoffset > 0);
 
-            PyObject *name = GETITEM(names, cache0->original_oparg);
             // inline version of _PyObject_GetDictPtr for offset >= 0
-            PyObject **dictptr = self_cls->tp_dictoffset != 0 ?
-                (PyObject **) ((char *)self + self_cls->tp_dictoffset) : NULL;
-            PyDictObject *dict = NULL;
-
-            // ensure self.__dict__ didn't modify keys
-            if (dictptr != NULL && *dictptr != NULL) {
-                dict = (PyDictObject *)*dictptr;
-                assert(PyDict_CheckExact(dict));
-                DEOPT_IF(dict->ma_keys->dk_version !=
-                    cache1->attr.dk_version_or_hint, LOAD_METHOD);
-            } // don't care if owner has no dict, could be builtin or __slots__
-
-            res = cache2->meth;
-            assert(res != NULL);
-            assert(_PyType_HasFeature(Py_TYPE(res), Py_TPFLAGS_METHOD_DESCRIPTOR));
+            PyObject *dict = self_cls->tp_dictoffset != 0 ?
+                *(PyObject **) ((char *)self + self_cls->tp_dictoffset) : NULL;
+
+            // Ensure self.__dict__ didn't modify keys.
+            // Don't care if self has no dict, it could be builtin or __slots__.
+            DEOPT_IF(dict != NULL &&
+                ((PyDictObject *)dict)->ma_keys->dk_version !=
+                cache1->dk_version_or_hint, LOAD_METHOD);
+
             STAT_INC(LOAD_METHOD, hit);
             record_cache_hit(cache0);
+            res = cache2->obj;
            assert(res != NULL);
+            assert(_PyType_HasFeature(Py_TYPE(res), Py_TPFLAGS_METHOD_DESCRIPTOR));
             Py_INCREF(res);
             SET_TOP(res);
             PUSH(self);

diff --git a/Python/specialize.c b/Python/specialize.c
@@ -232,7 +232,7 @@ static uint8_t adaptive_opcodes[256] = {
 static uint8_t cache_requirements[256] = {
     [LOAD_ATTR] = 2, /* _PyAdaptiveEntry and _PyAttrCache */
     [LOAD_GLOBAL] = 2, /* _PyAdaptiveEntry and _PyLoadGlobalCache */
-    [LOAD_METHOD] = 3, /* _PyAdaptiveEntry and 2 _PyLoadMethodCache */
+    [LOAD_METHOD] = 3, /* _PyAdaptiveEntry, _PyAttrCache and _PyObjectCache */
     [BINARY_SUBSCR] = 0,
     [STORE_ATTR] = 2, /* _PyAdaptiveEntry and _PyAttrCache */
 };
@@ -793,8 +793,8 @@ int
 _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache)
 {
     _PyAdaptiveEntry *cache0 = &cache->adaptive;
-    _PyLoadMethodCache *cache1 = &cache[-1].load_method;
-    _PyLoadMethodCache *cache2 = &cache[-2].load_method;
+    _PyAttrCache *cache1 = &cache[-1].attr;
+    _PyObjectCache *cache2 = &cache[-2].obj;
 
     PyTypeObject *owner_cls = Py_TYPE(owner);
     if (PyModule_CheckExact(owner)) {
@@ -837,13 +837,13 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name,
         cls_has_dict &&
         owner_cls->tp_dictoffset >= 0) {
 
-        // if o.__dict__ changes, the method might be found in o.__dict__
-        // instead of old type lookup. So record o.__dict__.
+        // If o.__dict__ changes, the method might be found in o.__dict__
+        // instead of old type lookup. So record o.__dict__'s keys.
         uint32_t keys_version = UINT32_MAX;
         if (owner_has_dict) {
+            // _PyDictKeys_GetVersionForCurrentState isn't accurate for
+            // custom dict subclasses at the moment.
             if (!PyDict_CheckExact(owner_dict)) {
-                // _PyDictKeys_GetVersionForCurrentState isn't accurate for
-                // custom dict subclasses at the moment
                 SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_DICT_SUBCLASS);
                 goto fail;
             }
@@ -867,9 +867,9 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name,
         } // else owner is maybe a builtin with no dict, or __slots__
 
         // Borrowed. Check tp_version_tag before accessing in case it's deleted.
-        cache2->meth = descr;
-        cache1->attr.dk_version_or_hint = keys_version;
-        cache1->attr.tp_version = owner_cls->tp_version_tag;
+        cache2->obj = descr;
+        cache1->dk_version_or_hint = keys_version;
+        cache1->tp_version = owner_cls->tp_version_tag;
         *instr = _Py_MAKECODEUNIT(LOAD_METHOD_CACHED, _Py_OPARG(*instr));
         goto success;