Re: pgbench - add pseudo-random permutation function

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Hironobu SUZUKI <hironobu@interdb.jp>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, David Steele <david@pgmasters.net>
Date: 2019-07-23T07:44:09Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pgbench: Function to generate random permutations.

  2. Add basic support for using the POPCNT and SSE4.2s LZCNT opcodes

  3. Further improve code for probing the availability of ARM CRC instructions.

Attachments

Hello Thomas,

>>> Function nbits(), which was previously discussed, has been simplified by
>>> using the function pg_popcount64().
>
> Hi Fabien, Suzuki-san,
>
> I am not smart enough to commit this

I'm in no hurry:-)

> or judge its value for benchmarking,

I think that it is valuable given that we have non uniform random 
generators in pgbench.

I'm wondering about the modular_multiply manual implementation which adds 
quite a few lines of non trivial code. If int128 is available on all/most 
platforms, it could be removed and marked as not supported on these, 
although it would create an issue with non regression tests.

> but I have a few trivial comments on the language:
>
> +    It allows to mix the output of non uniform random functions so that
>
> "It allows the output of non-uniform random functions to be mixed so that"

Fixed.

> +    ensures that a perfect permutation is applied: there are no collisions
> +    nor holes in the output values.
>
> "neither collisions nor holes", or "no collisions or holes"

I choose the first.

> +    The function errors if size is not positive.
>
> "raises an error"

Fixed.

> + * 24 bits mega primes from https://primes.utm.edu/lists/small/millions/
>
> "24 bit mega primes"

Fixed.

> +/* length of n binary representation */
> +static int
> +nbits(uint64 n)
> +{
> +    /* set lower bits to 1 and count them */
> +    return pg_popcount64(compute_mask(n));
> +}
>
> I suppose you could use n == 0 ? 0 : pg_leftmost_one_pos64(n) + 1, and then...

It would create a branch, that I'm trying to avoid.

> +/* return smallest mask holding n  */
> +static uint64
> +compute_mask(uint64 n)
> +{
> +    n |= n >> 1;
> +    n |= n >> 2;
> +    n |= n >> 4;
> +    n |= n >> 8;
> +    n |= n >> 16;
> +    n |= n >> 32;
> +    return n;
> +}
>
> ... here you could use 1 << nbits(n)) - 1.  I have no idea if doing it
> that way around is better, it's just a thought and removes a few lines
> of bit-swizzling code.

This would create a infinite recursion as nbits currently uses 
compute_mask. The 6 bitfield operation above is pretty efficient, I'd let 
it at that.

Attached a v16.

-- 
Fabien.