v8-0002-review.patch

From d80cdbcac8cb4105d35aeaa8f0600b3b5f530942 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@vondra.me>
Date: Mon, 31 Mar 2025 14:54:06 +0200
Subject: [PATCH v8 2/5] review

---
 src/backend/storage/ipc/shmem.c   |  2 +-
 src/backend/utils/hash/dynahash.c | 71 ++++++++++++++++---------------
 src/include/utils/hsearch.h       |  2 +-
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 3c030d5743d..8e19134cb5c 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -348,7 +348,7 @@ ShmemInitHash(const char *name,		/* table string name for shmem index */
 	/* look it up in the shmem index */
 	location = ShmemInitStruct(name,
 							   hash_get_init_size(infoP, hash_flags,
-												  true, init_size),
+												  init_size, true),
 							   &found);
 
 	/*
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 3dede9caa5d..0240a871395 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -383,7 +383,7 @@ hash_create(const char *tabname, long nelem, const HASHCTL *info, int flags)
 	HTAB	   *hashp;
 	HASHHDR    *hctl;
 	int			nelem_batch;
-	bool 	initial_elems = false;
+	bool		prealloc;
 
 	/*
 	 * Hash tables now allocate space for key and data, but you have to say
@@ -547,15 +547,11 @@ hash_create(const char *tabname, long nelem, const HASHCTL *info, int flags)
 	 * For a shared hash table, preallocate the requested number of elements.
 	 * This reduces problems with run-time out-of-shared-memory conditions.
 	 *
-	 * For a non-shared hash table, preallocate the requested number of
-	 * elements if it's less than our chosen nelem_alloc.  This avoids wasting
-	 * space if the caller correctly estimates a small table size.
+	 * For a private hash table, preallocate the requested number of elements
+	 * if it's less than our chosen nelem_alloc.  This avoids wasting space if
+	 * the caller correctly estimates a small table size.
 	 */
-	if ((flags & HASH_SHARED_MEM) ||
-		nelem < nelem_batch)
-		initial_elems = true;
-	else
-		initial_elems = false;
+	prealloc = (flags & HASH_SHARED_MEM) || (nelem < nelem_batch);
 
 	/*
 	 * Allocate the memory needed for hash header, directory, segments and
@@ -564,7 +560,7 @@ hash_create(const char *tabname, long nelem, const HASHCTL *info, int flags)
 	 */
 	if (!hashp->hctl)
 	{
-		Size		size = hash_get_init_size(info, flags, initial_elems, nelem);
+		Size		size = hash_get_init_size(info, flags, nelem, prealloc);
 
 		hashp->hctl = (HASHHDR *) hashp->alloc(size);
 		if (!hashp->hctl)
@@ -664,7 +660,8 @@ hash_create(const char *tabname, long nelem, const HASHCTL *info, int flags)
 		/*
 		 * Calculate the offset at which to find the first partition of
 		 * elements. We have to skip space for the header, segments and
-		 * buckets.
+		 * buckets. We need to recalculate the number of segments, which we
+		 * don't store anywhere.
 		 */
 		compute_buckets_and_segs(nelem, hctl->num_partitions, hctl->ssize,
 								 &nbuckets, &nsegs);
@@ -899,21 +896,24 @@ hash_select_dirsize(long num_entries)
 }
 
 /*
- * Compute the required initial memory allocation for a hashtable with the given
- * parameters. The hash table may be shared or private. We need space for the
- * HASHHDR, for the directory, segments and the init_size elements in buckets.
+ * hash_get_init_size -- determine memory needed for a new dynamic hash table
  *
- * For shared hash tables the directory size is non-expansive.
- *
- * nelem is total number of elements requested during hash table creation.
+ *	info: hash table parameters
+ *	flags: bitmask indicating which parameters to take from *info
+ *	nelem: maximum number of elements expected
+ *	prealloc: request preallocation of elements
  *
- * initial_nelems is true if it is decided to pre-allocate elements and false
- * otherwise.
+ * Compute the required initial memory allocation for a hashtable with the given
+ * parameters. We need space for the HASHHDR, for the directory, segments and
+ * preallocated elements.
  *
- * For a shared hash table initial_elems is always true.
+ * The hash table may be private or shared. For shared hash tables the directory
+ * size is non-expansive, and we preallocate all elements (nelem). For private
+ * hash tables, we preallocate elements only if the expected number of elements
+ * is small (less than nelem_alloc).
  */
 Size
-hash_get_init_size(const HASHCTL *info, int flags, bool initial_elems, long nelem)
+hash_get_init_size(const HASHCTL *info, int flags, long nelem, bool prealloc)
 {
 	int			nbuckets;
 	int			nsegs;
@@ -921,21 +921,29 @@ hash_get_init_size(const HASHCTL *info, int flags, bool initial_elems, long nele
 	long		ssize;
 	long		dsize;
 	Size		elementSize = HASH_ELEMENT_SIZE(info);
-	int 	init_size = 0;
+	long		nelem_prealloc = 0;
 
+#ifdef USE_ASSERT_CHECKING
+	/* shared hash tables have non-expansive directory */
+	/* XXX what about segment size? should check have HASH_SEGMENT? */
 	if (flags & HASH_SHARED_MEM)
 	{
 		Assert(flags & HASH_DIRSIZE);
 		Assert(info->dsize == info->max_dsize);
+		Assert(prealloc);
 	}
+#endif
 
 	/* Non-shared hash tables may not specify dir size */
-	if (!(flags & HASH_DIRSIZE))
-	{
+	if (flags & HASH_DIRSIZE)
+		dsize = info->dsize;
+	else
 		dsize = DEF_DIRSIZE;
-	}
+
+	if (flags & HASH_SEGMENT)
+		ssize = info->ssize;
 	else
-		dsize = info->dsize;
+		ssize = DEF_SEGSIZE;
 
 	if (flags & HASH_PARTITION)
 	{
@@ -948,21 +956,16 @@ hash_get_init_size(const HASHCTL *info, int flags, bool initial_elems, long nele
 	else
 		num_partitions = 0;
 
-	if (flags & HASH_SEGMENT)
-		ssize = info->ssize;
-	else
-		ssize = DEF_SEGSIZE;
-
 	compute_buckets_and_segs(nelem, num_partitions, ssize,
 							 &nbuckets, &nsegs);
 
 	/* initial_elems as false indicates no elements are to be pre-allocated */
-	if (initial_elems)
-		init_size = nelem;
+	if (prealloc)
+		nelem_prealloc = nelem;
 
 	return sizeof(HASHHDR) + dsize * sizeof(HASHSEGMENT)
 		+ sizeof(HASHBUCKET) * ssize * nsegs
-		+ init_size * elementSize;
+		+ nelem_prealloc * elementSize;
 }
 
 
diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index 4d5f09081b4..5dc74db6e96 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -152,7 +152,7 @@ extern void hash_freeze(HTAB *hashp);
 extern Size hash_estimate_size(long num_entries, Size entrysize);
 extern long hash_select_dirsize(long num_entries);
 extern Size hash_get_init_size(const HASHCTL *info, int flags,
-							   bool initial_elems, long nelem);
+							   long nelem, bool prealloc);
 extern void AtEOXact_HashTables(bool isCommit);
 extern void AtEOSubXact_HashTables(bool isCommit, int nestDepth);
 
-- 
2.49.0