On 2014-06-23 10:29:54 -0400, Robert Haas wrote: > On Thu, Jun 19, 2014 at 10:43 AM, Merlin Moncure <mmonc...@gmail.com> wrote: > > On Thu, Jun 19, 2014 at 7:07 AM, Abhijit Menon-Sen <a...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
