From: Ildar Musin <i.musin@postgrespro.ru>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: General purpose hashing func in pgbench
Date: Wed, 10 Jan 2018 16:37:03 +0300
Message-ID: <43a8fbbb-32fa-6478-30a9-f64041adf019@postgrespro.ru> (raw)
In-Reply-To: <63f5a6ac-0c5b-8d59-ca8b-fc8e11244a66@postgrespro.ru>
References: <0e8bd39e-dfcd-2879-f88f-272799ad7ef2@postgrespro.ru>
<alpine.DEB.2.20.1712200807550.5165@lancre>
<699225ee-c30c-b22e-4028-415e9eab26b2@postgrespro.ru>
<alpine.DEB.2.20.1712211620040.32625@lancre>
<80e9c570-5c49-1a01-f309-3d8d0cbbae42@postgrespro.ru>
<alpine.DEB.2.20.1712240846440.22976@lancre>
<38f1503e-7b63-4612-24a9-041af25bb022@postgrespro.ru>
<alpine.DEB.2.20.1712251506160.22976@lancre>
<af1ccd41-dcd0-f56b-0a05-a558da54762b@postgrespro.ru>
<alpine.DEB.2.20.1712251702170.22976@lancre>
<9603bac9-ae5d-36c5-5978-a88bdb700e37@postgrespro.ru>
<alpine.DEB.2.20.1801092000230.2326@hendaye>
<63f5a6ac-0c5b-8d59-ca8b-fc8e11244a66@postgrespro.ru>
10/01/2018 16:35, Ildar Musin пишет:
> 09/01/2018 23:11, Fabien COELHO пишет:
>> Hello Ildar,
>>
>>> Sorry for a long delay. I've added hash() function which is just an
>>> alias for murmur2. I've also utilized variable arguments feature from
>>> least()/greates() functions to make optional seed parameter, but I
>>> consider this as a hack.
>> Patch needs a rebase after Teodor push for a set of pgbench functions.
> Done. Congratulations on your patch finally being committed : )
>>> Should we probably add some infrastructure for optional arguments?
>> You can look at the handling of "CASE" which may or may not have an
>> "ELSE" clause.
>>
>> I'd suggest you use a new negative argument with the special meaning
>> for hash, and create the seed value when missing when building the
>> function, so as to simplify the executor code.
> Added a new nargs option -3 for hash functions and moved arguments check
> to parser. It's starting to look a bit odd and I'm thinking about
> replacing bare numbers (-1, -2, -3) with #defined macros. E.g.:
>
> #define PGBENCH_NARGS_VARIABLE (-1)
> #define PGBENCH_NARGS_CASE (-2)
> #define PGBENCH_NARGS_HASH (-3)
>> Instead of 0, I'd consider providing a random default so that the
>> hashing behavior is not the same from one run to the next. What do you
>> think?
>>
> Makes sence since each thread is also initializes its random_state with
> random numbers before start. So I added global variable 'hash_seed' and
> initialize it with random() before threads spawned.
>> Like the previous version, murmur2 with seed looks much more random
>> than fnv with seed...
>>
Sorry, here is the patch
--
Ildar Musin
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Attachments:pgbench_hash_v4.patch (text/plain, 4.0K) downloaddiff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.yindex e23ca51..847b011 100644--- a/src/bin/pgbench/exprparse.y+++ b/src/bin/pgbench/exprparse.y@@ -349,6 +349,15 @@ static const struct
{
"!case_end", -2, PGBENCH_CASE
},
+ {+ "hash", -3, PGBENCH_HASH_MURMUR2+ },+ {+ "hash_murmur2", -3, PGBENCH_HASH_MURMUR2+ },+ {+ "hash_fnv1a", -3, PGBENCH_HASH_FNV1A+ },
/* keep as last array element */
{
NULL, 0, 0
@@ -447,6 +456,15 @@ make_func(yyscan_t yyscanner, int fnumber, PgBenchExprList *args)
expr_yyerror_more(yyscanner, "odd and >= 3 number of arguments expected",
"case control structure");
}
+ /* special case: hash functions with optional arguments */+ if (PGBENCH_FUNCTIONS[fnumber].nargs == -3)+ {+ int len = elist_length(args);++ if (len < 1 || len > 2)+ expr_yyerror_more(yyscanner, "unexpected number of arguments",+ PGBENCH_FUNCTIONS[fnumber].fname);+ }
expr->etype = ENODE_FUNCTION;
expr->u.function.function = PGBENCH_FUNCTIONS[fnumber].tag;
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.cindex 31ea6ca..6bf94cc 100644--- a/src/bin/pgbench/pgbench.c+++ b/src/bin/pgbench/pgbench.c@@ -61,6 +61,14 @@
#define ERRCODE_UNDEFINED_TABLE "42P01"
/*
+ * Hashing constants+ */+#define FNV_PRIME 0x100000001b3+#define FNV_OFFSET_BASIS 0xcbf29ce484222325+#define MM2_MUL 0xc6a4a7935bd1e995+#define MM2_ROT 47++/*
* Multi-platform pthread implementations
*/
@@ -439,6 +447,8 @@ static int num_scripts; /* number of scripts in sql_script[] */
static int num_commands = 0; /* total number of Command structs */
static int64 total_weight = 0;
+static int hash_seed; /* default seed used in hash functions */+
static int debug = 0; /* debug flag */
/* Builtin test scripts */
@@ -915,6 +925,51 @@ getZipfianRand(TState *thread, int64 min, int64 max, double s)
}
/*
+ * FNV-1a hash function+ */+static int64+getHashFnv1a(int64 val, uint64 seed)+{+ int64 result;+ int i;++ result = FNV_OFFSET_BASIS ^ seed;+ for (i = 0; i < 8; ++i)+ {+ int32 octet = val & 0xff;++ val = val >> 8;+ result = result ^ octet;+ result = result * FNV_PRIME;+ }++ return result;+}++/*+ * Murmur2 hash function+ */+static int64+getHashMurmur2(int64 val, uint64 seed)+{+ uint64 result = seed ^ (sizeof(int64) * MM2_MUL);+ uint64 k = (uint64) val;++ k *= MM2_MUL;+ k ^= k >> MM2_ROT;+ k *= MM2_MUL;++ result ^= k;+ result *= MM2_MUL;++ result ^= result >> MM2_ROT;+ result *= MM2_MUL;+ result ^= result >> MM2_ROT;++ return (int64) result;+}++/*
* Initialize the given SimpleStats struct to all zeroes
*/
static void
@@ -2209,6 +2264,34 @@ evalStandardFunc(
return true;
}
+ /* hashing */+ case PGBENCH_HASH_FNV1A:+ case PGBENCH_HASH_MURMUR2:+ {+ int64 val;+ int64 seed;+ int64 result;++ Assert(nargs >= 1);++ if (!coerceToInt(&vargs[0], &val))+ return false;++ /* read optional seed value */+ if (nargs > 1)+ {+ if (!coerceToInt(&vargs[1], &seed))+ return false;+ }+ else+ seed = hash_seed;++ result = (func == PGBENCH_HASH_FNV1A) ?+ getHashFnv1a(val, seed) : getHashMurmur2(val, seed);+ setIntValue(retval, result);+ return true;+ }+
default:
/* cannot get here */
Assert(0);
@@ -5054,6 +5137,9 @@ main(int argc, char **argv)
INSTR_TIME_SET_CURRENT(start_time);
srandom((unsigned int) INSTR_TIME_GET_MICROSEC(start_time));
+ /* set default seed for hash functions */+ hash_seed = random();+
/* set up thread data structures */
threads = (TState *) pg_malloc(sizeof(TState) * nthreads);
nclients_dealt = 0;
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.hindex 0705ccd..6983865 100644--- a/src/bin/pgbench/pgbench.h+++ b/src/bin/pgbench/pgbench.h@@ -97,7 +97,9 @@ typedef enum PgBenchFunction
PGBENCH_LE,
PGBENCH_LT,
PGBENCH_IS,
- PGBENCH_CASE+ PGBENCH_CASE,+ PGBENCH_HASH_FNV1A,+ PGBENCH_HASH_MURMUR2
} PgBenchFunction;
typedef struct PgBenchExpr PgBenchExpr;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: i.musin@postgrespro.ru, coelho@cri.ensmp.fr
Subject: Re: General purpose hashing func in pgbench
In-Reply-To: <43a8fbbb-32fa-6478-30a9-f64041adf019@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox