Re: Atomics hardware support table & supported architectures - Mailing list pgsql-hackers

On Mon, Jun 23, 2014 at 11:16 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> This criterion has been proposed before, but I'm not sure I really
>> agree with it.  If having code around that targets obscure platforms
>> is hindering the development of new features, then that's a reason to
>> get rid of it.
>
> I think that's the case.

At some level, I agree, but see below.

> That we still have !PG_USE_INLINE support although all buildfarm animals
> support it since 4c8aa8b (fixing acc) causes absurd constructs
> (STATIC_IF_INLINE) and fugly macro usage making it harder to read
> and modify code. I spend a good chunk of time just to make that work. Even if
> nobody's going to ever use it.

I expect that we're eventually going to make a decision to require
inline support, but that commit was only last month, so probably not
just yet.  We've been trying hard to maintain support for C89, but I
think even Tom is starting to wonder whether we ought to let a few
widely-implemented post-C89 features in.

> That we need fallback for older gccs instead of relying on somebody else
> having done the atomics abstraction causes significant waste of time.

I agree, but I feel that's time well-spent.  gcc is not the only
compiler, not everyone can or wants to run bleeding-edge versions, and
I don't trust the gcc implementors to a sufficient degree to throw
away our existing TAS implementations wholesale and rely on theirs
instead.  Building cross-platform software is sometimes difficult; but
it's also got a lot of value.  I've spent enough time over the years
working on obscure platforms to value software that works even on
obscure platforms, and I'm glad that PostgreSQL tries to do that, even
if we don't always succeed perfectly.

> That we have support for platforms that we haven't even documented as
> working (e.g. SuperH) or platforms that don't work in the realword
> (m32r) means that that one has to think about and research so many more
> edge cases that it's really hard to make progress. I don't know how
> often I've now sequentially gone through s_lock.h, s_lock.c,
> src/backend/port/tas to understand all the supported combinations.

I agree that s_lock.h is the most painful part of this whole thing,
mostly because I'd really like to get to the point where
SpinLockAcquire() and SpinLockRelease() function as compiler barriers.

>> If we're pretty sure it's useless and doesn't work, so
>> is that.  But if it just hasn't been tested lately, I don't personally
>> think that's a sufficient reason to excise it.
>
> Sure, it just not being tested isn't a reason alone. But it's about more
> than that. I've now spent *far* too much time understanding the idioms
> of architectures that'll never get used again so I don't break them
> accidentally. Sure, that's my choice. But it's imo necessary for
> postgres to scale....
> I don't know about you, but for me reading/understanding/modifying
> assembly on some platform you've never used is *HARD*. And even harder
> if there's nobody around that's going to test it. It's entirely possible
> to write incorrect locking assembly that'll break when used.

Agree.

>> Telling people that
>> they can't have even the most minimal platform support code in
>> PostgreSQL unless they're willing to contribute and maintain a BF VM
>> indefinitely is not very friendly.  Of course, the risk of their
>> platform getting broken is higher if they don't, but that's different
>> than making it a hard requirement.
>
> I agree that we shouldn't actively try to break stuff. But having to
> understand & blindly modify unused code is on the other hand of actively
> breaking platforms. It's actively hindering development.

It's actively hindering a small number of important patches, but most
developers, most of the time, do not need to care.

>> We should be looking at ways to
>> make it easier not harder for people who don't yet run PostgreSQL to
>> start doing so.  We are an open source community; we should try to be,
>> as far as possible, open.
>
> Do you really think supporting platform that 20 people worldwide run for
> fun in their sparetime once in a while (sparc < v8plus) is achieving
> that?
> I think it's more likely that easier to understand code, quick review
> for patches and better testing help with that.

I don't think we can solve this problem with a sledgehammer.  We can
whittle down the number of platforms in s_lock.h that require special
treatment, and eventually some of the odder cases may go away
altogether, and that's great.  But I'd rather not make this patch
dependent on the rate at which we feel comfortable removing cases from
that file.

>> But I think this is all a bit off-topic for this thread.  Andres has
>> already implemented a fallback for people who haven't got CAS and
>> fetch-and-add on their platform, so whether or not we deprecate some
>> more platforms has no bearing at all on this patch.
>
> While I indeed have that fallback code, that's statement is still not
> entirely true. We still need to add atomics support for lots of
> platforms, otherwise they're just going to be 'less supported' than
> now. Are we fine with that and just'll accept patches?

I guess it depends on which platforms are affected.  I certainly don't
think any new facilities we add need to be more comprehensive than
what I did in barrier.h, and in fact I think we could omit a few of
the things I have there, like PA-RISC, Itanium, and Alpha.  And, yeah,
if somebody needs an atomics implementation for a platform we haven't
got yet, they can submit a patch.

>> The question that
>> needs to be resolved in order to move forward here is this: Consider
>> the set of platforms we support, ignoring any that don't have
>> spinlocks.  Do any of them have only one of fetch-and-add, or do they
>> all have both or neither?  If it's the latter, then we're talking
>> about moving from a two-tiered system that looks like this:
>
> Since fetch-and-add is trivially implemented using CAS, there's not much
> need to distinguish between CAS and CAS + fetch_and_add. From my POV the
> restriction to just CAS/fetch_and_add isn't actually buying much. Pretty
> much all platforms but older gcc ones have atomic intrinsics since
> forever. Once you've dug up the documentation for one operation not
> adding two more or so doesn't save much.

Again, the concern that was expressed at the developer's meeting was
that the performance characteristics of the CAS loop might be
significantly different from a native atomic op as to cause us
heartburn.  I think that's a valid concern... but if everything in
common use has both CAS and fetch-and-add, then I think there's
probably no issue here.

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