On Tue, Aug 11, 2015 at 01:04:48PM +0200, Andres Freund wrote:
> On 2015-08-05 15:46:36 +0200, Andres Freund wrote:
> > Here's a conversion for fastgetattr() and heap_getattr().
> In my opinion this drastically increases readability and thus should be
> applied.
Atomics were a miner's canary for pademelon's trouble with post-de6fd1c
inlining. Expect pademelon to break whenever a frontend-included file gains
an inline function that calls a backend function. Atomics were the initial
examples, but this patch adds more:
$ make -s PROFILE='-O0 -DPG_FORCE_DISABLE_INLINE=1'
pg_resetxlog.o: In function `fastgetattr':
/data/nmisch/src/pg/postgresql/src/bin/pg_resetxlog/../../../src/include/access/htup_details.h:754: undefined reference
to`nocachegetattr'
pg_resetxlog.o: In function `heap_getattr':
/data/nmisch/src/pg/postgresql/src/bin/pg_resetxlog/../../../src/include/access/htup_details.h:783: undefined reference
to`heap_getsysattr'
The htup_details.h case is trickier in a way, because pg_resetxlog has a
legitimate need for SizeofHeapTupleHeader via TOAST_MAX_CHUNK_SIZE. Expect
this class of problem to recur as we add more inline functions. Our method to
handle these first two instances will set a precedent.
That gave me new respect for STATIC_IF_INLINE. While it does add tedious work
to the task of introducing a new batch of inline functions, the work is
completely mechanical. Anyone can write it; anyone can review it; there's one
correct way to write it. Header surgery like
0001-Don-t-include-low-level-locking-code-from-frontend-c.patch requires
expert design from scratch, and it (trivially) breaks builds for a sample of
non-core code. Let's return to STATIC_IF_INLINE and convert fastgetattr()
therewith. (A possible future line of inquiry is to generate the
STATIC_IF_INLINE transformation at build time, so the handwritten headers can
use C99 inlining directly as though it is always available.)
Thanks,
nm