From d025cf88ba5a64487ee4a17ef23e8f55b1536606 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Wed, 8 Apr 2020 16:55:03 +1200 Subject: [PATCH] Modify various power 2 calculations to use new helper functions First pass of modifying various places that obtain the next power of 2 of a number and make them use the new functions added in pg_bitutils.h instead. This also removes the _hash_log2() function. There are no longer any callers in core. Other users can swap their _hash_log2(n) call to make use of pg_ceil_log2_32(n). Author: David Fetter, with some minor adjustments by me Reviewed-by: John Naylor, Jesse Zhang Discussion: https://postgr.es/m/20200114173553.GE32763%40fetter.org --- src/backend/access/hash/hashpage.c | 24 +++++++++++------------- src/backend/access/hash/hashsort.c | 3 ++- src/backend/access/hash/hashutil.c | 19 ++----------------- src/backend/utils/hash/dynahash.c | 14 +++++++------- src/include/access/hash.h | 1 - src/include/lib/simplehash.h | 27 ++++----------------------- src/include/port/pg_bitutils.h | 6 ++++++ 7 files changed, 32 insertions(+), 62 deletions(-) diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index 55d85644a4e..a664ecf494a 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -31,6 +31,7 @@ #include "access/hash.h" #include "access/hash_xlog.h" #include "miscadmin.h" +#include "port/pg_bitutils.h" #include "storage/lmgr.h" #include "storage/predicate.h" #include "storage/smgr.h" @@ -502,7 +503,7 @@ _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid, double dnumbuckets; uint32 num_buckets; uint32 spare_index; - uint32 i; + uint32 lshift; /* * Choose the number of initial bucket pages to match the fill factor @@ -542,15 +543,12 @@ _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid, metap->hashm_nmaps = 0; 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; + lshift = pg_leftmost_one_pos32(metap->hashm_bsize); + Assert(lshift > 0); + metap->hashm_bmsize = 1 << lshift; + metap->hashm_bmshift = lshift + BYTE_TO_BIT; Assert((1 << BMPG_SHIFT(metap)) == (BMPG_MASK(metap) + 1)); /* @@ -570,7 +568,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 = pg_nextpower2_32(num_buckets + 1) - 1; metap->hashm_lowmask = (metap->hashm_highmask >> 1); MemSet(metap->hashm_spares, 0, sizeof(metap->hashm_spares)); @@ -659,9 +657,9 @@ restart_expand: * * 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 + * to half that to prevent failure of pg_ceil_log2_32() 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. * diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c index 9cb41d62e7c..2c7b5857b53 100644 --- a/src/backend/access/hash/hashsort.c +++ b/src/backend/access/hash/hashsort.c @@ -29,6 +29,7 @@ #include "commands/progress.h" #include "miscadmin.h" #include "pgstat.h" +#include "port/pg_bitutils.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 = pg_nextpower2_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 9efc8016bc0..b23d19474e6 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 @@ -158,8 +144,7 @@ _hash_spareindex(uint32 num_bucket) { uint32 splitpoint_group; uint32 splitpoint_phases; - - splitpoint_group = _hash_log2(num_bucket); + splitpoint_group = pg_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 b5381958e70..5b4b9e487f5 100644 --- a/src/backend/utils/hash/dynahash.c +++ b/src/backend/utils/hash/dynahash.c @@ -87,6 +87,7 @@ #include "access/xact.h" #include "common/hashfn.h" +#include "port/pg_bitutils.h" #include "storage/shmem.h" #include "storage/spin.h" #include "utils/dynahash.h" @@ -1718,16 +1719,15 @@ hash_corrupted(HTAB *hashp) int my_log2(long num) { - int i; - long limit; - - /* guard against too-large input, which would put us into infinite loop */ + /* guard against too-large input, which would be invalid for pg_ceil_log2_*() */ if (num > LONG_MAX / 2) num = LONG_MAX / 2; - for (i = 0, limit = 1; limit < num; i++, limit <<= 1) - ; - return i; +#if SIZEOF_LONG < 8 + return pg_ceil_log2_32(num); +#else + return pg_ceil_log2_64(num); +#endif } /* calculate first power of 2 >= num, bounded to what will fit in a long */ diff --git a/src/include/access/hash.h b/src/include/access/hash.h index 8cda938cbe4..94b643cc779 100644 --- a/src/include/access/hash.h +++ b/src/include/access/hash.h @@ -451,7 +451,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 5a6783f6532..8cb03cda6cc 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 = pg_nextpower2_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 == pg_nextpower2_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 4ca92f076dc..887e7829111 100644 --- a/src/include/port/pg_bitutils.h +++ b/src/include/port/pg_bitutils.h @@ -13,9 +13,15 @@ #ifndef PG_BITUTILS_H #define PG_BITUTILS_H +#ifndef FRONTEND extern PGDLLIMPORT const uint8 pg_leftmost_one_pos[256]; extern PGDLLIMPORT const uint8 pg_rightmost_one_pos[256]; extern PGDLLIMPORT const uint8 pg_number_of_ones[256]; +#else +extern const uint8 pg_leftmost_one_pos[256]; +extern const uint8 pg_rightmost_one_pos[256]; +extern const uint8 pg_number_of_ones[256]; +#endif /* * pg_leftmost_one_pos32