Address partial review

python · corona10 · Dec 19, 2024 · Dec 8, 2024 · Dec 8, 2024 · Dec 8, 2024
commit 55aa2407ec7d602f790743dbc89e2401ee9cdff3
diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h
diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h
@@ -278,9 +278,7 @@ typedef int (*_py_validate_type)(PyTypeObject *);
 // and if the validation is passed, it will set the ``tp_version`` as valid
 // tp_version_tag from the ``ty``.
 extern int _PyType_Validate(PyTypeObject *ty, _py_validate_type validate, unsigned int *tp_version);
-extern PyObject *_PyType_GetItemFromCache(PyTypeObject *type);
-extern PyObject *_PyType_GetItemFromCacheWithVersion(PyTypeObject *type, uint32_t *version);
-extern int _PyType_CacheGetItemForSpecialization(PyTypeObject *type, PyObject *descriptor);
+extern int _PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor);
 
 #ifdef __cplusplus
 }

diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h
@@ -5693,16 +5693,12 @@ _PyType_CacheInitForSpecialization(PyHeapTypeObject *type, PyObject *init,
 }
 
 int
-_PyType_CacheGetItemForSpecialization(PyTypeObject *type, PyObject *descriptor)
+_PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor)
 {
     int ret = 0;
-    if (!type) {
-        return -1;
-    }
     BEGIN_TYPE_LOCK();
     // This pointer is invalidated by PyType_Modified (see the comment on
     // struct _specialization_cache):
-    PyHeapTypeObject *ht = (PyHeapTypeObject *)type;
     PyFunctionObject *func = (PyFunctionObject *)descriptor;
     uint32_t version = _PyFunction_GetVersionForCurrentState(func);
     if (!_PyFunction_IsVersionValid(version)) {
@@ -5716,40 +5712,6 @@ _PyType_CacheGetItemForSpecialization(PyTypeObject *type, PyObject *descriptor)
     return ret;
 }
 
-PyObject *
-_PyType_GetItemFromCache(PyTypeObject *type)
-{
-    PyObject *res = NULL;
-    BEGIN_TYPE_LOCK();
-    PyHeapTypeObject *ht = (PyHeapTypeObject *)type;
-    res = ht->_spec_cache.getitem;
-    if (res == NULL || !PyFunction_Check(res)) {
-        res = NULL;
-    }
-    END_TYPE_LOCK();
-    return res;
-}
-
-PyObject *
-_PyType_GetItemFromCacheWithVersion(PyTypeObject *type, uint32_t *version)
-{
-    PyObject *res = NULL;
-    BEGIN_TYPE_LOCK();
-    PyHeapTypeObject *ht = (PyHeapTypeObject *)type;
-    res = ht->_spec_cache.getitem;
-    if (res == NULL) {
-        goto end;
-    }
-    if (!PyFunction_Check(res)) {
-        res = NULL;
-        goto end;
-    }
-    *version = ht->_spec_cache.getitem_version;
-end:
-    END_TYPE_LOCK();
-    return res;
-}
-
 static void
 set_flags(PyTypeObject *self, unsigned long mask, unsigned long flags)
 {

@@ -867,10 +867,11 @@ dummy_func(
         op(_BINARY_SUBSCR_CHECK_FUNC, (container, unused -- container, unused)) {
             PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
             DEOPT_IF(!PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE));
-            uint32_t cached_version;
-            PyObject *getitem = _PyType_GetItemFromCacheWithVersion(tp, &cached_version);
+            PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
+            PyObject *getitem = FT_ATOMIC_LOAD_PTR_RELAXED(ht->_spec_cache.getitem);
             DEOPT_IF(getitem == NULL);
             assert(PyFunction_Check(getitem));
+            uint32_t cached_version = FT_ATOMIC_LOAD_UINT32_RELAXED(ht->_spec_cache.getitem_version);
             DEOPT_IF(((PyFunctionObject *)getitem)->func_version != cached_version);
             PyCodeObject *code = (PyCodeObject *)PyFunction_GET_CODE(getitem);
             assert(code->co_argcount == 2);
@@ -880,7 +881,8 @@ dummy_func(
 
         op(_BINARY_SUBSCR_INIT_CALL, (container, sub -- new_frame: _PyInterpreterFrame* )) {
             PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
-            PyObject *getitem = _PyType_GetItemFromCache(tp);
+            PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
+            PyObject *getitem = FT_ATOMIC_LOAD_PTR_RELAXED(ht->_spec_cache.getitem);
             DEOPT_IF(getitem == NULL);
             new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame);
             new_frame->localsplus[0] = container;

diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
diff --git a/Python/specialize.c b/Python/specialize.c
@@ -1780,7 +1780,9 @@ _Py_Specialize_BinarySubscr(
             SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_WRONG_NUMBER_ARGUMENTS);
             goto fail;
         }
-        if (_PyType_CacheGetItemForSpecialization(container_type, descriptor) < 0) {
+
+        PyHeapTypeObject *ht = (PyHeapTypeObject *)container_type;
+        if (_PyType_CacheGetItemForSpecialization(ht, descriptor) < 0) {
             SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_OUT_OF_VERSIONS);
             goto fail;
         }