From: Robert Haas Date: Wed, 3 Aug 2011 20:26:40 +0000 (-0400) Subject: Make pgbench use erand48() rather than random(). X-Git-Tag: REL9_2_BETA1~1335 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4af43ee3f165c8e4b332a7e680a44f4b7ba2d3c1;p=postgresql Make pgbench use erand48() rather than random(). glibc renders random() thread-safe by wrapping a futex lock around it; testing reveals that this limits the performance of pgbench on machines with many CPU cores. Rather than switching to random_r(), which is only available on GNU systems and crashes unless you use undocumented alchemy to initialize the random state properly, switch to our built-in implementation of erand48(), which is both thread-safe and concurrent. Since the list of reasons not to use the operating system's erand48() is getting rather long, rename ours to pg_erand48() (and similarly for our implementations of lrand48() and srand48()) and just always use those. We were already doing this on Cygwin anyway, and the glibc implementation is not quite thread-safe, so pgbench wouldn't be able to use that either. Per discussion with Tom Lane. --- diff --git a/configure b/configure index 0e537d94f8..ddc25d53ac 100755 --- a/configure +++ b/configure @@ -20493,8 +20493,7 @@ LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` - -for ac_func in crypt erand48 getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul +for ac_func in crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul do as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5 @@ -21010,17 +21009,6 @@ esac fi -# Cygwin's erand48() is broken (always returns zero) in some releases, -# so force use of ours. -if test "$PORTNAME" = "cygwin"; then - case " $LIBOBJS " in - *" erand48.$ac_objext "* ) ;; - *) LIBOBJS="$LIBOBJS erand48.$ac_objext" - ;; -esac - -fi - # Win32 support if test "$PORTNAME" = "win32"; then diff --git a/configure.in b/configure.in index 150f9a53d9..a844afc67f 100644 --- a/configure.in +++ b/configure.in @@ -1311,7 +1311,7 @@ fi pgac_save_LIBS="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -AC_REPLACE_FUNCS([crypt erand48 getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul]) +AC_REPLACE_FUNCS([crypt getopt getrusage inet_aton random rint srandom strdup strerror strlcat strlcpy strtol strtoul]) case $host_os in @@ -1362,12 +1362,6 @@ if test "$PORTNAME" = "win32"; then AC_LIBOBJ(getopt_long) fi -# Cygwin's erand48() is broken (always returns zero) in some releases, -# so force use of ours. -if test "$PORTNAME" = "cygwin"; then - AC_LIBOBJ(erand48) -fi - # Win32 support if test "$PORTNAME" = "win32"; then AC_REPLACE_FUNCS(gettimeofday) diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c index 9d57436aeb..56dab6192d 100644 --- a/contrib/pgbench/pgbench.c +++ b/contrib/pgbench/pgbench.c @@ -198,6 +198,7 @@ typedef struct instr_time start_time; /* thread start time */ instr_time *exec_elapsed; /* time spent executing cmds (per Command) */ int *exec_count; /* number of cmd executions (per Command) */ + unsigned short random_state[3]; /* separate randomness for each thread */ } TState; #define INVALID_THREAD ((pthread_t) 0) @@ -380,13 +381,18 @@ usage(const char *progname) /* random number generator: uniform distribution from min to max inclusive */ static int -getrand(int min, int max) +getrand(TState *thread, int min, int max) { /* * Odd coding is so that min and max have approximately the same chance of * being selected as do numbers between them. + * + * pg_erand48() is thread-safe and concurrent, which is why we use it + * rather than random(), which in glibc is non-reentrant, and therefore + * protected by a mutex, and therefore a bottleneck on machines with many + * CPUs. */ - return min + (int) (((max - min + 1) * (double) random()) / (MAX_RANDOM_VALUE + 1.0)); + return min + (int) ((max - min + 1) * pg_erand48(thread->random_state)); } /* call PQexec() and exit() on failure */ @@ -901,7 +907,7 @@ top: if (commands[st->state] == NULL) { st->state = 0; - st->use_file = getrand(0, num_files - 1); + st->use_file = getrand(thread, 0, num_files - 1); commands = sql_files[st->use_file]; } } @@ -1068,9 +1074,9 @@ top: } #ifdef DEBUG - printf("min: %d max: %d random: %d\n", min, max, getrand(min, max)); + printf("min: %d max: %d random: %d\n", min, max, getrand(thread, min, max)); #endif - snprintf(res, sizeof(res), "%d", getrand(min, max)); + snprintf(res, sizeof(res), "%d", getrand(thread, min, max)); if (!putVariable(st, argv[0], argv[1], res)) { @@ -2242,6 +2248,9 @@ main(int argc, char **argv) thread->tid = i; thread->state = &state[nclients / nthreads * i]; thread->nstate = nclients / nthreads; + thread->random_state[0] = random(); + thread->random_state[1] = random(); + thread->random_state[2] = random(); if (is_latencies) { @@ -2384,7 +2393,7 @@ threadRun(void *arg) Command **commands = sql_files[st->use_file]; int prev_ecnt = st->ecnt; - st->use_file = getrand(0, num_files - 1); + st->use_file = getrand(thread, 0, num_files - 1); if (!doCustom(thread, st, &result->conn_time, logfile)) remains--; /* I've aborted */ @@ -2583,17 +2592,6 @@ pthread_create(pthread_t *thread, if (duration > 0) setalarm(duration); - /* - * Set a different random seed in each child process. Otherwise they all - * inherit the parent's state and generate the same "random" sequence. (In - * the threaded case, the different threads will obtain subsets of the - * output of a single random() sequence, which should be okay for our - * purposes.) - */ - INSTR_TIME_SET_CURRENT(start_time); - srandom(((unsigned int) INSTR_TIME_GET_MICROSEC(start_time)) + - ((unsigned int) getpid())); - ret = start_routine(arg); write(th->pipes[1], ret, sizeof(TResult)); close(th->pipes[1]); diff --git a/src/backend/optimizer/geqo/geqo_random.c b/src/backend/optimizer/geqo/geqo_random.c index 6d04c8ec2f..2d8cb6e242 100644 --- a/src/backend/optimizer/geqo/geqo_random.c +++ b/src/backend/optimizer/geqo/geqo_random.c @@ -36,5 +36,5 @@ geqo_rand(PlannerInfo *root) { GeqoPrivateData *private = (GeqoPrivateData *) root->join_search_private; - return erand48(private->random_state); + return pg_erand48(private->random_state); } diff --git a/src/backend/optimizer/geqo/geqo_selection.c b/src/backend/optimizer/geqo/geqo_selection.c index af70c735df..642ac9319a 100644 --- a/src/backend/optimizer/geqo/geqo_selection.c +++ b/src/backend/optimizer/geqo/geqo_selection.c @@ -64,9 +64,9 @@ geqo_selection(PlannerInfo *root, Chromosome *momma, Chromosome *daddy, * Ensure we have selected different genes, except if pool size is only * one, when we can't. * - * This code has been observed to hang up in an infinite loop when the - * platform's implementation of erand48() is broken. We consider that a - * feature: it lets you know you'd better fix the random-number generator. + * This code was observed to hang up in an infinite loop when the + * platform's implementation of erand48() was broken. We now always + * use our own version. */ if (pool->size > 1) { diff --git a/src/include/optimizer/geqo.h b/src/include/optimizer/geqo.h index 62f1fd9418..2dd28253e1 100644 --- a/src/include/optimizer/geqo.h +++ b/src/include/optimizer/geqo.h @@ -73,7 +73,7 @@ extern double Geqo_seed; /* 0 .. 1 */ typedef struct { List *initial_rels; /* the base relations we are joining */ - unsigned short random_state[3]; /* state for erand48() */ + unsigned short random_state[3]; /* state for pg_erand48() */ } GeqoPrivateData; diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 24b951e7bd..09a014b385 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -149,9 +149,6 @@ /* Define to 1 if you have the header file. */ #undef HAVE_EDITLINE_READLINE_H -/* Define to 1 if you have the `erand48' function. */ -#undef HAVE_ERAND48 - /* Define to 1 if you have the `ERR_set_mark' function. */ #undef HAVE_ERR_SET_MARK diff --git a/src/include/port.h b/src/include/port.h index 2cab65fbde..d9d61d0083 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -380,12 +380,9 @@ extern off_t ftello(FILE *stream); #endif #endif -#ifndef HAVE_ERAND48 -/* we assume all of these are present or missing together */ -extern double erand48(unsigned short xseed[3]); -extern long lrand48(void); -extern void srand48(long seed); -#endif +extern double pg_erand48(unsigned short xseed[3]); +extern long pg_lrand48(void); +extern void pg_srand48(long seed); #ifndef HAVE_FSEEKO #define fseeko(a, b, c) fseek(a, b, c) diff --git a/src/port/Makefile b/src/port/Makefile index 60295dcdef..ffbe95effc 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -30,8 +30,8 @@ include $(top_builddir)/src/Makefile.global override CPPFLAGS := -I$(top_builddir)/src/port -DFRONTEND $(CPPFLAGS) LIBS += $(PTHREAD_LIBS) -OBJS = $(LIBOBJS) chklocale.o dirmod.o exec.o inet_net_ntop.o noblock.o \ - path.o pgcheckdir.o pgmkdirp.o pgsleep.o pgstrcasecmp.o \ +OBJS = $(LIBOBJS) chklocale.o dirmod.o erand48.o exec.o inet_net_ntop.o \ + noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o pgstrcasecmp.o \ qsort.o qsort_arg.o sprompt.o thread.o # foo_srv.o and foo.o are both built from foo.c, but only foo.o has -DFRONTEND diff --git a/src/port/erand48.c b/src/port/erand48.c index 64db7a5376..1c0eb467f6 100644 --- a/src/port/erand48.c +++ b/src/port/erand48.c @@ -2,10 +2,14 @@ * * erand48.c * - * This file supplies versions of erand48(), lrand48(), and srand48() - * for machines that lack them. (These are all the members of the drand48 - * family that Postgres currently requires. We name the file after erand48 - * because that is the one that configure tests for.) + * This file supplies pg_erand48(), pg_lrand48(), and pg_srand48(), which + * are just like erand48(), lrand48(), and srand48() except that we use + * our own implementation rather than the one provided by the operating + * system. We used to test for an operating system version rather than + * unconditionally using our own, but (1) some versions of Cygwin have a + * buggy erand48() that always returns zero and (2) as of 2011, glibc's + * erand48() is strangely coded to be almost-but-not-quite thread-safe, + * which doesn't matter for the backend but is important for pgbench. * * * Copyright (c) 1993 Martin Birgmeier @@ -72,7 +76,7 @@ _dorand48(unsigned short xseed[3]) double -erand48(unsigned short xseed[3]) +pg_erand48(unsigned short xseed[3]) { _dorand48(xseed); return ldexp((double) xseed[0], -48) + @@ -81,14 +85,14 @@ erand48(unsigned short xseed[3]) } long -lrand48(void) +pg_lrand48(void) { _dorand48(_rand48_seed); return ((long) _rand48_seed[2] << 15) + ((long) _rand48_seed[1] >> 1); } void -srand48(long seed) +pg_srand48(long seed) { _rand48_seed[0] = RAND48_SEED_0; _rand48_seed[1] = (unsigned short) seed; diff --git a/src/port/random.c b/src/port/random.c index 0fe492b2c6..973108e6c4 100644 --- a/src/port/random.c +++ b/src/port/random.c @@ -21,5 +21,5 @@ long random() { - return lrand48(); + return pg_lrand48(); } diff --git a/src/port/srandom.c b/src/port/srandom.c index d70e3d855f..8a25a21e92 100644 --- a/src/port/srandom.c +++ b/src/port/srandom.c @@ -21,5 +21,5 @@ void srandom(unsigned int seed) { - srand48((long int) seed); + pg_srand48((long int) seed); }