Re: pgbench - add pseudo-random permutation function

Hello Hironobu-san,

> I reviewed pgbench-prp-func-6.patch.

Thanks.

> (1) modular_multiply()
> In modular_multiply(), the numbers of digits of inputs are checked in order
> to determine using the interleaved modular multiplication algorithm or not.
> However, the check condition absolutely depends on the implementation of
> pseudorandom_perm() even though modular_multiply() is a general function.
>
> I think it is better that pseudorandom_perm() directly checks the condition
> because it can be worked efficiently and modular_multiply() can be used in
> general purpose.

You moved the shortcut to the caller. Why not, it removes one modulo
operation and avoids the call altogether.

> (2) pseudorandom_perm() and 001_pgbench_with_server.pl
> Reading the discussion of __builtin_xxx functions, I remembered another
> overflow issue.
>
> pseudorandom_perm() uses the Donald Knuth's linear congruential generator
> method to obtain pseudo-random numbers. This method, not only this but also
> all linear congruential generator methods, always overflows.

> If we do not need to guarantee the portability of this code,

ISTM that we do not need such changes: the code is already portable
because standard C unsigned operations overflow consistently and this is
what it really expected for the linear congruential generator.

If an alternate implementation should be provided, given the heavy cost of
the modular multiplication function, it would be only for those
architectures which fails, detected at configure time. I would not go into
this without an actual failing architecture & C compiler.

Also, the alternate implementation should not change the result, so
something looks amiss in your version. Moreover, I'm unclear how to
implement an overflow multiply with the safe no overflow version.

Here is a v8 which:
- moves the shortcut to the caller
- changes the r formula as you did
- does nothing about Knuth's formula, as nothing should be needed
- fixes tests after Peter's exit status changes

--
Fabien.