Optimize InvalidateAttoptCacheCallback() and TypeCacheTypCallback()

These callbacks are receiving hash values as arguments, which doesn't allow
direct lookups for AttoptCacheHash and TypeCacheHash.  This is why subject
callbacks currently use full iteration over corresponding hashes.

This commit avoids full hash iteration in InvalidateAttoptCacheCallback(),
and TypeCacheTypCallback().  At first, we switch AttoptCacheHash and
TypeCacheHash to use same hash function as syscache.  As second, we
use hash_seq_init_with_hash_value() to iterate only hash entries with matching
hash value.

Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1%40sigaev.ru
Author: Teodor Sigaev
Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov
Reviewed-by: Andrei Lepikhov

diff --git a/src/backend/utils/cache/attoptcache.c b/src/backend/utils/cache/attoptcache.c
index af978ccd4b1ef1b237db2566a4777ef1c84c7329..259865d5b306ed81d050e3a81f0f8519fe272666 100644 (file)
--- a/src/backend/utils/cache/attoptcache.c
+++ b/src/backend/utils/cache/attoptcache.c
@@ -44,12 +44,10 @@ typedef struct
 
 /*
  * InvalidateAttoptCacheCallback
- *     Flush all cache entries when pg_attribute is updated.
+ *     Flush cache entry (or entries) when pg_attribute is updated.
  *
  * When pg_attribute is updated, we must flush the cache entry at least
- * for that attribute.  Currently, we just flush them all.  Since attribute
- * options are not currently used in performance-critical paths (such as
- * query execution), this seems OK.
+ * for that attribute.
  */
 static void
 InvalidateAttoptCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
@@ -57,7 +55,16 @@ InvalidateAttoptCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
    HASH_SEQ_STATUS status;
    AttoptCacheEntry *attopt;
 
-   hash_seq_init(&status, AttoptCacheHash);
+   /*
+    * By convention, zero hash value is passed to the callback as a sign that
+    * it's time to invalidate the whole cache. See sinval.c, inval.c and
+    * InvalidateSystemCachesExtended().
+    */
+   if (hashvalue == 0)
+       hash_seq_init(&status, AttoptCacheHash);
+   else
+       hash_seq_init_with_hash_value(&status, AttoptCacheHash, hashvalue);
+
    while ((attopt = (AttoptCacheEntry *) hash_seq_search(&status)) != NULL)
    {
        if (attopt->opts)
@@ -70,6 +77,18 @@ InvalidateAttoptCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
    }
 }
 
+/*
+ * Hash function compatible with two-arg system cache hash function.
+ */
+static uint32
+relatt_cache_syshash(const void *key, Size keysize)
+{
+   const AttoptCacheKey *ckey = key;
+
+   Assert(keysize == sizeof(*ckey));
+   return GetSysCacheHashValue2(ATTNUM, ckey->attrelid, ckey->attnum);
+}
+
 /*
  * InitializeAttoptCache
  *     Initialize the attribute options cache.
@@ -82,9 +101,17 @@ InitializeAttoptCache(void)
    /* Initialize the hash table. */
    ctl.keysize = sizeof(AttoptCacheKey);
    ctl.entrysize = sizeof(AttoptCacheEntry);
+
+   /*
+    * AttoptCacheEntry takes hash value from the system cache. For
+    * AttoptCacheHash we use the same hash in order to speedup search by hash
+    * value. This is used by hash_seq_init_with_hash_value().
+    */
+   ctl.hash = relatt_cache_syshash;
+
    AttoptCacheHash =
        hash_create("Attopt cache", 256, &ctl,
-                   HASH_ELEM | HASH_BLOBS);
+                   HASH_ELEM | HASH_FUNCTION);
 
    /* Make sure we've initialized CacheMemoryContext. */
    if (!CacheMemoryContext)

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index aa4720cb5985cae5fe02d592f56acffd165c78ef..0b9e60845b2411bafeb4d5edf0b0a48deb0ae914 100644 (file)
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -331,6 +331,16 @@ static dsa_pointer share_tupledesc(dsa_area *area, TupleDesc tupdesc,
                                   uint32 typmod);
 
 
+/*
+ * Hash function compatible with one-arg system cache hash function.
+ */
+static uint32
+type_cache_syshash(const void *key, Size keysize)
+{
+   Assert(keysize == sizeof(Oid));
+   return GetSysCacheHashValue1(TYPEOID, ObjectIdGetDatum(*(const Oid *) key));
+}
+
 /*
  * lookup_type_cache
  *
@@ -355,8 +365,16 @@ lookup_type_cache(Oid type_id, int flags)
 
        ctl.keysize = sizeof(Oid);
        ctl.entrysize = sizeof(TypeCacheEntry);
+
+       /*
+        * TypeEntry takes hash value from the system cache. For TypeCacheHash
+        * we use the same hash in order to speedup search by hash value. This
+        * is used by hash_seq_init_with_hash_value().
+        */
+       ctl.hash = type_cache_syshash;
+
        TypeCacheHash = hash_create("Type information cache", 64,
-                                   &ctl, HASH_ELEM | HASH_BLOBS);
+                                   &ctl, HASH_ELEM | HASH_FUNCTION);
 
        /* Also set up callbacks for SI invalidations */
        CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
@@ -407,8 +425,7 @@ lookup_type_cache(Oid type_id, int flags)
 
        /* These fields can never change, by definition */
        typentry->type_id = type_id;
-       typentry->type_id_hash = GetSysCacheHashValue1(TYPEOID,
-                                                      ObjectIdGetDatum(type_id));
+       typentry->type_id_hash = get_hash_value(TypeCacheHash, &type_id);
 
        /* Keep this part in sync with the code below */
        typentry->typlen = typtup->typlen;
@@ -2358,20 +2375,28 @@ TypeCacheTypCallback(Datum arg, int cacheid, uint32 hashvalue)
    TypeCacheEntry *typentry;
 
    /* TypeCacheHash must exist, else this callback wouldn't be registered */
-   hash_seq_init(&status, TypeCacheHash);
+
+   /*
+    * By convention, zero hash value is passed to the callback as a sign that
+    * it's time to invalidate the whole cache. See sinval.c, inval.c and
+    * InvalidateSystemCachesExtended().
+    */
+   if (hashvalue == 0)
+       hash_seq_init(&status, TypeCacheHash);
+   else
+       hash_seq_init_with_hash_value(&status, TypeCacheHash, hashvalue);
+
    while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
    {
-       /* Is this the targeted type row (or it's a total cache flush)? */
-       if (hashvalue == 0 || typentry->type_id_hash == hashvalue)
-       {
-           /*
-            * Mark the data obtained directly from pg_type as invalid.  Also,
-            * if it's a domain, typnotnull might've changed, so we'll need to
-            * recalculate its constraints.
-            */
-           typentry->flags &= ~(TCFLAGS_HAVE_PG_TYPE_DATA |
-                                TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS);
-       }
+       Assert(hashvalue == 0 || typentry->type_id_hash == hashvalue);
+
+       /*
+        * Mark the data obtained directly from pg_type as invalid.  Also, if
+        * it's a domain, typnotnull might've changed, so we'll need to
+        * recalculate its constraints.
+        */
+       typentry->flags &= ~(TCFLAGS_HAVE_PG_TYPE_DATA |
+                            TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS);
    }
 }