Re: better atomics - Mailing list pgsql-hackers

On 2013-10-28 16:29:35 -0400, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2013-10-28 16:06:47 -0400, Tom Lane wrote:
> >> You're both just handwaving.  How many is "many", and which ones might
> >> we actually have enough use for to justify dealing with such a dependency?
> >> I don't think we should buy into this without some pretty concrete
> >> justification.
> 
> > Well, I don't think either of us is suggesting to make it required. But
> > it's going to be painful to go through the list of compilers repeatedly
> > instead of just adding all operations at one. I am willing to do that
> > for several compilers once, but I don't want to do it in each and every
> > feature submission using another atomic op.
> 
> Basically I'm arguing that that choice is backwards.  We should decide
> first what the list of supported atomics is going to be, and each and
> every element of that list has to have a convincing concrete argument
> why we need to support it.

The list I have previously suggested was:

* pg_atomic_load_u32(uint32 *)
* uint32 pg_atomic_store_u32(uint32 *)

To be able to write code without spreading volatiles all over while
making it very clear that that part of the code is worthy of attention.

* uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
* bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32 newval)

Used in most lockfree algorithms, can be used to provide fallbacks for
when any of the other 32bit operations is not implemented for a platform
(so it's easier to bring up a platform).
E.g. used in in the wait-free LW_SHARED patch.

* uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add)

Plain math.
E.g. used in in the wait-free LW_SHARED patch.

* uint32 pg_atomic_fetch_and_u32(uint32 *ptr, uint32 add)
* uint32 pg_atomic_fetch_or_u32(uint32 *ptr, uint32 add)

Useful for (un-)setting flags atomically. Needed for (my approach to)
spinlock-less Pin/UnpinBuffer.

* u64 variants of the above

lockfree list manipulation need at least pointer width operations of at
least compare_exchange().

I *personally* don't have a case for 8byte math yet, but I am pretty
sure parallel sort might be interested in it.

* bool pg_atomic_test_set(void *ptr)
* void pg_atomic_clear(void *ptr)

Can be used as the implementation for spinlocks based on compiler
intrinsics if no native implementation is existing. Makes it easier to
bring up new platforms.

Wrappers around the above:
* uint32 pg_atomic_fetch_sub_u32(uint32 *ptr, uint32 add)
Generic wrapper that imo makes the code easier to read. No
per-platform/compiler overhead.

* pg_atomic_add_until_u32(uint32 *ptr, uint32 val, uint32 limit)
Useful for managing refcounts like pin counts.

* pg_atomic_(add|sub|and|or)_fetch_u32()
Generic wrappers for more legible code.  No
per-platform/compiler overhead.

* pg_atomic_compare_and_swap_128()

Many algorithms are faster (e.g. some lockless hashtables, which'd be
great for the buffer mapping) when a double-pointer-width CAS is
available, but also work with an pointer-width CAS in a less efficient
manner.

> Or if you like: no matter what you do, the first feature submission
> that makes use of a given atomic op is going to suffer pain.  I don't
> want to still be suffering that pain two or three years out.  The shorter
> the list of allowed atomic ops, the sooner we'll be done with debugging
> them.

Yea. As, partially, even evidenced by this discussion ;).

Which would you like to see go?

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services