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.falcon@postgrespro.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