Re: Fix performance of generic atomics

Good day, every one.

I'm just posting benchmark numbers for atomics patch.

Hardware: 4 socket 72 core (144HT) x86_64 Centos 7.1
postgresql.conf tuning:
shared_buffers = 32GB
fsync = on
synchronous_commit = on
full_page_writes = off
wal_buffers = 16MB
wal_writer_flush_after = 16MB
commit_delay = 2
max_wal_size = 16GB

Results:
pgbench -i -s 300 + pgbench --skip-some-updates

Clients | master | atomics
========+=========+=======
50 | 53.1k | 53.2k
100 | 101.2k | 103.5k
150 | 119.1k | 121.9k
200 | 128.7k | 132.5k
252 | 120.2k | 130.0k
304 | 100.8k | 115.9k
356 | 78.1k | 90.1k
395 | 70.2k | 79.0k
434 | 61.6k | 70.7k

Also graph with more points attached.

On 2017-05-25 18:12, Sokolov Yura wrote:
> Hello, Tom.
>
> I agree that lonely semicolon looks bad.
> Applied your suggestion for empty loop body (/* skip */).
>
> Patch in first letter had while(true), but I removed it cause
> I think it is uglier:
> - `while(true)` was necessary for grouping read with `if`,
> - but now there is single statement in a loop body and it is
> condition for loop exit, so it is clearly just a loop.
>
> Optimization is valid cause compare_exchange always store old value
> in `old` variable in a same atomic manner as atomic read.
>
> Tom Lane wrote 2017-05-25 17:39:
>> Sokolov Yura <funny(dot)falcon(at)postgrespro(dot)ru> writes:
>> @@ -382,12 +358,8 @@ static inline uint64
>> pg_atomic_fetch_and_u64_impl(volatile pg_atomic_uint64 *ptr, uint64
>> and_)
>> {
>> uint64 old;
>> - while (true)
>> - {
>> - old = pg_atomic_read_u64_impl(ptr);
>> - if (pg_atomic_compare_exchange_u64_impl(ptr, &old, old & and_))
>> - break;
>> - }
>> + old = pg_atomic_read_u64_impl(ptr);
>> + while (!pg_atomic_compare_exchange_u64_impl(ptr, &old, old & and_));
>> return old;
>> }
>> #endif
>>
>> FWIW, I do not think that writing the loops like that is good style.
>> It looks like a typo and will confuse readers. You could perhaps
>> write the same code with better formatting, eg
>>
>> while (!pg_atomic_compare_exchange_u64_impl(ptr, &old, old & and_))
>> /* skip */ ;
>>
>> but why not leave the formulation with while(true) and a break alone?
>>
>> (I take no position on whether moving the read of "old" outside the
>> loop is a valid optimization.)
>>
>> regards, tom lane

With regards,
--
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres Company