Re: Macro nesting hell

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Macro nesting hell
Date: 2015-08-12 23:43:55
Message-ID: 20150812234355.GB701@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015-08-12 10:18:12 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2015-07-01 12:55:48 -0300, Alvaro Herrera wrote:
> >> Tom Lane wrote:
> >>> I'm thinking we really ought to mount a campaign to replace some of these
> >>> macros with inlined-if-possible functions.
>
> >> My guess is that changing a very small amount of them will do a large
> >> enough portion of the job.
>
> > I think it'd be good to convert the macros in bufpage.h and bufmgr.h
> > that either currently have multiple evaluation hazards, or have a
> > AssertMacro() in them. The former for obvious reasons, the latter
> > because that immediately makes them rather large (on and it implies
> > multiple evaluation hazards anyway).
>
> > That'd mean inlining PageSetPageSizeAndVersion(), PageGetSpecialSize(),
> > PageGetSpecialPointer(), PageGetItem(), PageGetMaxOffsetNumber(),
> > PageIsPrunable(), PageSetPrunable(), PageClearPrunable(),
> > BufferIsValid(), BufferGetBlock(), BufferGetPageSize().
>
> Sounds reasonable to me. If you do this, I'll see whether pademelon
> can be adjusted to build using the minimum macro expansion buffer
> size specified by the C standard.

Here's the patch attached.

There's two more macros on the list that I had missed:
PageXLogRecPtrSet(), PageXLogRecPtrGet(). Unfortunately *Set() requires
to pas a pointer to the PageXLogRecPtr - there's only two callers tho.
We could instead just leave these, or add an indirecting macro. Seems
fine to me though.

With it applied pg compiles without the warning I saw before:
/home/andres/src/postgresql/src/backend/access/brin/brin_pageops.c:778:5: warning: string literal of length 7732 exceeds maximum length 4095
that ISO C99 compilers are required to support [-Woverlength-strings]
Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf)));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We could obviously be more aggressive and convert all the applicable
defines, but they are already readable and have no multiple eval
hazards, so I'm not inclined to that.

Andres

Attachment Content-Type Size
0001-Change-some-buffer-and-page-related-macros-to-inline.patch text/x-patch 8.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2015-08-13 00:19:01 Re: count_nulls(VARIADIC "any")
Previous Message Andres Freund 2015-08-12 23:31:56 Re: [COMMITTERS] pgsql: Close some holes in BRIN page assignment