Re: Reducing opr_sanity test's runtime under CLOBBER_CACHE_ALWAYS

Andres Freund <andres@anarazel.de> writes:
> On 2021-05-10 14:06:16 -0400, Tom Lane wrote:
>> I wonder if there's anything we could do to make ResetCatalogCache
>> faster?  It wouldn't help much for normal execution of course,
>> but it might do something to bring CCA testing time down out of
>> the stratosphere.

> We could make the hashtables shrink, not just grow...

I noticed that we already have counters that can tell whether a
catcache or dynahash table is empty, so I experimented with the
attached patch.  Testing one of the slow queries from privileges.sql
(which might not be very representative of the overall issue),
I see:

HEAD:
Time: 536429.715 ms (08:56.430)

with ResetCatalogCache hack:
Time: 488938.597 ms (08:08.939)

plus hash_seq_search hack:
Time: 475400.634 ms (07:55.401)

Of course, the issue with these patches is that they change these
counters from things that (I think) we only trust for statistical
purposes into things that had better be right or you're going to
have impossible-to-track-down bugs with sometimes failing to
invalidate cache entries.  My gut feeling is that the risk-to-reward
ratio is worth it for changing ResetCatalogCache, but not for
hash_seq_search.  This is partly because of the greater absolute
payback and partly because ResetCatalogCache doesn't deal with
shared data structures, reducing the risk of counting issues.

            regards, tom lane

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 4fbdc62d8c..df5f219254 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -644,6 +644,10 @@ ResetCatalogCache(CatCache *cache)
     dlist_mutable_iter iter;
     int            i;

+    /* If the cache is entirely empty, there's nothing to do */
+    if (cache->cc_ntup == 0)
+        return;
+
     /* Remove each list in this cache, or at least mark it dead */
     dlist_foreach_modify(iter, &cache->cc_lists)
     {
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 6546e3c7c7..1ac87725c0 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1455,11 +1455,23 @@ hash_seq_search(HASH_SEQ_STATUS *status)
     }

     /*
-     * Search for next nonempty bucket starting at curBucket.
+     * Nothing to do if hash table is entirely empty.  (For a partitioned
+     * hash table, we'd have to check all the freelists, which is unlikely
+     * to be a win.)
      */
-    curBucket = status->curBucket;
     hashp = status->hashp;
     hctl = hashp->hctl;
+    if (hctl->freeList[0].nentries == 0 &&
+        !IS_PARTITIONED(hctl))
+    {
+        hash_seq_term(status);
+        return NULL;            /* table is empty */
+    }
+
+    /*
+     * Search for next nonempty bucket starting at curBucket.
+     */
+    curBucket = status->curBucket;
     ssize = hashp->ssize;
     max_bucket = hctl->max_bucket;