better atomics - Mailing list pgsql-hackers

On Wed, Oct 16, 2013 at 12:52 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> Partially that only works because we sprinkle 'volatile's on lots of
> places. That can actually hurt performance... it'd usually be something
> like:
> #define pg_atomic_load(uint32) (*(volatile uint32 *)(atomic))
>
> Even if not needed in some places because a variable is already
> volatile, it's very helpful in pointing out what happens and where you
> need to be careful.

That's fine with me.

>> > * uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
>> > * bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32 newval)
>> > * uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)
>> > * uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
>> > * uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
>> > * uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)
>>
>> Do we really need all of those?  Compare-and-swap is clearly needed,
>> and fetch-and-add is also very useful.  I'm not sure about the rest.
>> Not that I necessarily object to having them, but it will be a lot
>> more work.
>
> Well, _sub can clearly be implemented with _add generically. I find code
> using _sub much easier to read than _add(-whatever), but that's maybe
> just me.

Yeah, I wouldn't bother with _sub.

> But _and, _or are really useful because they can be used to atomically
> clear and set flag bits.

Until we have some concrete application that requires this
functionality for adequate performance, I'd be inclined not to bother.I think we have bigger fish to fry, and (to
extendmy nautical
 
metaphor) trying to get this working everywhere might amount to trying
to boil the ocean.

>> > * u64 variants of the above
>> > * bool pg_atomic_test_set(void *ptr)
>> > * void pg_atomic_clear(void *ptr)
>>
>> What are the intended semantics here?
>
> It's basically TAS() without defining whether setting a value that needs
> to be set is a 1 or a 0. Gcc provides this and I think we should make
> our spinlock implementation use it if there is no special cased
> implementation available.

I'll reserve judgement until I see the patch.

>> More general question: how do we move the ball down the field in this
>> area?  I'm willing to do some of the legwork, but I doubt I can do all
>> of it, and I really think we need to make some progress here soon, as
>> it seems that you and I and Ants are all running into the same
>> problems in slightly different ways.  What's our strategy for getting
>> something done here?
>
> That's a pretty good question.
>
> I am willing to write the gcc implementation, plus the generic wrappers
> and provide the infrastructure to override it per-platform. I won't be
> able to do anything about non-linux, non-gcc platforms in that timeframe
> though.
>
> I was thinking of something like:
> include/storage/atomic.h
> include/storage/atomic-gcc.h
> include/storage/atomic-msvc.h
> include/storage/atomic-acc-ia64.h
> where atomic.h first has a list of #ifdefs including the specialized
> files and then lots of #ifndef providing generic variants if not
> already provided by the platorm specific file.

Seems like we might want to put some of this in one of the various
directories called "port", or maybe a new subdirectory of one of them.This basically sounds sane, but I'm unwilling to
committo dropping
 
support for all obscure platforms we currently support unless there
are about 500 people +1ing the idea, so I think we need to think about
what happens when we don't have platform support for these primitives.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company