Fabien COELHO wrote 2021-07-03 11:45:
And a v5 where an unused test file does also compile if we insist.
About patch:
1. PostgreSQL source uses `uint64` and `uint32`, but not
`uint64_t`/`uint32_t`
2. I don't see why pg_prng_state could not be `typedef uint64
pg_prng_state[2];`
3. Then SamplerRandomState and pgbench RandomState could stay.
Patch will be a lot shorter.
I don't like mix of semantic refactoring and syntactic refactoring in
the
same patch.
While I could agree with replacing `SamplerRandomState =>
pg_prng_state`, I'd
rather see it in separate commit.
And that separate commit could contain transition:
`typedef uint64 pg_prng_state[2];` => `typedef struct { uint64 s0, s1
} pg_prng_state;`
4. There is no need in ReservoirStateData->randstate_initialized. There
could
be macros/function:
`bool pg_prng_initiated(state) { return (state[0]|state[1]) != 0; }`
5. Is there need for 128bit prng at all? At least 2*64bit.
There are 2*32bit xoroshiro64
https://prng.di.unimi.it/xoroshiro64starstar.c
And there is 4*32bit xoshiro128:
https://prng.di.unimi.it/xoshiro128plusplus.c
32bit operations are faster on 32bit platforms.
But 32bit platforms are quite rare in production this days.
Therefore I don't have strong opinion on this.
regards,
Sokolov Yura.
