Re: Raising our compiler requirements for 9.6

From: Andres Freund <andres(at)anarazel(dot)de>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Raising our compiler requirements for 9.6
Date: 2015-08-17 10:06:42
Message-ID: 20150817100642.GD3522@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015-08-16 03:31:48 -0400, Noah Misch wrote:
> As we see from the patches' need to add #include statements to .c files and
> from Robert's report of a broken EDB build, this change creates work for
> maintainers of non-core code, both backend code (modules) and frontend code
> (undocumented, but folks do it). That's to be expected and doesn't take a
> great deal of justification, but users should get benefits in connection with
> the work. This brings to mind the htup_details.h introduction, which created
> so much busy work in non-core code. I don't expect the lock.h split to be
> quite that disruptive. Statistics on PGXN modules:

> Four modules (imcs, pg_stat_kcache, query_histogram, query_recorder) mentioned
> LWLock without mentioning lwlock.h. These patches (trivially) break the imcs
> build. The other three failed to build for unrelated reasons, but I suspect
> these patches give those modules one more thing to update. What do users get
> out of this? They'll learn if their code is not portable to pademelon, but
> #warning "atomics.h in frontend code is not portable" would communicate the
> same without compelling non-core code to care.

I'd love to make it a #warning intead of an error, but unfortunately
that's not standard C :(

> Other than that benefit, making headers #error-on-FRONTEND mostly lets
> us congratulate ourselves for having introduced the start of a header
> layer distinction. I'd be for that if PostgreSQL were new, but I
> can't justify doing it at the user cost already apparent. That would
> be bad business.

To me that's basically saying that we'll never ever have any better
separation between frontend/backend headers since each incremental
improvement won't be justifiable. Adding an explicit include which
exists in all version of postgres is really rather low cost, you don't
even need version dependant define. The modules you name are, as you
noticed, likely to require minor surgery for new major versions anyway,
being rather low level.

As you say breaking C code over major releases doesn't have to meet a
high barrier, and getting rid of the wart of lock.h being used that
widely seems to be more than suffient to meet that qualification.

If others think this is the right way, ok, I can live with that and
implement it, but I won't agree.

> Therefore, I urge you to instead add the aforementioned #warning and wrap with
> #ifdef FRONTEND the two #include "port/atomics.h" in headers. That tightly
> limits build breakage; it can only break frontend code, which is rare outside
> core. Hackers will surely notice if a patch makes the warning bleat, so
> mistakes won't survive long.

> Non-core frontend code, if willing to cede a shred of portability, can
> experiment with atomics. Folks could do nifty things with that.

I don't think that's something worth keeping in mind from our side. If
you don't care about portability you can just use C11 atomics or
such.

If somebody actually wants to add FRONTEND support for the fallback code
- possibly falling back to pthread mutexes - ok, but before that...

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-08-17 10:59:12 Re: checkpointer continuous flushing
Previous Message Fabien COELHO 2015-08-17 08:12:13 Re: pgbench stats per script & other stuff