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 |
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 |