From d8e101775e3a0b4713d53b9cfa7491450b7766fa Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Jun 2021 12:14:58 +0100 Subject: [PATCH 1/5] Improve LOAD_ATTR specialization * Specialize obj.__class__ with LOAD_ATTR_SLOT * Specialize instance attribute lookup with attribute on class, provided attribute on class is not an overriding descriptor. --- Include/internal/pycore_code.h | 4 +- Python/specialize.c | 259 ++++++++++++++++++++++++--------- 2 files changed, 189 insertions(+), 74 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index a471c20265cbf7..53ff3b5d34623f 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -324,8 +324,8 @@ cache_backoff(_PyAdaptiveEntry *entry) { int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache); int _Py_Specialize_LoadGlobal(PyObject *globals, PyObject *builtins, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache); -#define SPECIALIZATION_STATS 0 -#define SPECIALIZATION_STATS_DETAILED 0 +#define SPECIALIZATION_STATS 1 +#define SPECIALIZATION_STATS_DETAILED 1 #if SPECIALIZATION_STATS diff --git a/Python/specialize.c b/Python/specialize.c index ca3dcdac4d0396..7509526f89abee 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -302,6 +302,8 @@ _Py_Quicken(PyCodeObject *code) { return 0; } + + static int specialize_module_load_attr( PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, @@ -349,6 +351,80 @@ specialize_module_load_attr( return 0; } + + +/* Attribute specialization */ + +typedef enum { + OVERRIDING, /* Is an overriding descriptor, and will remain so. */ + METHOD, /* Attribute has Py_TPFLAGS_METHOD_DESCRIPTOR set */ + PROPERTY, /* Is a property */ + OBJECT_SLOT, /* Is an object slot descriptor */ + OTHER_SLOT, /* Is a slot descriptor of another type */ + NON_OVERRIDING, /* Is another non-overriding descriptor, and is an instance of an immutable class*/ + NON_DESCRIPTOR, /* Is not a descriptor, and is an instance of an immutable class */ + MUTABLE, /* Instance of a mutable class; might, or might not, be a descriptor */ + ABSENT, /* Attribute is not present on the class */ + __CLASS__, /* __class__ attribute */ + GETATTRIBUTE_OVERRIDDEN /* __getattribute__ has been overridden */ +} DesciptorClassification; + +typedef enum { + SUCCESS = 0, + FAIL = -1 +} ReturnCode; + +typedef struct { + DesciptorClassification descriptor; + uint8_t index; /* The index of the key in the cached dict, or slot offset (if index < 256) */ + uint8_t in_dict; + PyObject *attribute; +} AttributeAnalysisResult; + +static DesciptorClassification +analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr) +{ + if (type->tp_getattro != PyObject_GenericGetAttr) { + *descr = NULL; + return GETATTRIBUTE_OVERRIDDEN; + } + PyObject *descriptor = _PyType_Lookup(type, name); + *descr = descriptor; + if (descriptor == NULL) { + return ABSENT; + } + PyTypeObject *desc_cls = Py_TYPE(descriptor); + if (!(desc_cls->tp_flags & Py_TPFLAGS_IMMUTABLETYPE)) { + return MUTABLE; + } + if (desc_cls->tp_descr_set) { + if (desc_cls == &PyMemberDescr_Type) { + PyMemberDescrObject *member = (PyMemberDescrObject *)descriptor; + struct PyMemberDef *dmem = member->d_member; + if (dmem->type == T_OBJECT_EX) { + return OBJECT_SLOT; + } + return OTHER_SLOT; + } + if (desc_cls == &PyProperty_Type) { + return PROPERTY; + } + if (PyUnicode_CompareWithASCIIString(name, "__class__") == 0) { + if (descriptor == _PyType_Lookup(&PyBaseObject_Type, name)) { + return __CLASS__; + } + } + return OVERRIDING; + } + if (desc_cls->tp_descr_get) { + if (desc_cls->tp_flags & Py_TPFLAGS_METHOD_DESCRIPTOR) { + return METHOD; + } + return NON_OVERRIDING; + } + return NON_DESCRIPTOR; +} + int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache) { @@ -362,94 +438,134 @@ _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, Sp goto success; } PyTypeObject *type = Py_TYPE(owner); - if (type->tp_getattro != PyObject_GenericGetAttr) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "__getattribute__ overridden"); - goto fail; - } if (type->tp_dict == NULL) { if (PyType_Ready(type) < 0) { return -1; } } - PyObject *descr = _PyType_Lookup(type, name); - if (descr != NULL) { - // We found an attribute with a data-like descriptor. - PyTypeObject *dtype = Py_TYPE(descr); - if (dtype != &PyMemberDescr_Type) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "not a member descriptor"); + PyObject *descr; + DesciptorClassification kind = analyze_descriptor(type, name, &descr); + switch(kind) { + case OVERRIDING: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "overriding descriptor"); goto fail; - } - // It's a slot - PyMemberDescrObject *member = (PyMemberDescrObject *)descr; - struct PyMemberDef *dmem = member->d_member; - if (dmem->type != T_OBJECT_EX) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "non-object slot"); + case METHOD: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "method"); goto fail; - } - Py_ssize_t offset = dmem->offset; - if (offset != (uint16_t)offset) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "offset out of range"); + case PROPERTY: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "property"); goto fail; + case OBJECT_SLOT: + { + PyMemberDescrObject *member = (PyMemberDescrObject *)descr; + struct PyMemberDef *dmem = member->d_member; + Py_ssize_t offset = dmem->offset; + if (offset != (uint16_t)offset) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "offset out of range"); + goto fail; + } + assert(dmem->type == T_OBJECT_EX); + assert(offset > 0); + cache0->index = (uint16_t)offset; + cache1->tp_version = type->tp_version_tag; + *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SLOT, _Py_OPARG(*instr)); + goto success; } - assert(offset > 0); - cache0->index = (uint16_t)offset; - cache1->tp_version = type->tp_version_tag; - *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SLOT, _Py_OPARG(*instr)); - goto success; - } - // No desciptor - if (type->tp_dictoffset <= 0) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "no dict or negative offset"); - goto fail; - } - PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset); - if (*dictptr == NULL || !PyDict_CheckExact(*dictptr)) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "no dict or not a dict"); + case __CLASS__: + { + Py_ssize_t offset = offsetof(PyObject, ob_type); + assert(offset == (uint16_t)offset); + cache0->index = (uint16_t)offset; + cache1->tp_version = type->tp_version_tag; + *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SLOT, _Py_OPARG(*instr)); + goto success; + } + case OTHER_SLOT: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "non-object slot"); + goto fail; + case MUTABLE: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "mutable class attribute"); + goto fail; + case GETATTRIBUTE_OVERRIDDEN: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "__getattribute__ overridden"); + goto fail; + case NON_OVERRIDING: + case NON_DESCRIPTOR: + case ABSENT: + break; + } + assert(kind == NON_OVERRIDING || kind == NON_DESCRIPTOR || kind == ABSENT); + // No desciptor, or non overriding. + if (type->tp_dictoffset < 0) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "negative offset"); goto fail; } - // We found an instance with a __dict__. - PyDictObject *dict = (PyDictObject *)*dictptr; - if ((type->tp_flags & Py_TPFLAGS_HEAPTYPE) - && dict->ma_keys == ((PyHeapTypeObject*)type)->ht_cached_keys - ) { - // Keys are shared - assert(PyUnicode_CheckExact(name)); - Py_hash_t hash = PyObject_Hash(name); - if (hash == -1) { - return -1; - } - PyObject *value; - Py_ssize_t index = _Py_dict_lookup(dict, name, hash, &value); - assert (index != DKIX_ERROR); - if (index != (uint16_t)index) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "index out of range"); + if (type->tp_dictoffset > 0) { + PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset); + if (*dictptr == NULL || !PyDict_CheckExact(*dictptr)) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "no dict or not a dict"); goto fail; } - uint32_t keys_version = _PyDictKeys_GetVersionForCurrentState(dict); - if (keys_version == 0) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "no more key versions"); - goto fail; + // We found an instance with a __dict__. + PyDictObject *dict = (PyDictObject *)*dictptr; + if ((type->tp_flags & Py_TPFLAGS_HEAPTYPE) + && dict->ma_keys == ((PyHeapTypeObject*)type)->ht_cached_keys + ) { + // Keys are shared + assert(PyUnicode_CheckExact(name)); + Py_hash_t hash = PyObject_Hash(name); + if (hash == -1) { + return -1; + } + PyObject *value; + Py_ssize_t index = _Py_dict_lookup(dict, name, hash, &value); + assert (index != DKIX_ERROR); + if (index != (uint16_t)index) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, + index < 0 ? "attribute not in dict" : "index out of range"); + goto fail; + } + uint32_t keys_version = _PyDictKeys_GetVersionForCurrentState(dict); + if (keys_version == 0) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "no more key versions"); + goto fail; + } + cache1->dk_version_or_hint = keys_version; + cache1->tp_version = type->tp_version_tag; + cache0->index = (uint16_t)index; + *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SPLIT_KEYS, _Py_OPARG(*instr)); + goto success; + } + else { + PyObject *value = NULL; + Py_ssize_t hint = + _PyDict_GetItemHint(dict, name, -1, &value); + if (hint != (uint32_t)hint) { + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "hint out of range"); + goto fail; + } + cache1->dk_version_or_hint = (uint32_t)hint; + cache1->tp_version = type->tp_version_tag; + *instr = _Py_MAKECODEUNIT(LOAD_ATTR_WITH_HINT, _Py_OPARG(*instr)); + goto success; } - cache1->dk_version_or_hint = keys_version; - cache1->tp_version = type->tp_version_tag; - cache0->index = (uint16_t)index; - *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SPLIT_KEYS, _Py_OPARG(*instr)); - goto success; } - else { - PyObject *value = NULL; - Py_ssize_t hint = - _PyDict_GetItemHint(dict, name, -1, &value); - if (hint != (uint32_t)hint) { - SPECIALIZATION_FAIL(LOAD_ATTR, Py_TYPE(owner), name, "hint out of range"); + assert(type->tp_dictoffset == 0); + /* No attribute in instance dictionary */ + switch(kind) { + case NON_OVERRIDING: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "non-overriding descriptor"); goto fail; - } - cache1->dk_version_or_hint = (uint32_t)hint; - cache1->tp_version = type->tp_version_tag; - *instr = _Py_MAKECODEUNIT(LOAD_ATTR_WITH_HINT, _Py_OPARG(*instr)); - goto success; + case NON_DESCRIPTOR: + /* To do -- Optimize this case */ + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "non descriptor"); + goto fail; + case ABSENT: + SPECIALIZATION_FAIL(LOAD_ATTR, type, name, "no attribute"); + goto fail; + default: + Py_UNREACHABLE(); } - fail: STAT_INC(LOAD_ATTR, specialization_failure); assert(!PyErr_Occurred()); @@ -462,7 +578,6 @@ _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, Sp return 0; } - int _Py_Specialize_LoadGlobal( PyObject *globals, PyObject *builtins, From 65524d103fc432b0abedcf4d2b45fcd62beb8f2f Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Jun 2021 12:37:22 +0100 Subject: [PATCH 2/5] Add stat for how many times the unquickened instruction has executed. --- Include/internal/pycore_code.h | 1 + Python/ceval.c | 2 ++ Python/specialize.c | 1 + 3 files changed, 4 insertions(+) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 53ff3b5d34623f..ebd65bb903baf3 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -336,6 +336,7 @@ typedef struct _stats { uint64_t deferred; uint64_t miss; uint64_t deopt; + uint64_t unquickened; #if SPECIALIZATION_STATS_DETAILED PyObject *miss_types; #endif diff --git a/Python/ceval.c b/Python/ceval.c index 79ec143f5ea327..8e87cca0b530b9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2805,6 +2805,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag) case TARGET(LOAD_GLOBAL): { PREDICTED(LOAD_GLOBAL); + STAT_INC(LOAD_GLOBAL, unquickened); PyObject *name = GETITEM(names, oparg); PyObject *v; if (PyDict_CheckExact(GLOBALS()) @@ -3289,6 +3290,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag) case TARGET(LOAD_ATTR): { PREDICTED(LOAD_ATTR); + STAT_INC(LOAD_ATTR, unquickened); PyObject *name = GETITEM(names, oparg); PyObject *owner = TOP(); PyObject *res = PyObject_GetAttr(owner, name); diff --git a/Python/specialize.c b/Python/specialize.c index 7509526f89abee..fe8c944f5a8f6c 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -47,6 +47,7 @@ print_stats(SpecializationStats *stats, const char *name) PRINT_STAT(name, deferred); PRINT_STAT(name, miss); PRINT_STAT(name, deopt); + PRINT_STAT(name, unquickened); #if SPECIALIZATION_STATS_DETAILED if (stats->miss_types == NULL) { return; From d5e7d20ed7da5218f37dc780deff16fa10002772 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Jun 2021 12:46:32 +0100 Subject: [PATCH 3/5] Turn stats off by default --- Include/internal/pycore_code.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index ebd65bb903baf3..1e78b928865e78 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -324,8 +324,8 @@ cache_backoff(_PyAdaptiveEntry *entry) { int _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache); int _Py_Specialize_LoadGlobal(PyObject *globals, PyObject *builtins, _Py_CODEUNIT *instr, PyObject *name, SpecializedCacheEntry *cache); -#define SPECIALIZATION_STATS 1 -#define SPECIALIZATION_STATS_DETAILED 1 +#define SPECIALIZATION_STATS 0 +#define SPECIALIZATION_STATS_DETAILED 0 #if SPECIALIZATION_STATS From 3c0b4094d1078d6272a371be2ba8c268d9c35123 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Jun 2021 16:07:27 +0100 Subject: [PATCH 4/5] Remove unused struct and enum --- Python/specialize.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index fe8c944f5a8f6c..f9e9dc5de8ceca 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -370,18 +370,6 @@ typedef enum { GETATTRIBUTE_OVERRIDDEN /* __getattribute__ has been overridden */ } DesciptorClassification; -typedef enum { - SUCCESS = 0, - FAIL = -1 -} ReturnCode; - -typedef struct { - DesciptorClassification descriptor; - uint8_t index; /* The index of the key in the cached dict, or slot offset (if index < 256) */ - uint8_t in_dict; - PyObject *attribute; -} AttributeAnalysisResult; - static DesciptorClassification analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr) { From 5d9eb10bbb1d57c0f764cb82f688e3710f332c47 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 21 Jun 2021 10:34:11 +0100 Subject: [PATCH 5/5] Don't use reserved identifier. --- Python/specialize.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index f9e9dc5de8ceca..a8ae09ff0e3839 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -366,7 +366,7 @@ typedef enum { NON_DESCRIPTOR, /* Is not a descriptor, and is an instance of an immutable class */ MUTABLE, /* Instance of a mutable class; might, or might not, be a descriptor */ ABSENT, /* Attribute is not present on the class */ - __CLASS__, /* __class__ attribute */ + DUNDER_CLASS, /* __class__ attribute */ GETATTRIBUTE_OVERRIDDEN /* __getattribute__ has been overridden */ } DesciptorClassification; @@ -400,7 +400,7 @@ analyze_descriptor(PyTypeObject *type, PyObject *name, PyObject **descr) } if (PyUnicode_CompareWithASCIIString(name, "__class__") == 0) { if (descriptor == _PyType_Lookup(&PyBaseObject_Type, name)) { - return __CLASS__; + return DUNDER_CLASS; } } return OVERRIDING; @@ -460,7 +460,7 @@ _Py_Specialize_LoadAttr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, Sp *instr = _Py_MAKECODEUNIT(LOAD_ATTR_SLOT, _Py_OPARG(*instr)); goto success; } - case __CLASS__: + case DUNDER_CLASS: { Py_ssize_t offset = offsetof(PyObject, ob_type); assert(offset == (uint16_t)offset);