From ee48c7d54147ae906776b9f6f96e8920e097d0c4 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Sat, 20 Mar 2021 12:12:05 -0700 Subject: [PATCH] bpo-43452: Micro-optimizations to PyType_Lookup (GH-24804) The common case going through _PyType_Lookup is to have a cache hit. There are some small tweaks that can make this a little cheaper: * The name field identity is used for a cache hit and is kept alive by the cache. So there's no need to read the hash code o the name - instead, the address can be used as the hash. * There's no need to check if the name is cachable on the lookup either, it probably is, and if it is, it'll be in the cache. * If we clear the version tag when invalidating a type then we don't actually need to check for a valid version tag bit. --- .../2021-03-20-01-21-37.bpo-43452.tDVJkc.rst | 1 + Objects/typeobject.c | 51 +++++++++---------- 2 files changed, 24 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-03-20-01-21-37.bpo-43452.tDVJkc.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-03-20-01-21-37.bpo-43452.tDVJkc.rst b/Misc/NEWS.d/next/Core and Builtins/2021-03-20-01-21-37.bpo-43452.tDVJkc.rst new file mode 100644 index 00000000000..c2fe10afdee --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-03-20-01-21-37.bpo-43452.tDVJkc.rst @@ -0,0 +1 @@ +Added micro-optimizations to ``_PyType_Lookup()`` to improve cache lookup performance in the common case of cache hits. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 650e4144f16..2b6ff59e5fd 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -32,8 +32,7 @@ class object "PyObject *" "&PyBaseObject_Type" & ((1 << MCACHE_SIZE_EXP) - 1)) #define MCACHE_HASH_METHOD(type, name) \ - MCACHE_HASH((type)->tp_version_tag, \ - ((PyASCIIObject *)(name))->hash) + MCACHE_HASH((type)->tp_version_tag, ((Py_ssize_t)(name)) >> 3) #define MCACHE_CACHEABLE_NAME(name) \ PyUnicode_CheckExact(name) && \ PyUnicode_IS_READY(name) && \ @@ -338,6 +337,7 @@ PyType_Modified(PyTypeObject *type) } } type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG; + type->tp_version_tag = 0; /* 0 is not a valid version tag */ } static void @@ -396,6 +396,7 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { Py_XDECREF(type_mro_meth); type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| Py_TPFLAGS_VALID_VERSION_TAG); + type->tp_version_tag = 0; /* 0 is not a valid version tag */ } static int @@ -3351,18 +3352,15 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) PyObject *res; int error; - if (MCACHE_CACHEABLE_NAME(name) && - _PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { - /* fast path */ - unsigned int h = MCACHE_HASH_METHOD(type, name); - struct type_cache *cache = get_type_cache(); - struct type_cache_entry *entry = &cache->hashtable[h]; - if (entry->version == type->tp_version_tag && entry->name == name) { + unsigned int h = MCACHE_HASH_METHOD(type, name); + struct type_cache *cache = get_type_cache(); + struct type_cache_entry *entry = &cache->hashtable[h]; + if (entry->version == type->tp_version_tag && + entry->name == name) { #if MCACHE_STATS - cache->hits++; + cache->hits++; #endif - return entry->value; - } + return entry->value; } /* We may end up clearing live exceptions below, so make sure it's ours. */ @@ -3385,24 +3383,21 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) return NULL; } - if (MCACHE_CACHEABLE_NAME(name)) { - struct type_cache *cache = get_type_cache(); - if (assign_version_tag(cache, type)) { - unsigned int h = MCACHE_HASH_METHOD(type, name); - struct type_cache_entry *entry = &cache->hashtable[h]; - entry->version = type->tp_version_tag; - entry->value = res; /* borrowed */ - assert(((PyASCIIObject *)(name))->hash != -1); + if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(cache, type)) { + h = MCACHE_HASH_METHOD(type, name); + struct type_cache_entry *entry = &cache->hashtable[h]; + entry->version = type->tp_version_tag; + entry->value = res; /* borrowed */ + assert(((PyASCIIObject *)(name))->hash != -1); #if MCACHE_STATS - if (entry->name != Py_None && entry->name != name) { - cache->collisions++; - } - else { - cache->misses++; - } -#endif - Py_SETREF(entry->name, Py_NewRef(name)); + if (entry->name != Py_None && entry->name != name) { + cache->collisions++; } + else { + cache->misses++; + } +#endif + Py_SETREF(entry->name, Py_NewRef(name)); } return res; }