Okay, I've applied this piece for now. Not sure I'll have much room
to look at the rest.
Thank you very much!
Rest of patches, rebased.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/From b30af080d7768c2fdb6198e2e40ef93419f60732 Mon Sep 17 00:00:00 2001
From: Teodor Sigaev <teo...@sigaev.ru>
Date: Fri, 15 Mar 2024 13:55:10 +0300
Subject: [PATCH 3/3] usage of hash_search_with_hash_value
---
src/backend/utils/cache/attoptcache.c | 39 ++++++++++++++++----
src/backend/utils/cache/typcache.c | 52 +++++++++++++++++++--------
2 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/src/backend/utils/cache/attoptcache.c b/src/backend/utils/cache/attoptcache.c
index af978ccd4b1..3a18b2e9a77 100644
--- 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 convection, zero hash value is passed to the callback as a sign
+ * that it's time to invalidate the 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 7936c3b46d0..9145088f44d 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -339,6 +339,15 @@ static TupleDesc find_or_make_matching_shared_tupledesc(TupleDesc tupdesc);
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
@@ -364,8 +373,15 @@ 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);
ctl.keysize = sizeof(Oid);
ctl.entrysize = sizeof(mapRelTypeEntry);
@@ -421,8 +437,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;
@@ -2429,20 +2444,27 @@ 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 convection, zero hash value is passed to the callback as a sign
+ * that it's time to invalidate the 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);
}
}
--
2.43.2
From 917d70a81ad37d366d73a4e3a9ed92212b4698ad Mon Sep 17 00:00:00 2001
From: Teodor Sigaev <teo...@sigaev.ru>
Date: Fri, 15 Mar 2024 13:52:50 +0300
Subject: [PATCH 2/3] hash_search_with_hash_value
---
src/backend/utils/hash/dynahash.c | 38 +++++++++++++++++++++++++++++++
src/include/utils/hsearch.h | 4 ++++
2 files changed, 42 insertions(+)
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 4080833df0f..e981298ea47 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1387,10 +1387,30 @@ hash_seq_init(HASH_SEQ_STATUS *status, HTAB *hashp)
status->hashp = hashp;
status->curBucket = 0;
status->curEntry = NULL;
+ status->hasHashvalue = false;
if (!hashp->frozen)
register_seq_scan(hashp);
}
+/*
+ * Same as above but scan by the given hash value.
+ * See also hash_seq_search().
+ */
+void
+hash_seq_init_with_hash_value(HASH_SEQ_STATUS *status, HTAB *hashp,
+ uint32 hashvalue)
+{
+ HASHBUCKET *bucketPtr;
+
+ hash_seq_init(status, hashp);
+
+ status->hasHashvalue = true;
+ status->hashvalue = hashvalue;
+
+ status->curBucket = hash_initial_lookup(hashp, hashvalue, &bucketPtr);
+ status->curEntry = *bucketPtr;
+}
+
void *
hash_seq_search(HASH_SEQ_STATUS *status)
{
@@ -1404,6 +1424,24 @@ hash_seq_search(HASH_SEQ_STATUS *status)
uint32 curBucket;
HASHELEMENT *curElem;
+ if (status->hasHashvalue)
+ {
+ /*
+ * Scan entries only in the current bucket because only this bucket can
+ * contain entries with the given hash value.
+ */
+ while ((curElem = status->curEntry) != NULL)
+ {
+ status->curEntry = curElem->link;
+ if (status->hashvalue != curElem->hashvalue)
+ continue;
+ return (void *) ELEMENTKEY(curElem);
+ }
+
+ hash_seq_term(status);
+ return NULL;
+ }
+
if ((curElem = status->curEntry) != NULL)
{
/* Continuing scan of curBucket... */
diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index da26941f6db..c99d74625f7 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -122,6 +122,8 @@ typedef struct
HTAB *hashp;
uint32 curBucket; /* index of current bucket */
HASHELEMENT *curEntry; /* current entry in bucket */
+ bool hasHashvalue; /* true if hashvalue was provided */
+ uint32 hashvalue; /* hashvalue to start seqscan over hash */
} HASH_SEQ_STATUS;
/*
@@ -141,6 +143,8 @@ extern bool hash_update_hash_key(HTAB *hashp, void *existingEntry,
const void *newKeyPtr);
extern long hash_get_num_entries(HTAB *hashp);
extern void hash_seq_init(HASH_SEQ_STATUS *status, HTAB *hashp);
+extern void hash_seq_init_with_hash_value(HASH_SEQ_STATUS *status, HTAB *hashp,
+ uint32 hashvalue);
extern void *hash_seq_search(HASH_SEQ_STATUS *status);
extern void hash_seq_term(HASH_SEQ_STATUS *status);
extern void hash_freeze(HTAB *hashp);
--
2.43.2
From 93d3ff32c7c09ec96e7649f831d508c2921b5b5b Mon Sep 17 00:00:00 2001
From: Teodor Sigaev <teo...@sigaev.ru>
Date: Fri, 15 Mar 2024 13:52:34 +0300
Subject: [PATCH 1/3] type cache
---
src/backend/utils/cache/typcache.c | 159 +++++++++++++++++++++--------
1 file changed, 115 insertions(+), 44 deletions(-)
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 0d4d0b0a154..7936c3b46d0 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -77,6 +77,15 @@
/* The main type cache hashtable searched by lookup_type_cache */
static HTAB *TypeCacheHash = NULL;
+/* The map from relation's oid to its type oid */
+typedef struct mapRelTypeEntry
+{
+ Oid typrelid;
+ Oid type_id;
+} mapRelTypeEntry;
+
+static HTAB *mapRelType = NULL;
+
/* List of type cache entries for domain types */
static TypeCacheEntry *firstDomainTypeEntry = NULL;
@@ -358,6 +367,11 @@ lookup_type_cache(Oid type_id, int flags)
TypeCacheHash = hash_create("Type information cache", 64,
&ctl, HASH_ELEM | HASH_BLOBS);
+ ctl.keysize = sizeof(Oid);
+ ctl.entrysize = sizeof(mapRelTypeEntry);
+ mapRelType = hash_create("Map reloid to typeoid", 64,
+ &ctl, HASH_ELEM | HASH_BLOBS);
+
/* Also set up callbacks for SI invalidations */
CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
CacheRegisterSyscacheCallback(TYPEOID, TypeCacheTypCallback, (Datum) 0);
@@ -470,6 +484,24 @@ lookup_type_cache(Oid type_id, int flags)
ReleaseSysCache(tp);
}
+ /*
+ * Add a record to the relation->type map. We don't bother if type will
+ * become disconnected from the relation. Although it seems to be impossible,
+ * storing old data is safe in any case. In the worst case scenario we will
+ * just do an extra cleanup of a cache entry.
+ */
+ if (OidIsValid(typentry->typrelid) && typentry->typtype == TYPTYPE_COMPOSITE)
+ {
+ mapRelTypeEntry *relentry;
+
+ relentry = (mapRelTypeEntry*) hash_search(mapRelType,
+ &typentry->typrelid,
+ HASH_ENTER, NULL);
+
+ relentry->typrelid = typentry->typrelid;
+ relentry->type_id = typentry->type_id;
+ }
+
/*
* Look up opclasses if we haven't already and any dependent info is
* requested.
@@ -2264,6 +2296,33 @@ SharedRecordTypmodRegistryAttach(SharedRecordTypmodRegistry *registry)
CurrentSession->shared_typmod_table = typmod_table;
}
+static void
+invalidateCompositeTypeCacheEntry(TypeCacheEntry *typentry)
+{
+ /* Delete tupdesc if we have it */
+ if (typentry->tupDesc != NULL)
+ {
+ /*
+ * Release our refcount and free the tupdesc if none remain. We can't
+ * use DecrTupleDescRefCount here because this reference is not logged
+ * by the current resource owner.
+ */
+ Assert(typentry->tupDesc->tdrefcount > 0);
+ if (--typentry->tupDesc->tdrefcount == 0)
+ FreeTupleDesc(typentry->tupDesc);
+ typentry->tupDesc = NULL;
+
+ /*
+ * Also clear tupDesc_identifier, so that anyone watching
+ * it will realize that the tupdesc has changed.
+ */
+ typentry->tupDesc_identifier = 0;
+ }
+
+ /* Reset equality/comparison/hashing validity information */
+ typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
+}
+
/*
* TypeCacheRelCallback
* Relcache inval callback function
@@ -2273,63 +2332,46 @@ SharedRecordTypmodRegistryAttach(SharedRecordTypmodRegistry *registry)
* whatever info we have cached about the composite type's comparability.
*
* This is called when a relcache invalidation event occurs for the given
- * relid. We must scan the whole typcache hash since we don't know the
- * type OID corresponding to the relid. We could do a direct search if this
- * were a syscache-flush callback on pg_type, but then we would need all
- * ALTER-TABLE-like commands that could modify a rowtype to issue syscache
- * invals against the rel's pg_type OID. The extra SI signaling could very
- * well cost more than we'd save, since in most usages there are not very
- * many entries in a backend's typcache. The risk of bugs-of-omission seems
- * high, too.
- *
- * Another possibility, with only localized impact, is to maintain a second
- * hashtable that indexes composite-type typcache entries by their typrelid.
- * But it's still not clear it's worth the trouble.
+ * relid. We can't use syscache to find a type corresponding to the given
+ * relation because the code can be called outside of transaction. Thus we use
+ * a dedicated relid->type map, mapRelType.
*/
static void
TypeCacheRelCallback(Datum arg, Oid relid)
{
- HASH_SEQ_STATUS status;
TypeCacheEntry *typentry;
- /* TypeCacheHash must exist, else this callback wouldn't be registered */
- hash_seq_init(&status, TypeCacheHash);
- while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
+ /*
+ * mapRelType and TypeCacheHash should exist, otherwise this callback
+ * wouldn't be registered
+ */
+
+ if (OidIsValid(relid))
{
- if (typentry->typtype == TYPTYPE_COMPOSITE)
+ mapRelTypeEntry *relentry;
+
+ relentry = (mapRelTypeEntry *) hash_search(mapRelType,
+ &relid,
+ HASH_FIND, NULL);
+
+ if (relentry != NULL)
{
- /* Skip if no match, unless we're zapping all composite types */
- if (relid != typentry->typrelid && relid != InvalidOid)
- continue;
+ typentry = (TypeCacheEntry *) hash_search(TypeCacheHash,
+ &relentry->type_id,
+ HASH_FIND, NULL);
- /* Delete tupdesc if we have it */
- if (typentry->tupDesc != NULL)
+ if (typentry != NULL)
{
- /*
- * Release our refcount, and free the tupdesc if none remain.
- * (Can't use DecrTupleDescRefCount because this reference is
- * not logged in current resource owner.)
- */
- Assert(typentry->tupDesc->tdrefcount > 0);
- if (--typentry->tupDesc->tdrefcount == 0)
- FreeTupleDesc(typentry->tupDesc);
- typentry->tupDesc = NULL;
+ Assert(typentry->typtype == TYPTYPE_COMPOSITE);
+ Assert(relid == typentry->typrelid);
- /*
- * Also clear tupDesc_identifier, so that anything watching
- * that will realize that the tupdesc has possibly changed.
- * (Alternatively, we could specify that to detect possible
- * tupdesc change, one must check for tupDesc != NULL as well
- * as tupDesc_identifier being the same as what was previously
- * seen. That seems error-prone.)
- */
- typentry->tupDesc_identifier = 0;
+ invalidateCompositeTypeCacheEntry(typentry);
}
-
- /* Reset equality/comparison/hashing validity information */
- typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
}
- else if (typentry->typtype == TYPTYPE_DOMAIN)
+
+ for (typentry = firstDomainTypeEntry;
+ typentry != NULL;
+ typentry = typentry->nextDomain)
{
/*
* If it's domain over composite, reset flags. (We don't bother
@@ -2341,6 +2383,35 @@ TypeCacheRelCallback(Datum arg, Oid relid)
typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
}
}
+ else
+ {
+ HASH_SEQ_STATUS status;
+
+ /*
+ * Relid = 0, so we need to reset all composite types in cache. Also, we
+ * should reset flags for domain types, and we loop over all entries
+ * in hash, so, do it in a single scan.
+ */
+ hash_seq_init(&status, TypeCacheHash);
+ while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
+ {
+ if (typentry->typtype == TYPTYPE_COMPOSITE)
+ {
+ invalidateCompositeTypeCacheEntry(typentry);
+ }
+ else if (typentry->typtype == TYPTYPE_DOMAIN)
+ {
+ /*
+ * If it's domain over composite, reset flags. (We don't bother
+ * trying to determine whether the specific base type needs a
+ * reset.) Note that if we haven't determined whether the base
+ * type is composite, we don't need to reset anything.
+ */
+ if (typentry->flags & TCFLAGS_DOMAIN_BASE_IS_COMPOSITE)
+ typentry->flags &= ~TCFLAGS_OPERATOR_FLAGS;
+ }
+ }
+ }
}
/*
--
2.43.2
