Re: pgbench - add pseudo-random permutation function

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: hironobu(at)interdb(dot)jp, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench - add pseudo-random permutation function
Date: 2018-09-26 04:49:09
Message-ID: CAEepm=2Z7yuLTNuoo3FdSD+y-Hj6pCJGknXqT6a6URx4Sc1u5Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 19, 2018 at 7:07 AM Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> > I reviewed 'pgbench-prp-func/pgbench-prp-func-4.patch'. [...] I thinks
> > this patch is fine.

modular_multiply() is an interesting device. I will leave this to
committers with a stronger mathematical background than me, but I have
a small comment in passing:

+#ifdef HAVE__BUILTIN_POPCOUNTLL
+ return __builtin_popcountll(n);
+#else /* use clever no branching bitwise operator version */

I think it is not enough for the compiler to have
__builtin_popcountll(). The CPU that this is eventually executed on
must actually have the POPCNT instruction[1] (or equivalent on ARM,
POWER etc), or the program will die with SIGILL. There are CPUs in
circulation produced in this decade that don't have it. I have
previously considered something like this[2], but realised you would
therefore need a runtime check. There are a couple of ways to do
that: see commit a7a73875 for one example, also
__builtin_cpu_supports(), and direct CPU ID bit checks (some
platforms). There is also the GCC "multiversion" system that takes
care of that for you but that worked only for C++ last time I checked.

[1] https://en.wikipedia.org/wiki/SSE4#POPCNT_and_LZCNT
[2] https://www.postgresql.org/message-id/CAEepm%3D3g1_fjJGp38QGv%2B38BC2HHVkzUq6s69nk3mWLgPHqC3A%40mail.gmail.com

--
Thomas Munro
http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-09-26 06:29:16 Re: [PATCH] Add regress test for pg_read_all_stats role
Previous Message Michael Paquier 2018-09-26 03:41:38 Re: pgsql: Remove absolete function TupleDescGetSlot().