Re: Fix performance of generic atomics

From: Sokolov Yura <funny(dot)falcon(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fix performance of generic atomics
Date: 2017-05-25 15:12:44
Message-ID: 626c2f135acc668e56905e47f004f216@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Fix-performance-of-Atomics-generic-implementation.patch text/x-diff 5.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-05-25 15:13:25 Re: [POC] hash partitioning
Previous Message Michael Paquier 2017-05-25 14:52:23 Re: Server ignores contents of SASLInitialResponse