postgres.git / summary / log / commit / refs
commit 2f487116e9988615d05a6c380c3ff8fa8a4286f8
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon Jul 05 20:51:57 2021 +0000
First released in: 15.0, 9.6.23, 10.18, 11.13, 12.8, 13.4, 14.0
Reduce overhead of cache-clobber testing in LookupOpclassInfo().
Commit 03ffc4d6d added logic to bypass all caching behavior in
LookupOpclassInfo when CLOBBER_CACHE_ALWAYS is enabled. It doesn't
look like I stopped to think much about what that would cost, but
recent investigation shows that the cost is enormous: it roughly
doubles the time needed for cache-clobber test runs.
There does seem to be value in this behavior when trying to test
the opclass-cache loading logic itself, but for other purposes the
cost is excessive. Hence, let's back off to doing this only when
debug_invalidate_system_caches_always is at least 3; or in older
branches, when CLOBBER_CACHE_RECURSIVELY is defined.
While here, clean up some other minor issues in LookupOpclassInfo.
Re-order the code so we aren't left with broken cache entries (leading
to later core dumps) in the unlikely case that we suffer OOM while
trying to allocate space for a new entry. (That seems to be my
oversight in 03ffc4d6d.) Also, in >= v13, stop allocating one array
entry too many. That's evidently left over from sloppy reversion in
851b14b0c.
Back-patch to all supported branches, mainly to reduce the runtime
of cache-clobbering buildfarm animals.
Discussion: https://postgr.es/m/1370856.1625428625@sss.pgh.pa.us
src/backend/utils/cache/relcache.c | 43 ++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 23405711d1d..32e75560ead 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1622,15 +1622,15 @@ LookupOpclassInfo(Oid operatorClassOid,
/* First time through: initialize the opclass cache */
HASHCTL ctl;
+ /* Also make sure CacheMemoryContext exists */
+ if (!CacheMemoryContext)
+ CreateCacheMemoryContext();
+
MemSet(&ctl, 0, sizeof(ctl));
ctl.keysize = sizeof(Oid);
ctl.entrysize = sizeof(OpClassCacheEnt);
OpClassCache = hash_create("Operator class cache", 64,
&ctl, HASH_ELEM | HASH_BLOBS);
-
- /* Also make sure CacheMemoryContext exists */
- if (!CacheMemoryContext)
- CreateCacheMemoryContext();
}
opcentry = (OpClassCacheEnt *) hash_search(OpClassCache,
@@ -1639,16 +1639,10 @@ LookupOpclassInfo(Oid operatorClassOid,
if (!found)
{
- /* Need to allocate memory for new entry */
+ /* Initialize new entry */
opcentry->valid = false; /* until known OK */
opcentry->numSupport = numSupport;
-
- if (numSupport > 0)
- opcentry->supportProcs = (RegProcedure *)
- MemoryContextAllocZero(CacheMemoryContext,
- (numSupport + 1) * sizeof(RegProcedure));
- else
- opcentry->supportProcs = NULL;
+ opcentry->supportProcs = NULL; /* filled below */
}
else
{
@@ -1656,13 +1650,15 @@ LookupOpclassInfo(Oid operatorClassOid,
}
/*
- * When testing for cache-flush hazards, we intentionally disable the
- * operator class cache and force reloading of the info on each call. This
- * is helpful because we want to test the case where a cache flush occurs
- * while we are loading the info, and it's very hard to provoke that if
- * this happens only once per opclass per backend.
+ * When aggressively testing cache-flush hazards, we disable the operator
+ * class cache and force reloading of the info on each call. This models
+ * no real-world behavior, since the cache entries are never invalidated
+ * otherwise. However it can be helpful for detecting bugs in the cache
+ * loading logic itself, such as reliance on a non-nailed index. Given
+ * the limited use-case and the fact that this adds a great deal of
+ * expense, we enable it only in CLOBBER_CACHE_RECURSIVELY mode.
*/
-#if defined(CLOBBER_CACHE_ALWAYS)
+#if defined(CLOBBER_CACHE_RECURSIVELY)
opcentry->valid = false;
#endif
@@ -1670,8 +1666,15 @@ LookupOpclassInfo(Oid operatorClassOid,
return opcentry;
/*
- * Need to fill in new entry.
- *
+ * Need to fill in new entry. First allocate space, unless we already did
+ * so in some previous attempt.
+ */
+ if (opcentry->supportProcs == NULL && numSupport > 0)
+ opcentry->supportProcs = (RegProcedure *)
+ MemoryContextAllocZero(CacheMemoryContext,
+ numSupport * sizeof(RegProcedure));
+
+ /*
* To avoid infinite recursion during startup, force heap scans if we're
* looking up info for the opclasses used by the indexes we would like to
* reference here.
[parent: 27621cc55526]