From d80cdbcac8cb4105d35aeaa8f0600b3b5f530942 Mon Sep 17 00:00:00 2001 From: Tomas Vondra 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