From a73cdc21d2d62df32bc087b2b4620e7c7f3f5b24 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Fri, 31 Jan 2020 07:08:48 -0800
Subject: [PATCH v5 2/3] Spread bitutils into hashing
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.24.1"

This is a multi-part message in MIME format.
--------------2.24.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 55d85644a4..29dca21be2 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -30,6 +30,7 @@
 
 #include "access/hash.h"
 #include "access/hash_xlog.h"
+#include "port/pg_bitutils.h"
 #include "miscadmin.h"
 #include "storage/lmgr.h"
 #include "storage/predicate.h"
@@ -502,7 +503,6 @@ _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid,
 	double		dnumbuckets;
 	uint32		num_buckets;
 	uint32		spare_index;
-	uint32		i;
 
 	/*
 	 * Choose the number of initial bucket pages to match the fill factor
@@ -543,14 +543,8 @@ _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid,
 	metap->hashm_ffactor = ffactor;
 	metap->hashm_bsize = HashGetMaxBitmapSize(page);
 	/* find largest bitmap array size that will fit in page size */
-	for (i = _hash_log2(metap->hashm_bsize); i > 0; --i)
-	{
-		if ((1 << i) <= metap->hashm_bsize)
-			break;
-	}
-	Assert(i > 0);
-	metap->hashm_bmsize = 1 << i;
-	metap->hashm_bmshift = i + BYTE_TO_BIT;
+	metap->hashm_bmsize = 1 << pg_leftmost_one_pos32(metap->hashm_bsize);
+	metap->hashm_bmshift = pg_leftmost_one_pos32(metap->hashm_bsize) + BYTE_TO_BIT;
 	Assert((1 << BMPG_SHIFT(metap)) == (BMPG_MASK(metap) + 1));
 
 	/*
@@ -570,7 +564,7 @@ _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid,
 	 * Set highmask as next immediate ((2 ^ x) - 1), which should be
 	 * sufficient to cover num_buckets.
 	 */
-	metap->hashm_highmask = (1 << (_hash_log2(num_buckets + 1))) - 1;
+	metap->hashm_highmask = next_power_of_2_32(num_buckets + 1) - 1;
 	metap->hashm_lowmask = (metap->hashm_highmask >> 1);
 
 	MemSet(metap->hashm_spares, 0, sizeof(metap->hashm_spares));
@@ -657,13 +651,12 @@ restart_expand:
 	 * Can't split anymore if maxbucket has reached its maximum possible
 	 * value.
 	 *
-	 * Ideally we'd allow bucket numbers up to UINT_MAX-1 (no higher because
-	 * the calculation maxbucket+1 mustn't overflow).  Currently we restrict
-	 * to half that because of overflow looping in _hash_log2() and
-	 * insufficient space in hashm_spares[].  It's moot anyway because an
-	 * index with 2^32 buckets would certainly overflow BlockNumber and hence
-	 * _hash_alloc_buckets() would fail, but if we supported buckets smaller
-	 * than a disk block then this would be an independent constraint.
+	 * Ideally we'd allow bucket numbers up to UINT_MAX-1 (no higher because the
+	 * calculation maxbucket+1 mustn't overflow).  Currently we restrict to half
+	 * that because of insufficient space in hashm_spares[].  It's moot anyway
+	 * because an index with 2^32 buckets would certainly overflow BlockNumber
+	 * and hence _hash_alloc_buckets() would fail, but if we supported buckets
+	 * smaller than a disk block then this would be an independent constraint.
 	 *
 	 * If you change this, see also the maximum initial number of buckets in
 	 * _hash_init().
diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c
index 9cb41d62e7..322379788c 100644
--- a/src/backend/access/hash/hashsort.c
+++ b/src/backend/access/hash/hashsort.c
@@ -27,6 +27,7 @@
 
 #include "access/hash.h"
 #include "commands/progress.h"
+#include "port/pg_bitutils.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "utils/tuplesort.h"
@@ -69,7 +70,7 @@ _h_spoolinit(Relation heap, Relation index, uint32 num_buckets)
 	 * NOTE : This hash mask calculation should be in sync with similar
 	 * calculation in _hash_init_metabuffer.
 	 */
-	hspool->high_mask = (((uint32) 1) << _hash_log2(num_buckets + 1)) - 1;
+	hspool->high_mask = next_power_of_2_32(num_buckets + 1) - 1;
 	hspool->low_mask = (hspool->high_mask >> 1);
 	hspool->max_buckets = num_buckets - 1;
 
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 9efc8016bc..738572ca40 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -17,6 +17,7 @@
 #include "access/hash.h"
 #include "access/reloptions.h"
 #include "access/relscan.h"
+#include "port/pg_bitutils.h"
 #include "storage/buf_internals.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
@@ -134,21 +135,6 @@ _hash_hashkey2bucket(uint32 hashkey, uint32 maxbucket,
 	return bucket;
 }
 
-/*
- * _hash_log2 -- returns ceil(lg2(num))
- */
-uint32
-_hash_log2(uint32 num)
-{
-	uint32		i,
-				limit;
-
-	limit = 1;
-	for (i = 0; limit < num; limit <<= 1, i++)
-		;
-	return i;
-}
-
 /*
  * _hash_spareindex -- returns spare index / global splitpoint phase of the
  *					   bucket
@@ -159,7 +145,7 @@ _hash_spareindex(uint32 num_bucket)
 	uint32		splitpoint_group;
 	uint32		splitpoint_phases;
 
-	splitpoint_group = _hash_log2(num_bucket);
+	splitpoint_group = ceil_log2_32(num_bucket);
 
 	if (splitpoint_group < HASH_SPLITPOINT_GROUPS_WITH_ONE_PHASE)
 		return splitpoint_group;
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 6e98b61b2f..deef317520 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -86,6 +86,7 @@
 #include <limits.h>
 
 #include "access/xact.h"
+#include "port/pg_bitutils.h"
 #include "storage/shmem.h"
 #include "storage/spin.h"
 #include "utils/dynahash.h"
@@ -1718,16 +1719,11 @@ hash_corrupted(HTAB *hashp)
 int
 my_log2(uint64 num)
 {
-	int			i;
-	uint64		limit;
-
 	/* guard against too-large input, which would put us into infinite loop */
 	if (num > LONG_MAX / 2)
 		num = LONG_MAX / 2;
 
-	for (i = 0, limit = 1; limit < num; i++, limit <<= 1)
-		;
-	return i;
+	return ceil_log2_64(num);
 }
 
 /* calculate first power of 2 >= num, bounded to what will fit in a uint64 */
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 9fc0696096..298c05e6fe 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -450,7 +450,6 @@ extern uint32 _hash_datum2hashkey(Relation rel, Datum key);
 extern uint32 _hash_datum2hashkey_type(Relation rel, Datum key, Oid keytype);
 extern Bucket _hash_hashkey2bucket(uint32 hashkey, uint32 maxbucket,
 								   uint32 highmask, uint32 lowmask);
-extern uint32 _hash_log2(uint32 num);
 extern uint32 _hash_spareindex(uint32 num_bucket);
 extern uint32 _hash_get_totalbuckets(uint32 splitpoint_phase);
 extern void _hash_checkpage(Relation rel, Buffer buf, int flags);
diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h
index 5a6783f653..1a35a054d8 100644
--- a/src/include/lib/simplehash.h
+++ b/src/include/lib/simplehash.h
@@ -57,6 +57,8 @@
  *	  backwards, unless they're empty or already at their optimal position.
  */
 
+#include "port/pg_bitutils.h"
+
 /* helpers */
 #define SH_MAKE_PREFIX(a) CppConcat(a,_)
 #define SH_MAKE_NAME(name) SH_MAKE_NAME_(SH_MAKE_PREFIX(SH_PREFIX),name)
@@ -215,27 +217,6 @@ SH_SCOPE void SH_STAT(SH_TYPE * tb);
 #ifndef SIMPLEHASH_H
 #define SIMPLEHASH_H
 
-/* FIXME: can we move these to a central location? */
-
-/* calculate ceil(log base 2) of num */
-static inline uint64
-sh_log2(uint64 num)
-{
-	int			i;
-	uint64		limit;
-
-	for (i = 0, limit = 1; limit < num; i++, limit <<= 1)
-		;
-	return i;
-}
-
-/* calculate first power of 2 >= num */
-static inline uint64
-sh_pow2(uint64 num)
-{
-	return ((uint64) 1) << sh_log2(num);
-}
-
 #ifdef FRONTEND
 #define sh_error(...) pg_log_error(__VA_ARGS__)
 #define sh_log(...) pg_log_info(__VA_ARGS__)
@@ -259,7 +240,7 @@ SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
 	size = Max(newsize, 2);
 
 	/* round up size to the next power of 2, that's how bucketing works */
-	size = sh_pow2(size);
+	size = next_power_of_2_64(size);
 	Assert(size <= SH_MAX_SIZE);
 
 	/*
@@ -434,7 +415,7 @@ SH_GROW(SH_TYPE * tb, uint32 newsize)
 	uint32		startelem = 0;
 	uint32		copyelem;
 
-	Assert(oldsize == sh_pow2(oldsize));
+	Assert(oldsize == next_power_of_2_64(oldsize));
 	Assert(oldsize != SH_MAX_SIZE);
 	Assert(oldsize < newsize);
 
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 498e532308..88a9ea5b7f 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -20,19 +20,18 @@ extern PGDLLIMPORT const uint8 pg_number_of_ones[256];
 /*
  * pg_leftmost_one_pos32
  *		Returns the position of the most significant set bit in "word",
- *		measured from the least significant bit.  word must not be 0.
+ *		measured from the least significant bit.
  */
 static inline int
 pg_leftmost_one_pos32(uint32 word)
 {
 #ifdef HAVE__BUILTIN_CLZ
-	Assert(word != 0);
-
-	return 31 - __builtin_clz(word);
+	return word ? 31 - __builtin_clz(word) : 0;
 #else
 	int			shift = 32 - 8;
 
-	Assert(word != 0);
+	if (word == 0)
+		return 0;
 
 	while ((word >> shift) == 0)
 		shift -= 8;
@@ -49,19 +48,18 @@ static inline int
 pg_leftmost_one_pos64(uint64 word)
 {
 #ifdef HAVE__BUILTIN_CLZ
-	Assert(word != 0);
-
 #if defined(HAVE_LONG_INT_64)
-	return 63 - __builtin_clzl(word);
+	return word ? 63 - __builtin_clzl(word) : 0;
 #elif defined(HAVE_LONG_LONG_INT_64)
-	return 63 - __builtin_clzll(word);
+	return word ? 63 - __builtin_clzll(word) : 0;
 #else
 #error must have a working 64-bit integer datatype
 #endif
 #else							/* !HAVE__BUILTIN_CLZ */
 	int			shift = 64 - 8;
 
-	Assert(word != 0);
+	if (word == 0)
+		return 0;
 
 	while ((word >> shift) == 0)
 		shift -= 8;
@@ -73,19 +71,18 @@ pg_leftmost_one_pos64(uint64 word)
 /*
  * pg_rightmost_one_pos32
  *		Returns the position of the least significant set bit in "word",
- *		measured from the least significant bit.  word must not be 0.
+ *		measured from the least significant bit.
  */
 static inline int
 pg_rightmost_one_pos32(uint32 word)
 {
 #ifdef HAVE__BUILTIN_CTZ
-	Assert(word != 0);
-
-	return __builtin_ctz(word);
+	return word ? __builtin_ctz(word) : 32;
 #else
 	int			result = 0;
 
-	Assert(word != 0);
+	if (word == 0)
+		return 32;
 
 	while ((word & 255) == 0)
 	{
@@ -105,19 +102,18 @@ static inline int
 pg_rightmost_one_pos64(uint64 word)
 {
 #ifdef HAVE__BUILTIN_CTZ
-	Assert(word != 0);
-
 #if defined(HAVE_LONG_INT_64)
-	return __builtin_ctzl(word);
+	return word ? __builtin_ctzl(word) : 64;
 #elif defined(HAVE_LONG_LONG_INT_64)
-	return __builtin_ctzll(word);
+	return word ? __builtin_ctzll(word) : 64;
 #else
 #error must have a working 64-bit integer datatype
 #endif
 #else							/* !HAVE__BUILTIN_CTZ */
 	int			result = 0;
 
-	Assert(word != 0);
+	if (word == 0)
+		return 64;
 
 	while ((word & 255) == 0)
 	{
@@ -145,4 +141,34 @@ pg_rotate_right32(uint32 word, int n)
 	return (word >> n) | (word << (sizeof(word) * BITS_PER_BYTE - n));
 }
 
+/* ceil(lg2(num)) */
+static inline uint32
+ceil_log2_32(uint32 num)
+{
+	Assert(num > 0);
+	return pg_leftmost_one_pos32(num-1) + 1;
+}
+
+static inline uint64
+ceil_log2_64(uint64 num)
+{
+	Assert(num > 0);
+	return pg_leftmost_one_pos64(num-1) + 1;
+}
+
+/* Calculate the first power of 2 >= num */
+static inline uint32
+next_power_of_2_32(uint32 num)
+{
+	Assert(num > 0);
+	return ((uint32) 1) << (pg_leftmost_one_pos32(num-1) + 1);
+}
+
+static inline uint64
+next_power_of_2_64(uint64 num)
+{
+	Assert(num > 0);
+	return ((uint64) 1) << (pg_leftmost_one_pos64(num-1) + 1);
+}
+
 #endif							/* PG_BITUTILS_H */

--------------2.24.1--


