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.fal...@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
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to