Re: Macro nesting hell - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Macro nesting hell
Date
Msg-id 20150812081106.GA9522@awork2.anarazel.de
Whole thread Raw
In response to Re: Macro nesting hell  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: Macro nesting hell  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2015-07-01 12:55:48 -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Last night my ancient HP compiler spit up on HEAD:
> > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pademelon&dt=2015-07-01%2001%3A30%3A18
> > complaining thus:
> > cpp: "brin_pageops.c", line 626: error 4018: Macro param too large after substitution - use -H option.
> > I was able to revive pademelon by adding a new compiler flag as suggested,
> > but after looking at what the preprocessor is emitting, I can't say that
> > I blame it for being unhappy.  This simple-looking line
> > 
> >                 Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf)));
> > 
> > is expanding to this:

I just hit this in clang which also warns about too long literals unless
you silence it...

> Wow, that's kind of amazing.  I think this particular case boils down to
> just PageGetSpecialPointer (bufpage.h) and BufferGetBlock (bufmgr.h).

Inlining just BufferGetBlock already helps sufficiently to press it
below 4k (the standard's limit IIRC), but that doesn't mean we shouldn't
go a bit further.

> > 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().

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: patch : Allow toast tables to be moved to a different tablespace
Next
From: Simon Riggs
Date:
Subject: Re: optimizing vacuum truncation scans