On August 15, 2015 6:47:09 PM GMT+02:00, Noah Misch <noah@leadboat.com> wrote:
>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.)
I'll respond in detail later. But wouldn't it be easy in this case to just ifndef FRONTEND things in this case?
---
Please excuse brevity and formatting - I am writing this on my mobile phone.