Re: Atomics hardware support table & supported architectures

Hi,

At this year developer's meeting we'd discussed the atomics abstraction
which is necessary for some future improvements. We'd concluded that a
overview over the hardware capabilities of the supported platforms would
be helpful. I've started with that at:
https://wiki.postgresql.org/wiki/Atomics

Does somebody want other columns in there?

From that and previous discussions
(e.g. http://archives.postgresql.org/message-id/20131013004658.GG4056218%40alap2.anarazel.de
) I think we should definitely remove some platforms:
1) VAX. Production ceased 1997. We've never supported OpenVMS, just
netbsd (and probably openbsd)
2) m32r. Our implementation depends on an *unmerged* glibc header. The
website was dead for several *years*, even after the restore patches
can't be downloaded anymore (404).
3) sparcv8: Last released model 1997.
4) i386: Support dropped from windows 98 (yes, really), linux, openbsd
(yes, really), netbsd (yes, really). No code changes needed.
5) pa-risc.
6) armv-v5
Note that this is *not* a requirement for the atomics abstraction - it
now has a fallback to spinlocks if atomics aren't available. But I don't
think we should add code for practically irrelevant and likely not
working setups.

That'd get us to the situation that all supported platforms support
atomic add & cmpxchg.

Just as a recap: The reason I want the atomics abstraction is so we can
implement improvements like
http://www.postgresql.org/message-id/20130926225545.GB26663@awork2.anarazel.de
and quite a few others sanely.

Greetings,

Andres Freund

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

On 2014-06-15 03:12:21 +0200, Andres Freund wrote:
> Hi,
>
> At this year developer's meeting we'd discussed the atomics abstraction
> which is necessary for some future improvements. We'd concluded that a
> overview over the hardware capabilities of the supported platforms would
> be helpful. I've started with that at:
> https://wiki.postgresql.org/wiki/Atomics
>
> Does somebody want other columns in there?
>
> From that and previous discussions
> (e.g. http://archives.postgresql.org/message-id/20131013004658.GG4056218%40alap2.anarazel.de
> ) I think we should definitely remove some platforms:

> 3) sparcv8: Last released model 1997.

Just as a bit of context: < sparcv9 support got dropped from solaris
10. In 2005.

Greetings,

Andres Freund

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

On Sat, Jun 14, 2014 at 9:12 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> At this year developer's meeting we'd discussed the atomics abstraction
> which is necessary for some future improvements. We'd concluded that a
> overview over the hardware capabilities of the supported platforms would
> be helpful. I've started with that at:
> https://wiki.postgresql.org/wiki/Atomics

That looks like a great start. It could use a key explaining what the
columns mean. Here are some guesses, and some comments.

single-copy r/w atomicity = For what word sizes does this platform
have the ability to do atomic reads and writes of values? I don't
understand how "1" can ever NOT be supported - does any architecture
write data to memory a nibble at a time? I also don't really
understand the *** footnote; how can you do kernel emulation of an
atomic load or store.
TAS = Does this platform support test and set? Items in parentheses
are the instructions that implement this.
cmpxchg = Does this platform support compare and swap? If so, for
what word sizes? I assume "n" means "not supported at all" and "y"
means "supported but we don't know for which word sizes". Maybe call
this column "CAS" or "Compare and Swap" rather than using an
Intel-ism.
gcc support from version = Does this mean the version from which GCC
supported the architecture, or from which it supported atomics (which
ones?) on that architecture?

> Does somebody want other columns in there?

I think the main question at the developer meeting was how far we want
to go with supporting primitives like atomic add, atomic and, atomic
or, etc. So I think we should add columns for those.

> From that and previous discussions
> (e.g. http://archives.postgresql.org/message-id/20131013004658.GG4056218%40alap2.anarazel.de
> ) I think we should definitely remove some platforms:
> 1) VAX. Production ceased 1997. We've never supported OpenVMS, just
> netbsd (and probably openbsd)

Based on the letter you linked from the Wiki page, I am inclined to
agree. The letter lists the final ship date for the last surviving
type of VAX as December 31, 2000, with support ending in 2010. It
seems unlikely that anyone will wish to run a version of PostgreSQL
released in 2015 on such hardware (and if they do, we can make that
their problem rather than ours).

> 2) m32r. Our implementation depends on an *unmerged* glibc header. The
> website was dead for several *years*, even after the restore patches
> can't be downloaded anymore (404).

That is also an excellent argument for deprecation.

> 3) sparcv8: Last released model 1997.

I seem to recall hearing about this in a customer situation relatively
recently, so there may be a few of these still kicking around out
there.

> 4) i386: Support dropped from windows 98 (yes, really), linux, openbsd
> (yes, really), netbsd (yes, really). No code changes needed.

Wow, OK. In that case, yeah, let's dump it. But let's make sure we
adequately document that someplace in the code comments, along with
the reasons, because not everyone may realize how dead it is.

> 5) pa-risc.

This is a bit less dead than the other ones; support for existing
system only stopped at the end of 2013. But I don't personally have a
reason to keep it alive.

> 6) armv-v5

I think this is also a bit less dead than the other ones; Red Hat's
shows Bugzilla shows people filing bugs for platform-specific problems
as recently as January of 2013:

https://bugzilla.redhat.com/show_bug.cgi?id=892378

> Note that this is *not* a requirement for the atomics abstraction - it
> now has a fallback to spinlocks if atomics aren't available.

That seems great. Hopefully with a configure option to disable
atomics so that it's easy to test the fallback.

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

On 2014-06-17 13:14:26 -0400, Robert Haas wrote:
> On Sat, Jun 14, 2014 at 9:12 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > At this year developer's meeting we'd discussed the atomics abstraction
> > which is necessary for some future improvements. We'd concluded that a
> > overview over the hardware capabilities of the supported platforms would
> > be helpful. I've started with that at:
> > https://wiki.postgresql.org/wiki/Atomics
>
> That looks like a great start. It could use a key explaining what the
> columns mean. Here are some guesses, and some comments.
>
> single-copy r/w atomicity = For what word sizes does this platform
> have the ability to do atomic reads and writes of values?

Yes. It's a term found in literature, I'm happy to replace it by
something else. It's essentially defined to mean that a read after one
or more writes will only be influenced by one of the last writes, not a
mixture of them. I.e. whether there ever will be intermediate states
visible.

> I don't
> understand how "1" can ever NOT be supported - does any architecture
> write data to memory a nibble at a time?

Actually there seem to be some where that's possible for some operations
(VAX). But the concern is more whether 1 byte can actually be written
without also writing neighbouring values. I.e. there's hardware out
there that'll implement a 1byte store as reading 4 bytes, changing one
of the bytes in a register, and then write the 4 bytes out again. Which
would mean that a neighbouring write will possibly cause a wrong value
to be written...

> I also don't really
> understand the *** footnote; how can you do kernel emulation of an
> atomic load or store.

What happens is that gcc will do a syscall triggering the kernel to turn
of scheduling; perform the math and store the result; turn scheduling on
again. That way there cannot be a other operation influencing the
calculation/store. Imagine if you have hardware that, internally, only
does stores in 4 byte units. Even if it's a single CPU machine, which
most of those are, the kernel could schedule a separate process after
the first 4bytes have been written. Oops. The kernel has ways to prevent
that, userspace doesn't...

> TAS = Does this platform support test and set? Items in parentheses
> are the instructions that implement this.

> cmpxchg = Does this platform support compare and swap? If so, for
> what word sizes? I assume "n" means "not supported at all" and "y"
> means "supported but we don't know for which word sizes". Maybe call
> this column "CAS" or "Compare and Swap" rather than using an
> Intel-ism.

Ok.

> gcc support from version = Does this mean the version from which GCC
> supported the architecture, or from which it supported atomics (which
> ones?) on that architecture?

It means from which version on gcc has support for the __sync intrinsics
on that platform. I've only added versions where support for all atomics
of the supported sizes were available.
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins

Once these answers satisfy you I'll adapt the wiki.

> > Does somebody want other columns in there?
>
> I think the main question at the developer meeting was how far we want
> to go with supporting primitives like atomic add, atomic and, atomic
> or, etc. So I think we should add columns for those.

Well, once CAS is available, atomic add etc is all trivially
implementable - without further hardware support. It might be more
efficient to use the native instruction (e.g. xadd can be much better
than a cmpxchg loop because there's no retries), but that's just
optimization that won't matter unless you have a fair bit of
concurrency.

There's currently fallbacks like:
#ifndef PG_HAS_ATOMIC_FETCH_ADD_U32
#define PG_HAS_ATOMIC_FETCH_ADD_U32
STATIC_IF_INLINE uint32
pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 add_)
{
uint32 old;
while (true)
{
old = pg_atomic_read_u32_impl(ptr);
if (pg_atomic_compare_exchange_u32_impl(ptr, &old, old + add_))
break;
}
return old;
}

> > 3) sparcv8: Last released model 1997.
>
> I seem to recall hearing about this in a customer situation relatively
> recently, so there may be a few of these still kicking around out
> there.

Really? As I'd written in a reply solaris 10 (released 2005) dropped
support for it. Dropping support for a platform that's been desupported
10 years ago by it's manufacturer doesn't sound bad imo...

> > 4) i386: Support dropped from windows 98 (yes, really), linux, openbsd
> > (yes, really), netbsd (yes, really). No code changes needed.
>
> Wow, OK. In that case, yeah, let's dump it. But let's make sure we
> adequately document that someplace in the code comments, along with
> the reasons, because not everyone may realize how dead it is.

I'm generally wondering how to better document the supported os/platform
combinations. E.g. it's not apparent that we only support certain
platforms on a rather limited set of compilers...

Maybe a table with columns like: platform, platform version,
supported-OSs, supported-compilers?

> > 6) armv-v5
>
> I think this is also a bit less dead than the other ones; Red Hat's
> shows Bugzilla shows people filing bugs for platform-specific problems
> as recently as January of 2013:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=892378

Closed as WONTFIX :P.

Joking aside, I think there are still usecases for arm-v5 - but it's
embedded stuff without a real OS and such. Nothing you'd install PG
on. There's distributions that are dropping ARMv6 support already... My
biggest problem is that it's not even documented whether v5 has atomic
4byte stores - while it's documted for v6.

> > Note that this is *not* a requirement for the atomics abstraction - it
> > now has a fallback to spinlocks if atomics aren't available.
>
> That seems great. Hopefully with a configure option to disable
> atomics so that it's easy to test the fallback.

It's a #define right now. Do you think we really need a configure
option?

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-06-17 13:14:26 -0400, Robert Haas wrote:
>> On Sat, Jun 14, 2014 at 9:12 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:

>>> 3) sparcv8: Last released model 1997.
>>
>> I seem to recall hearing about this in a customer situation
>> relatively recently, so there may be a few of these still
>> kicking around out there.
>
> Really? As I'd written in a reply solaris 10 (released 2005)
> dropped support for it. Dropping support for a platform that's
> been desupported 10 years ago by it's manufacturer doesn't sound
> bad imo...

The release of version n doesn't mean that version n-1 is no longer
supported.  As of today, this page:

http://www.oracle.com/technetwork/server-storage/solaris10/overview/index-138972.html

says:

> The Oracle Solaris 9 operating system is now in the Extended
> Support period of it's life cycle. Customers with support
> contracts can still access patches and log new service requests,
> although support contracts carry a surcharge for Oracle Solaris 9
> support.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2014-06-17 13:32:51 -0700, Kevin Grittner wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2014-06-17 13:14:26 -0400, Robert Haas wrote:
> >> On Sat, Jun 14, 2014 at 9:12 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>
> >>> 3) sparcv8: Last released model 1997.
> >>
> >> I seem to recall hearing about this in a customer situation
> >> relatively recently, so there may be a few of these still
> >> kicking around out there.
> >
> > Really? As I'd written in a reply solaris 10 (released 2005)
> > dropped support for it. Dropping support for a platform that's
> > been desupported 10 years ago by it's manufacturer doesn't sound
> > bad imo...
>
> The release of version n doesn't mean that version n-1 is no longer
> supported.  As of today, this page:
>
> http://www.oracle.com/technetwork/server-storage/solaris10/overview/index-138972.html
>
> says:
>
> > The Oracle Solaris 9 operating system is now in the Extended
> > Support period of it's life cycle. Customers with support
> > contracts can still access patches and log new service requests,
> > although support contracts carry a surcharge for Oracle Solaris 9
> > support.

Yea, I know - for another 3 months or so. But that's not actually my
point. It's rather that I think that 10 years after the vendor itself
dropped support for an old cpu version in their new major software
release we can think about dropping it too. In our newest release, not
the old ones.
To be clear I *don't* mean that we shouldn't support solaris 9
anymore. Just that we don't need to support <= 1995 CPUs. That's pre
ultrasparc...

Note that freebsd, debian
(https://www.debian.org/devel/debian-installer/News/2007/20070721),
fedora (http://fedoraproject.org/wiki/Architectures/SPARC), gentoo,
firefox, and lots of others all don't have sparcv8 support anymore.

Greetings,

Andres Freund

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

On 17/06/14 22:32, Kevin Grittner wrote:
> The release of version n doesn't mean that version n-1 is no longer
> supported. As of today, this page:
>
> http://www.oracle.com/technetwork/server-storage/solaris10/overview/index-138972.html
>
> says:
>
>> The Oracle Solaris 9 operating system is now in the Extended
>> Support period of it's life cycle. Customers with support
>> contracts can still access patches and log new service requests,
>> although support contracts carry a surcharge for Oracle Solaris 9
>> support.
>

That will change in October when the Extended Support ends and Solaris 9
enters Sustaining Support (same as 8 is in now).

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

Kevin Grittner wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> wrote:

> The release of version n doesn't mean that version n-1 is no longer
> supported.  As of today, this page:
>
> http://www.oracle.com/technetwork/server-storage/solaris10/overview/index-138972.html
>
> says:
>
> > The Oracle Solaris 9 operating system is now in the Extended
> > Support period of it's life cycle. Customers with support
> > contracts can still access patches and log new service requests,
> > although support contracts carry a surcharge for Oracle Solaris 9
> > support.

Most likely, people will not be installing Postgres 9.5 in that
platform, though, which is what matters for the purposes of the proposal
at hand.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jun 17, 2014 at 1:55 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> But the concern is more whether 1 byte can actually be written
> without also writing neighbouring values. I.e. there's hardware out
> there that'll implement a 1byte store as reading 4 bytes, changing one
> of the bytes in a register, and then write the 4 bytes out again. Which
> would mean that a neighbouring write will possibly cause a wrong value
> to be written...

Ah, OK. I've heard about that before, but had forgotten.

> What happens is that gcc will do a syscall triggering the kernel to turn
> of scheduling; perform the math and store the result; turn scheduling on
> again. That way there cannot be a other operation influencing the
> calculation/store. Imagine if you have hardware that, internally, only
> does stores in 4 byte units. Even if it's a single CPU machine, which
> most of those are, the kernel could schedule a separate process after
> the first 4bytes have been written. Oops. The kernel has ways to prevent
> that, userspace doesn't...

Interesting. "Turn off scheduling" sounds like a pretty dangerous syscall.

>> > Does somebody want other columns in there?
>>
>> I think the main question at the developer meeting was how far we want
>> to go with supporting primitives like atomic add, atomic and, atomic
>> or, etc. So I think we should add columns for those.
>
> Well, once CAS is available, atomic add etc is all trivially
> implementable - without further hardware support. It might be more
> efficient to use the native instruction (e.g. xadd can be much better
> than a cmpxchg loop because there's no retries), but that's just
> optimization that won't matter unless you have a fair bit of
> concurrency.
>
> There's currently fallbacks like:
> #ifndef PG_HAS_ATOMIC_FETCH_ADD_U32
> #define PG_HAS_ATOMIC_FETCH_ADD_U32
> STATIC_IF_INLINE uint32
> pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 add_)
> {
> uint32 old;
> while (true)
> {
> old = pg_atomic_read_u32_impl(ptr);
> if (pg_atomic_compare_exchange_u32_impl(ptr, &old, old + add_))
> break;
> }
> return old;
> }

I understand, but the performance characteristics are quite different.
My understanding from the developer meeting was that we'd be OK with
having, say, three levels of support for atomic ops: all ops
supported, only TAS, none. Or maybe four: all ops, CAS + TAS, only
TAS, none. But I think there was resistance (in which I participate)
to the idea of, say, having platform 1 with "add" but not "and" and
"or", platform 2 with "and" and "or" but not "add", platform 3 with
both, platform 4 with neither, etc. Then it becomes too hard for
developers to predict whether something that is a win on their
platform will be a loss on some other platform.

>> > 3) sparcv8: Last released model 1997.
>>
>> I seem to recall hearing about this in a customer situation relatively
>> recently, so there may be a few of these still kicking around out
>> there.
>
> Really? As I'd written in a reply solaris 10 (released 2005) dropped
> support for it. Dropping support for a platform that's been desupported
> 10 years ago by it's manufacturer doesn't sound bad imo...

We definitely have at least one customer using Solaris 9. I don't
know their architecture for certain, but they did recently install a
new version of PostgreSQL.

>> > 4) i386: Support dropped from windows 98 (yes, really), linux, openbsd
>> > (yes, really), netbsd (yes, really). No code changes needed.
>>
>> Wow, OK. In that case, yeah, let's dump it. But let's make sure we
>> adequately document that someplace in the code comments, along with
>> the reasons, because not everyone may realize how dead it is.
>
> I'm generally wondering how to better document the supported os/platform
> combinations. E.g. it's not apparent that we only support certain
> platforms on a rather limited set of compilers...
>
> Maybe a table with columns like: platform, platform version,
> supported-OSs, supported-compilers?

Sounds worth at try.

>> > 6) armv-v5
>>
>> I think this is also a bit less dead than the other ones; Red Hat's
>> shows Bugzilla shows people filing bugs for platform-specific problems
>> as recently as January of 2013:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=892378
>
> Closed as WONTFIX :P.
>
> Joking aside, I think there are still usecases for arm-v5 - but it's
> embedded stuff without a real OS and such. Nothing you'd install PG
> on. There's distributions that are dropping ARMv6 support already... My
> biggest problem is that it's not even documented whether v5 has atomic
> 4byte stores - while it's documted for v6.

I think in doubtful cases we might as well keep the support in. If
you've got the fallback to non-atomics, keeping the other code around
doesn't hurt much, and might make it easier for someone who is
interested in one of those platforms. It's fine and good to kill
things that are totally dead, but I think it's better for a user of
some obscure platform to find that it doesn't *quite* work than that
we've deliberately broken it. But maybe I am being too conservative.

>> > Note that this is *not* a requirement for the atomics abstraction - it
>> > now has a fallback to spinlocks if atomics aren't available.
>>
>> That seems great. Hopefully with a configure option to disable
>> atomics so that it's easy to test the fallback.
>
> It's a #define right now. Do you think we really need a configure
> option?

Well, we've got one for --disable-spinlocks, so it seems like it would
be a good idea for symmetry. More than that, I actually really hate
things that don't have a configure option, like WAL_DEBUG, because you
have to change a checked-in file, which shows up as a diff, and if
you're not careful you check it in, and if you are careful it still
gets blown away every time you git reset --hard, which I do a lot. I
think the fact that both Heikki and I on separate occasions have made
commits enabling WAL_DEBUG shows pretty clearly the weaknesses of that
method of doing business.

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

On 2014-06-18 11:15:15 -0400, Robert Haas wrote:
> On Tue, Jun 17, 2014 at 1:55 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > What happens is that gcc will do a syscall triggering the kernel to turn
> > of scheduling; perform the math and store the result; turn scheduling on
> > again. That way there cannot be a other operation influencing the
> > calculation/store. Imagine if you have hardware that, internally, only
> > does stores in 4 byte units. Even if it's a single CPU machine, which
> > most of those are, the kernel could schedule a separate process after
> > the first 4bytes have been written. Oops. The kernel has ways to prevent
> > that, userspace doesn't...
>
> Interesting. "Turn off scheduling" sounds like a pretty dangerous syscall.

The kernel does that internally, just during cmpxchg. It'll reenable
interrupts before returning. There's hundreds of places inside at least
linux doing that internally...

Should you be interested in the details:
https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt

> >> > Does somebody want other columns in there?
> >>
> >> I think the main question at the developer meeting was how far we want
> >> to go with supporting primitives like atomic add, atomic and, atomic
> >> or, etc. So I think we should add columns for those.
> >
> > Well, once CAS is available, atomic add etc is all trivially
> > implementable - without further hardware support. It might be more
> > efficient to use the native instruction (e.g. xadd can be much better
> > than a cmpxchg loop because there's no retries), but that's just
> > optimization that won't matter unless you have a fair bit of
> > concurrency.
> >
> > There's currently fallbacks like:
> > #ifndef PG_HAS_ATOMIC_FETCH_ADD_U32
> > #define PG_HAS_ATOMIC_FETCH_ADD_U32
> > STATIC_IF_INLINE uint32
> > pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 add_)
> > {
> > uint32 old;
> > while (true)
> > {
> > old = pg_atomic_read_u32_impl(ptr);
> > if (pg_atomic_compare_exchange_u32_impl(ptr, &old, old + add_))
> > break;
> > }
> > return old;
> > }
>
> I understand, but the performance characteristics are quite different.

There might be cases where that's true, but in the majority of cases
where the variable isn't highly contended it's just about the same. In
many cases the microcode will implement atomic ops using ll/sc
operations internally anyway.

> My understanding from the developer meeting was that we'd be OK with
> having, say, three levels of support for atomic ops: all ops
> supported, only TAS, none. Or maybe four: all ops, CAS + TAS, only
> TAS, none. But I think there was resistance (in which I participate)
> to the idea of, say, having platform 1 with "add" but not "and" and
> "or", platform 2 with "and" and "or" but not "add", platform 3 with
> both, platform 4 with neither, etc. Then it becomes too hard for
> developers to predict whether something that is a win on their
> platform will be a loss on some other platform.

I don't think developers really have to care. In nearly all the cases
the above cmpxchg loop will loop precisely once. Usually the cacheline
will stay in the exclusive state on that process, and that's it. In most
cases what it'll be replacing will be something protected by a spinlock
anyways - and the above isn't any worse than that.

Note that we're far from the only one falling back to cmpxchg for all
these ops - that's pretty much a standard fallback.

I'm fine with always implementing everything but tas, cas, add ontop of
cas for now. I want or/and/sub/... to be conveniently available to C
code, but they don't need to be based on hardware ops. Having open-coded
versions of the above in several places sounds like a horrible idea to
me. Both, because we might want to optimize it at some point and because
it's horrible readability wise.

> >> > 3) sparcv8: Last released model 1997.
> >>
> >> I seem to recall hearing about this in a customer situation relatively
> >> recently, so there may be a few of these still kicking around out
> >> there.
> >
> > Really? As I'd written in a reply solaris 10 (released 2005) dropped
> > support for it. Dropping support for a platform that's been desupported
> > 10 years ago by it's manufacturer doesn't sound bad imo...
>
> We definitely have at least one customer using Solaris 9. I don't
> know their architecture for certain, but they did recently install a
> new version of PostgreSQL.

But Solaris 9 doesn't imply sparcv8. Ultrasparc (first sparcv9) support
was added in solaris 2.5... That's a *long* time ago.

If my googling turned up the right information you needed to use special
-xarch flags for sun studio to even compile binaries that are pure v8
(v8plus is fine btw) since sun studio 9... Same for gcc apparently (-mno-v8plus),
although I don't know how far back.

The reason I'd like to kill it is that there's basically zero chance of
ever testing it and we'd need to write configure check that not only
detect the suncc intrinsics but also runs them and tests whether they
work since they already exist for a long while. Ugly.

> >> > 6) armv-v5

> I think in doubtful cases we might as well keep the support in.

Fair point.

> If
> you've got the fallback to non-atomics, keeping the other code around
> doesn't hurt much, and might make it easier for someone who is
> interested in one of those platforms. It's fine and good to kill
> things that are totally dead, but I think it's better for a user of
> some obscure platform to find that it doesn't *quite* work than that
> we've deliberately broken it. But maybe I am being too conservative.

Well, I think my view is that it's placing burden on us to differentiate
more fine grained between architectures for the very unlikely case that
somebody will ever need it. I'm not that adverse to applying a patch
adding the extra support if somebody shows up actually interested; but
blindly writing code for something that's unlikely to ever be used
sounds like a waste of time.

FWIW, if it's on linux, we don't need to care anyway. Gcc abstracts it
away.

> >> > Note that this is *not* a requirement for the atomics abstraction - it
> >> > now has a fallback to spinlocks if atomics aren't available.
> >>
> >> That seems great. Hopefully with a configure option to disable
> >> atomics so that it's easy to test the fallback.
> >
> > It's a #define right now. Do you think we really need a configure
> > option?
>
> Well, we've got one for --disable-spinlocks, so it seems like it would
> be a good idea for symmetry.

Ok. I'd rather get rid of --disable-spinlocks alltogether, but I don't
really want to fight that fight now :)

> More than that, I actually really hate
> things that don't have a configure option, like WAL_DEBUG, because you
> have to change a checked-in file, which shows up as a diff, and if
> you're not careful you check it in, and if you are careful it still
> gets blown away every time you git reset --hard, which I do a lot. I
> think the fact that both Heikki and I on separate occasions have made
> commits enabling WAL_DEBUG shows pretty clearly the weaknesses of that
> method of doing business.

Why don't you pass it to configure or add it in Makefile.custom? That's
what I do.

Greetings,

Andres Freund

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

On Wed, Jun 18, 2014 at 12:13 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> There might be cases where that's true, but in the majority of cases
> where the variable isn't highly contended it's just about the same. In
> many cases the microcode will implement atomic ops using ll/sc
> operations internally anyway.

But if the variable isn't highly contended, then why are we messing
around with atomic ops in the first place?

> I'm fine with always implementing everything but tas, cas, add ontop of
> cas for now. I want or/and/sub/... to be conveniently available to C
> code, but they don't need to be based on hardware ops. Having open-coded
> versions of the above in several places sounds like a horrible idea to
> me. Both, because we might want to optimize it at some point and because
> it's horrible readability wise.

OK, so if we're only going to have TAS, CAS, and fetch-and-add as
primitives (which sounds sensible to me) and implement the rest in
terms of those for now, then the table on the wiki only needs one more
column: information about support for fetch-and-add.

>> More than that, I actually really hate
>> things that don't have a configure option, like WAL_DEBUG, because you
>> have to change a checked-in file, which shows up as a diff, and if
>> you're not careful you check it in, and if you are careful it still
>> gets blown away every time you git reset --hard, which I do a lot. I
>> think the fact that both Heikki and I on separate occasions have made
>> commits enabling WAL_DEBUG shows pretty clearly the weaknesses of that
>> method of doing business.
>
> Why don't you pass it to configure or add it in Makefile.custom? That's
> what I do.

Yeah, I should probably do that instead. But I still think the point
that two different commiters have managed to flip that flag by
accident is telling.

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

On 2014-06-18 17:04:49 -0400, Robert Haas wrote:
> On Wed, Jun 18, 2014 at 12:13 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > There might be cases where that's true, but in the majority of cases
> > where the variable isn't highly contended it's just about the same. In
> > many cases the microcode will implement atomic ops using ll/sc
> > operations internally anyway.
>
> But if the variable isn't highly contended, then why are we messing
> around with atomic ops in the first place?

a) Quite often the "strange" atomic op is only needed to allow a more
performance critical codepart to use atomic ops (e.g. setting flags
atomically is only required to make atomic pins work).
b) A spinlock protected region is often more likely to take longer than
a single atomic op because it'll often touch more cachelines and
such. For such cmpxchg loops there's pretty much no intermediary
instructions inbetween which the cacheline could be stolen by another
core.
c) The overall amount of bus traffic is often noticeably lower with a
atomic op because the average total number of locked instructions is
lower (unlocking often enough requires another atomic op or barrier)

> > Why don't you pass it to configure or add it in Makefile.custom? That's
> > what I do.
>
> Yeah, I should probably do that instead. But I still think the point
> that two different commiters have managed to flip that flag by
> accident is telling.

Not arguing against having the configure flag here (already decided),
just wondering whether your life could be made easier :)

Greetings,

Andres Freund

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

On 18/06/14 17:15, Robert Haas wrote:
>>>> 6) armv-v5
>>>
>>> I think this is also a bit less dead than the other ones; Red Hat's
>>> shows Bugzilla shows people filing bugs for platform-specific problems
>>> as recently as January of 2013:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=892378
>>
>> Closed as WONTFIX :P.
>>
>> Joking aside, I think there are still usecases for arm-v5 - but it's
>> embedded stuff without a real OS and such. Nothing you'd install PG
>> on. There's distributions that are dropping ARMv6 support already... My
>> biggest problem is that it's not even documented whether v5 has atomic
>> 4byte stores - while it's documted for v6.
>
> I think in doubtful cases we might as well keep the support in. If
> you've got the fallback to non-atomics, keeping the other code around
> doesn't hurt much, and might make it easier for someone who is
> interested in one of those platforms. It's fine and good to kill
> things that are totally dead, but I think it's better for a user of
> some obscure platform to find that it doesn't *quite* work than that
> we've deliberately broken it. But maybe I am being too conservative.
>

I think quite the opposite, it's better to say we don't support the
obscure platform than saying that we do and have no active testing or
proof that it indeed does and somebody finding the hard way that there
are issues.

I also have hard time imagining somebody in 2016 installing brand new
Postgres 9.5 on their 20 year old 200Mhz (or something) machine and
doing something meaningful with it. It's not like we are removing
supported platforms from old releases, but this basically means we are
going to support some of the obscure almost dead platforms till at least
2020, in some cases longer than their creators and even OSes.

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

At 2014-06-19 13:33:03 +0200, petr(at)2ndquadrant(dot)com wrote:
>
> I think quite the opposite, it's better to say we don't support the
> obscure platform than saying that we do and have no active testing or
> proof that it indeed does and somebody finding the hard way that there
> are issues.

Yes, I strongly agree. I've been in the position of having to get things
working on obscure platforms, and was definitely not fond of finding old
rotten code that was kept around "just in case", which nobody actually
cared about (or was familiar with) any more.

Having been on that side of the fence, I don't feel guilty saying that
if someone *really* cares about running the very latest Postgres on an
unsupported platform, they can do some digging around in the archives
and repository and do the necessary legwork.

Let's not pretend to support platforms we have no practical way of
verifying.

-- Abhijit

On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> Let's not pretend to support platforms we have no practical way of
> verifying.

This is key. The buildfarm defines the set of platforms we support.

merlin

Merlin Moncure wrote:
> On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> > Let's not pretend to support platforms we have no practical way of
> > verifying.
>
> This is key. The buildfarm defines the set of platforms we support.

This means that either Renesas should supply us with a bunch of
buildfarm members for their SuperH and M32R stuff, or they should be
considered unsupported as well.

I don't really care all that much about Linux and the glibc situation on
M32R; I mean that platform is weird enough that it might well have its
own, unheard-of Unix clone. But if people expect to use Postgres on it,
we need BF members.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-06-19 11:00:51 -0400, Alvaro Herrera wrote:
> Merlin Moncure wrote:
> > On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> > > Let's not pretend to support platforms we have no practical way of
> > > verifying.
> >
> > This is key. The buildfarm defines the set of platforms we support.
>
> This means that either Renesas should supply us with a bunch of
> buildfarm members for their SuperH and M32R stuff, or they should be
> considered unsupported as well.

Hm. I've missed SuperH in my comparison - it's not actually documented
to be a supported platform...

> I don't really care all that much about Linux and the glibc situation on
> M32R; I mean that platform is weird enough that it might well have its
> own, unheard-of Unix clone.

They have their own compiler for a some realtime os (not posix-y and
discontinued!) - but that doesn't support the gcc syntax used by
s_lock.h. So it's already not supported.

> But if people expect to use Postgres on it, we need BF members.

Yes. But I think it's exceedingly unlikely.

Greetings,

Andres Freund

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

On Thu, Jun 19, 2014 at 10:43 AM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
> On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
>> Let's not pretend to support platforms we have no practical way of
>> verifying.
>
> This is key. The buildfarm defines the set of platforms we support.

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. 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. 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. 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.

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. 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:

1. No spinlocks.
2. Spinlocks.

...to a three-tiered system that looks like this:

1. No atomics at all.
2. Spinlocks but nothing else.
3. Spinlocks, CAS, and fetch-and-add.

I think that's eminently reasonable from a code maintenance and
developer testing perspective. It sounds like all common systems and
architectures would fall in category 3, meaning that people wouldn't
have to worry about (just for example) significant differences between
the atomic ops supported on Intel vs. PPC. For older or more obscure
platforms, categories 2 and 1 would still be there when needed.

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

On 2014-06-23 10:29:54 -0400, Robert Haas wrote:
> On Thu, Jun 19, 2014 at 10:43 AM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
> > On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> >> Let's not pretend to support platforms we have no practical way of
> >> verifying.
> >
> > This is key. The buildfarm defines the set of platforms we support.
>
> 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.

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.

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

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.

> 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.

> 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.

> 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.

> 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?

> 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.

> 1. No spinlocks.
> 2. Spinlocks.
>
> ...to a three-tiered system that looks like this:
>
> 1. No atomics at all.
> 2. Spinlocks but nothing else.
> 3. Spinlocks, CAS, and fetch-and-add.
>
> I think that's eminently reasonable from a code maintenance and
> developer testing perspective. It sounds like all common systems and
> architectures would fall in category 3, meaning that people wouldn't
> have to worry about (just for example) significant differences between
> the atomic ops supported on Intel vs. PPC. For older or more obscure
> platforms, categories 2 and 1 would still be there when needed.

Yea, I think all reasonable systems are in 3). And because of the
fallback to __sync/__atomic intrinsics it's actually much easier to
bring up a new platform after this infrastructure is in place. Once C11
is more widely used it's even going to be just standard C facilities.

Greetings,

Andres Freund

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

On Mon, Jun 23, 2014 at 11:16 AM, Andres Freund <andres(at)2ndquadrant(dot)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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Jun 23, 2014 at 11:16 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> 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.

What I want to know is whether we are going to agree that the set of
atomics is going to be CAS plus fetch_and_add plus *nothing else*.

Andres seems to envision that those will be a minimal set and then
we'll freely use any other atomic op that seems handy as long as we can
provide a fallback implementation of it. I agree with Robert that
keeping track of the actual cross-platform performance characteristics
of such code would be a nightmare.

What I want to see is a list of atomic ops that is short enough that
we can agree we do not care about PG performance on any platform that
hasn't got (fast versions of) all of them. Then we will code and
optimize on the basis of having all those ops and no others. We can
offer fallback implementations of those ops so that older platforms
aren't entirely dead in the water, but they will not be the focus
of any performance testing.

regards, tom lane

On 2014-06-23 12:08:08 -0400, Robert Haas wrote:
> > 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 were in pretty much the same position before - it was just the quiet
inline test tripping us up...

> > 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.

s_lock.h is basically gone in my patch btw. Only old comments + defines
for TAS/TAS_SPIN/S_UNLOCK etc mapping the old calls onto atomic flags
remain.
The new code should, hopefully, all be barrier safe.

> > 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.

True. I guess I just seem to find myself hit by this a bit heavily :)

> > 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.

I'll omit !gcc PA-RISC unless somebody complains. I don't think I'll be
able to modify hpux_hppa.s + build infrastructure correctly and there's
no buildfarm. That means it'll require --disable-spinlocks.
I've now also removed sunstudio*.s because sunstudio has provided
intrinsics for a *long* while. No idea whether it'll work out of the
box, but it shouldn't be hard to mop up eventual bugs.

> >> 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.

Well, there's platforms where both CAS and atomic add are implemented
using load linked/store conditional. So neither really is 100%
native. But that's essentially how CAS et al are implemented in
microcode on pretty much all somewhat recent cpus anyway.

Greetings,

Andres Freund

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

On 2014-06-23 09:28:19 -0700, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Mon, Jun 23, 2014 at 11:16 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> 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.
>
> What I want to know is whether we are going to agree that the set of
> atomics is going to be CAS plus fetch_and_add plus *nothing else*.

It's going to be TAS, CAS, fetch_and_add, right? Since TAS is the only
thing supported on some platforms?

The only op I'm currently wondering about adding is a atomic exchange,
without compare to that set. All platforms that support CAS also have a
non-comparing version of it.

Right now the patch also uses __sync_fetch_and_sub() in the generic gcc
implementation instead of doing the negation itself, but that's easily
"fixable".

> Andres seems to envision that those will be a minimal set and then
> we'll freely use any other atomic op that seems handy as long as we can
> provide a fallback implementation of it.

Well, I *do* also want pg_atomic_fetch_and/or_u32() - but I'm totally
fine with those two only being implemented with CAS. On all
platforms. Otherwise the next scalability patch I'm going to submit will
just have to open code a CAS loop for it which doesn't seem to help.

Greetings,

Andres Freund

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

On Mon, Jun 23, 2014 at 12:29 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > 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.
>
> s_lock.h is basically gone in my patch btw. Only old comments + defines
> for TAS/TAS_SPIN/S_UNLOCK etc mapping the old calls onto atomic flags
> remain.
> The new code should, hopefully, all be barrier safe.

Urp. I'm not sure that's at all a good idea. I don't love s_lock.h,
but if we make a wholesale change, we risk breaking things that work
today in obscure ways that are hard to find and debug. I thought we
were talking about adding new facilities with their own fallbacks, not
massively reengineering the facilities we've already got.

> I'll omit !gcc PA-RISC unless somebody complains. I don't think I'll be
> able to modify hpux_hppa.s + build infrastructure correctly and there's
> no buildfarm. That means it'll require --disable-spinlocks.

I dunno whether we should care in that particular case, but in general
I don't think it's OK for lack of support for the *new* atomics to
prevent us from using the *existing* TAS support.

>> 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.
>
> Well, there's platforms where both CAS and atomic add are implemented
> using load linked/store conditional. So neither really is 100%
> native. But that's essentially how CAS et al are implemented in
> microcode on pretty much all somewhat recent cpus anyway.

I think on platforms that expose LL/SC, we have to assume that
spurious failures will be infrequent enough not to matter to
performance. If they do, that's something that's going to have to be
fixed by the hardware manufacturer, not us. There is a danger here
that we could end up with optimizations that are wins on some
platforms and losses on others, but I think we're going to have to
live with that. Refusing to use atomic ops at all isn't a viable way
forward.

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

On 2014-06-23 12:46:11 -0400, Robert Haas wrote:
> On Mon, Jun 23, 2014 at 12:29 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> > 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.
> >
> > s_lock.h is basically gone in my patch btw. Only old comments + defines
> > for TAS/TAS_SPIN/S_UNLOCK etc mapping the old calls onto atomic flags
> > remain.
> > The new code should, hopefully, all be barrier safe.
>
> Urp. I'm not sure that's at all a good idea.

I'm not sure either. But it got harder and harder to do it without that
because of currently interweaved dependencies. Barriers depend on
spinlocks, atomics depend on barriers, atomics depend on spinlocks. And
for a useful generic spinlock implementation spinlocks depend on
atomics.

And then there's the problem that the spinlock based atomics emulation
needs to influence the semaphore spinlock implementation because you
otherwise can get deadlocks (imagine a atomic var being manipulated
while a spinlock is held. If they map to the same semaphore...).

I guess it's possible to remove all traces of s_lock.h from barrier.h;
add a atomics using fallback to s_lock.h, forceable using a define; make
the the public part of the spinlock based fallback not require the
spinlock implementation somehow.
But even if, I think we should get rid of s_lock.h in a later commit for
9.5.

> I don't love s_lock.h,
> but if we make a wholesale change, we risk breaking things that work
> today in obscure ways that are hard to find and debug. I thought we
> were talking about adding new facilities with their own fallbacks, not
> massively reengineering the facilities we've already got.

I think we can separate it if we want, but I doubt it'll reall make
stuff simpler.

> > I'll omit !gcc PA-RISC unless somebody complains. I don't think I'll be
> > able to modify hpux_hppa.s + build infrastructure correctly and there's
> > no buildfarm. That means it'll require --disable-spinlocks.
>
> I dunno whether we should care in that particular case, but in general
> I don't think it's OK for lack of support for the *new* atomics to
> prevent us from using the *existing* TAS support.

I think I'll just need help from Tom for that case.

Greetings,

Andres Freund

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

On Mon, Jun 23, 2014 at 05:16:15PM +0200, Andres Freund wrote:
> On 2014-06-23 10:29:54 -0400, Robert Haas wrote:
> > 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.

What I'm hearing is that you see two options, (1) personally authoring
e.g. sparcv8 code or (2) purging the source tree of sparcv8 code before
submitting the patch that would otherwise change it. I favor middle ground
that lets minor platforms pay their own way. Write your changes with as
little effort as you wish toward whether they run on sparcv8. If they break
sparcv8, then either (a) that was okay, or (b) a user will show up with a
report and/or patch, and we'll deal with that.

For any minor-platform user sighing now, the community offers an unbeatable
deal on PostgreSQL committer time. Provide a currently-passing buildfarm
member, and no PostgreSQL committer will be content until his new code works
there. How can you pass that up?

(By "break sparcv8", I mean a build failure, test suite failure, or large
performance regression. If a change has the potential to make some
architectures give wrong answers only at odd times, that's a different kind of
problem. For that reason, actively breaking Alpha is a good thing.)

> > 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?

+1

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
> On Mon, Jun 23, 2014 at 05:16:15PM +0200, Andres Freund wrote:
> > On 2014-06-23 10:29:54 -0400, Robert Haas wrote:
> > > 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.
>
> What I'm hearing is that you see two options, (1) personally authoring
> e.g. sparcv8 code or (2) purging the source tree of sparcv8 code before
> submitting the patch that would otherwise change it. I favor middle ground
> that lets minor platforms pay their own way. Write your changes with as
> little effort as you wish toward whether they run on sparcv8. If they break
> sparcv8, then either (a) that was okay, or (b) a user will show up with a
> report and/or patch, and we'll deal with that.

Sounds sensible to me. But we should document such platforms as not
being officially supported in that case.

> If a change has the potential to make some architectures give wrong
> answers only at odd times, that's a different kind of problem. For
> that reason, actively breaking Alpha is a good thing.

Not sure what you mean with the 'actively breaking Alpha' statement?
That we should drop Alpha?

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
>> If a change has the potential to make some architectures give wrong
>> answers only at odd times, that's a different kind of problem. For
>> that reason, actively breaking Alpha is a good thing.

> Not sure what you mean with the 'actively breaking Alpha' statement?
> That we should drop Alpha?

+1. Especially with no buildfarm critter. Would anyone here care
to bet even the price of a burger that Alpha isn't broken already?

Even if we *had* an Alpha in the buildfarm, I'd have pretty small
confidence in whether our code really worked on it. The buildfarm
tests just don't stress heavily-concurrent behavior enough.

regards, tom lane

On Tue, Jun 24, 2014 at 07:09:08PM +0200, Andres Freund wrote:
> On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
> > What I'm hearing is that you see two options, (1) personally authoring
> > e.g. sparcv8 code or (2) purging the source tree of sparcv8 code before
> > submitting the patch that would otherwise change it. I favor middle ground
> > that lets minor platforms pay their own way. Write your changes with as
> > little effort as you wish toward whether they run on sparcv8. If they break
> > sparcv8, then either (a) that was okay, or (b) a user will show up with a
> > report and/or patch, and we'll deal with that.
>
> Sounds sensible to me. But we should document such platforms as not
> being officially supported in that case.

It is usually safe to make the documentation match the facts.

> > If a change has the potential to make some architectures give wrong
> > answers only at odd times, that's a different kind of problem. For
> > that reason, actively breaking Alpha is a good thing.
>
> Not sure what you mean with the 'actively breaking Alpha' statement?
> That we should drop Alpha?

Yes:
http://www.postgresql.org/message-id/CA+TgmoZHgv_gowyFVCRYETihPWNTtK1DYeA-o3f5+puE3TweaQ@mail.gmail.com

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

On 2014-06-24 10:22:08 -0700, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
> >> If a change has the potential to make some architectures give wrong
> >> answers only at odd times, that's a different kind of problem. For
> >> that reason, actively breaking Alpha is a good thing.
>
> > Not sure what you mean with the 'actively breaking Alpha' statement?
> > That we should drop Alpha?
>
> +1. Especially with no buildfarm critter. Would anyone here care
> to bet even the price of a burger that Alpha isn't broken already?

I'd actually be willing to bet a fair amount of money that it already is
broken. Especially in combination with an aggressively optimizing
compiler.

Then let's do that.

> Even if we *had* an Alpha in the buildfarm, I'd have pretty small
> confidence in whether our code really worked on it. The buildfarm
> tests just don't stress heavily-concurrent behavior enough.

Yea.

Greetings,

Andres Freund

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

On 2014-06-24 10:22:08 -0700, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
> >> If a change has the potential to make some architectures give wrong
> >> answers only at odd times, that's a different kind of problem. For
> >> that reason, actively breaking Alpha is a good thing.
>
> > Not sure what you mean with the 'actively breaking Alpha' statement?
> > That we should drop Alpha?
>
> +1. Especially with no buildfarm critter. Would anyone here care
> to bet even the price of a burger that Alpha isn't broken already?

Here's a patch removing alpha/true64/osf/1 support. I think I got most
relevant references, not sure if I missed something.

Since there seems to be (unanimous?) support for dropping alpha and some
patches coming up that need to deal with platform dependent stuff it
seems sensible to do this first.

Greetings,

Andres Freund

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

On Fri, Jun 27, 2014 at 9:59 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-06-24 10:22:08 -0700, Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> > On 2014-06-24 13:03:37 -0400, Noah Misch wrote:
>> >> If a change has the potential to make some architectures give wrong
>> >> answers only at odd times, that's a different kind of problem. For
>> >> that reason, actively breaking Alpha is a good thing.
>>
>> > Not sure what you mean with the 'actively breaking Alpha' statement?
>> > That we should drop Alpha?
>>
>> +1. Especially with no buildfarm critter. Would anyone here care
>> to bet even the price of a burger that Alpha isn't broken already?
>
> Here's a patch removing alpha/true64/osf/1 support. I think I got most
> relevant references, not sure if I missed something.
>
> Since there seems to be (unanimous?) support for dropping alpha and some
> patches coming up that need to deal with platform dependent stuff it
> seems sensible to do this first.

I have noticed that most PostgreSQL committers seem for format their
commit messages so that paragraphs are separated by a blank line, but
you seem not to do that. I find that less readable.

I don't personally object to dropping Alpha, but when this was
discussed back in October, Stefan did:

http://www.postgresql.org/message-id/52616373.10004@kaltenbrunner.cc

But I think he's rather in the minority anyway. Also, if we added a
fallback implementation for spinlocks that uses GCC intrinsics, it
would probably work again, as much as it does now.

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

On 2014-06-27 13:12:31 -0400, Robert Haas wrote:
> I have noticed that most PostgreSQL committers seem for format their
> commit messages so that paragraphs are separated by a blank line, but
> you seem not to do that. I find that less readable.

I'll change that.

> > Since there seems to be (unanimous?) support for dropping alpha and some
> > patches coming up that need to deal with platform dependent stuff it
> > seems sensible to do this first.

> I don't personally object to dropping Alpha, but when this was
> discussed back in October, Stefan did:
>
> http://www.postgresql.org/message-id/52616373.10004@kaltenbrunner.cc

Ah, right. I still am in favor of dropping it because I don't it is
likely to work, but, as a compromise, we could remove only the Tru64
variant? Openbsd + gcc is much less of a hassle.

> But I think he's rather in the minority anyway.

Looks like it.

Greetings,

Andres Freund

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

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-06-27 13:12:31 -0400, Robert Haas wrote:
>> I don't personally object to dropping Alpha, but when this was
>> discussed back in October, Stefan did:
>>
>> http://www.postgresql.org/message-id/52616373.10004@kaltenbrunner.cc

As an ex-packager I do not believe the argument that it will matter
to packagers if we desupport one of their secondary architectures.
There are many, many packages that have never claimed to work on
oddball architectures at all. Packagers would be better served
by honesty about what we can support.

> Ah, right. I still am in favor of dropping it because I don't it is
> likely to work, but, as a compromise, we could remove only the Tru64
> variant? Openbsd + gcc is much less of a hassle.

>> But I think he's rather in the minority anyway.

> Looks like it.

There would be value in continuing to support Alpha if we had one
in the buildfarm. We don't, and have not had in any recent memory,
and I haven't noticed anyone offering to provide one in future.

The actual situation is that we're shipping a "port" that most
likely doesn't work, and we have no way to fix it. That's of
no benefit to anyone.

regards, tom lane

On 06/27/2014 08:26 PM, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> On 2014-06-27 13:12:31 -0400, Robert Haas wrote:
>>> I don't personally object to dropping Alpha, but when this was
>>> discussed back in October, Stefan did:
>>>
>>> http://www.postgresql.org/message-id/52616373.10004@kaltenbrunner.cc
>
> As an ex-packager I do not believe the argument that it will matter
> to packagers if we desupport one of their secondary architectures.
> There are many, many packages that have never claimed to work on
> oddball architectures at all. Packagers would be better served
> by honesty about what we can support.

yeah I guess so - I was mostly pointing out that alpha looked like to be
a way more "active" platform than most of what was discussed in that
thread. I personally dont think that continuing to support alpha will
buy us anything...

>
>> Ah, right. I still am in favor of dropping it because I don't it is
>> likely to work, but, as a compromise, we could remove only the Tru64
>> variant? Openbsd + gcc is much less of a hassle.
>
>>> But I think he's rather in the minority anyway.
>
>> Looks like it.
>
> There would be value in continuing to support Alpha if we had one
> in the buildfarm. We don't, and have not had in any recent memory,
> and I haven't noticed anyone offering to provide one in future.
>
> The actual situation is that we're shipping a "port" that most
> likely doesn't work, and we have no way to fix it. That's of
> no benefit to anyone.

yeah I dont have access to any alpha hardware to provide a buildfarm box
so I cant "help" there :(

Stefan

On Fri, Jun 27, 2014 at 07:51:32PM +0200, Andres Freund wrote:
> On 2014-06-27 13:12:31 -0400, Robert Haas wrote:
> > I don't personally object to dropping Alpha, but when this was
> > discussed back in October, Stefan did:
> >
> > http://www.postgresql.org/message-id/52616373.10004@kaltenbrunner.cc
>
> Ah, right. I still am in favor of dropping it because I don't it is
> likely to work, but, as a compromise, we could remove only the Tru64
> variant? Openbsd + gcc is much less of a hassle.

Retaining Alpha support means placing data dependency barriers (Linux kernel
term) where the Alpha memory model requires them. I doubt enough users will
stress Alpha builds for us to distinguish a missing barrier from hardware
flakiness, so we'd never find out whether we did it right. That's why I favor
removing Alpha-specific code completely, and it is an OS-independent and
compiler-independent motive.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com