Remove HASH_DIRSIZE, always use the default algorithm to select it

It's not very useful to specify a non-standard directory size. The
HASH_DIRSIZE option was only used for shared memory hash tables, and
those always used hash_select_dirsize() to choose the size, which in
turn just uses the default algorithm anyway. That assumption was
ingrained in hash_estimate_size(), too.

Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://www.postgresql.org/message-id/01ab1d41-3eda-4705-8bbd-af898f5007f1@iki.fi

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index bf1b3f1e8f14acec8faa2aea42344a4a5975b479..3cb51ad62f8bf41719d9121f023810dea0baa271 100644 (file)
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -399,10 +399,9 @@ shmem_hash_create(void *location, size_t size, bool found,
         *
         * The shared memory allocator must be specified too.
         */
-       infoP->dsize = infoP->max_dsize = hash_select_dirsize(nelems);
        infoP->alloc = ShmemHashAlloc;
        infoP->alloc_arg = NULL;
-       hash_flags |= HASH_SHARED_MEM | HASH_ALLOC | HASH_DIRSIZE | HASH_FIXED_SIZE;
+       hash_flags |= HASH_SHARED_MEM | HASH_ALLOC | HASH_FIXED_SIZE;
 
        /*
         * if it already exists, attach to it rather than allocate and initialize

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 447b638b7c9249573e0c91c7b01e05521e0d686a..d49a7a92c64bd30a58f2d6100d31516aa132ea8e 100644 (file)
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -115,10 +115,12 @@
  * a whole lot of records per bucket or performance goes down.
  *
  * In a hash table allocated in shared memory, the directory cannot be
- * expanded because it must stay at a fixed address.  The directory size
- * should be selected using hash_select_dirsize (and you'd better have
- * a good idea of the maximum number of entries!).  For non-shared hash
- * tables, the initial directory size can be left at the default.
+ * expanded because it must stay at a fixed address.  The directory size is
+ * chosen at creation based on the initial number of elements, so even though
+ * we support allocating more elements later, performance will suffer if the
+ * table grows much beyond the initial size.  (Currently, shared memory hash
+ * tables are only created by ShmemInitHash() though, which doesn't support
+ * growing at all.)
  */
 #define HASH_SEGSIZE                      256
 #define HASH_SEGSIZE_SHIFT        8    /* must be log2(HASH_SEGSIZE) */
@@ -378,7 +380,7 @@ hash_create(const char *tabname, int64 nelem, const HASHCTL *info, int flags)
         * a context specified by the caller, or TopMemoryContext if nothing is
         * specified.
         *
-        * Note that HASH_DIRSIZE and HASH_ALLOC had better be set as well.
+        * Note that HASH_ALLOC had better be set as well.
         */
        if (flags & HASH_SHARED_MEM)
        {
@@ -552,15 +554,6 @@ hash_create(const char *tabname, int64 nelem, const HASHCTL *info, int flags)
                hctl->num_partitions = info->num_partitions;
        }
 
-       /*
-        * SHM hash tables have fixed directory size passed by the caller.
-        */
-       if (flags & HASH_DIRSIZE)
-       {
-               hctl->max_dsize = info->max_dsize;
-               hctl->dsize = info->dsize;
-       }
-
        /* remember the entry sizes, too */
        hctl->keysize = info->keysize;
        hctl->entrysize = info->entrysize;
@@ -639,9 +632,6 @@ hdefault(HTAB *hashp)
 
        MemSet(hctl, 0, sizeof(HASHHDR));
 
-       hctl->dsize = DEF_DIRSIZE;
-       hctl->nsegs = 0;
-
        hctl->num_partitions = 0;       /* not partitioned */
 
        /* table has no fixed maximum size */
@@ -734,8 +724,11 @@ init_htab(HTAB *hashp, int64 nelem)
        /*
         * Make sure directory is big enough.
         */
-       if (nsegs > hctl->dsize)
-               hctl->dsize = nsegs;
+       hctl->dsize = Max(DEF_DIRSIZE, nsegs);
+
+       /* SHM hash tables have a fixed directory. */
+       if (hashp->isshared)
+               hctl->max_dsize = hctl->dsize;
 
        /* Allocate a directory */
        hctl->dir = (HASHSEGMENT *)
@@ -781,9 +774,7 @@ hash_estimate_size(int64 num_entries, Size entrysize)
        /* # of segments needed for nBuckets */
        nSegments = next_pow2_int64((nBuckets - 1) / HASH_SEGSIZE + 1);
        /* directory entries */
-       nDirEntries = DEF_DIRSIZE;
-       while (nDirEntries < nSegments)
-               nDirEntries <<= 1;              /* dir_alloc doubles dsize at each call */
+       nDirEntries = Max(DEF_DIRSIZE, nSegments);
 
        /* fixed control info */
        size = MAXALIGN(sizeof(HASHHDR));       /* but not HTAB, per above */
@@ -803,34 +794,6 @@ hash_estimate_size(int64 num_entries, Size entrysize)
        return size;
 }
 
-/*
- * Select an appropriate directory size for a hashtable with the given
- * maximum number of entries.
- * This is only needed for hashtables in shared memory, whose directories
- * cannot be expanded dynamically.
- * NB: assumes that all hash structure parameters have default values!
- *
- * XXX this had better agree with the behavior of init_htab()...
- */
-int64
-hash_select_dirsize(int64 num_entries)
-{
-       int64           nBuckets,
-                               nSegments,
-                               nDirEntries;
-
-       /* estimate number of buckets wanted */
-       nBuckets = next_pow2_int64(num_entries);
-       /* # of segments needed for nBuckets */
-       nSegments = next_pow2_int64((nBuckets - 1) / HASH_SEGSIZE + 1);
-       /* directory entries */
-       nDirEntries = DEF_DIRSIZE;
-       while (nDirEntries < nSegments)
-               nDirEntries <<= 1;              /* dir_alloc doubles dsize at each call */
-
-       return nDirEntries;
-}
-
 
 /********************** DESTROY ROUTINES ************************/
 

diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index 6a1931b0d21fcf5139c616f0c82a9d0d06358ac8..896a22a2142eef6831e9c9ce2a83bbbfe9162469 100644 (file)
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -65,9 +65,6 @@ typedef struct HASHCTL
 {
        /* Used if HASH_PARTITION flag is set: */
        int64           num_partitions; /* # partitions (must be power of 2) */
-       /* Used if HASH_DIRSIZE flag is set: */
-       int64           dsize;                  /* (initial) directory size */
-       int64           max_dsize;              /* limit to dsize if dir size is limited */
        /* Used if HASH_ELEM flag is set (which is now required): */
        Size            keysize;                /* hash key length in bytes */
        Size            entrysize;              /* total user element size in bytes */
@@ -89,7 +86,7 @@ typedef struct HASHCTL
 /* Flag bits for hash_create; most indicate which parameters are supplied */
 #define HASH_PARTITION 0x0001  /* Hashtable is used w/partitioned locking */
 /* 0x0002 is unused */
-#define HASH_DIRSIZE   0x0004  /* Set directory size (initial and max) */
+/* 0x0004 is unused */
 #define HASH_ELEM              0x0008  /* Set keysize and entrysize (now required!) */
 #define HASH_STRINGS   0x0010  /* Select support functions for string keys */
 #define HASH_BLOBS             0x0020  /* Select support functions for binary keys */
@@ -148,7 +145,6 @@ extern void *hash_seq_search(HASH_SEQ_STATUS *status);
 extern void hash_seq_term(HASH_SEQ_STATUS *status);
 extern void hash_freeze(HTAB *hashp);
 extern Size hash_estimate_size(int64 num_entries, Size entrysize);
-extern int64 hash_select_dirsize(int64 num_entries);
 extern void AtEOXact_HashTables(bool isCommit);
 extern void AtEOSubXact_HashTables(bool isCommit, int nestDepth);