>From 8203fd286c276051060a6fe8c98c8b576c644f14 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 13 Aug 2015 01:37:44 +0200 Subject: [PATCH] Change some buffer and page related macros to inline functions. Firstly some of these macros could recursively expand to string literals larger than the minimum required to be supported by an environment by the C standard. That's primarily due to included assertions. Secondly some macros had multiple evaluation hazards. This required one minor API change in that PageXLogRecPtrSet now takes a pointer to a PageXLogRecPtr instead the value directly. There shouldn't be callers to it out there... Discussion: 4407.1435763473@sss.pgh.pa.us --- src/include/access/gist.h | 2 +- src/include/storage/bufmgr.h | 29 +++++----- src/include/storage/bufpage.h | 124 ++++++++++++++++++++++++++---------------- 3 files changed, 95 insertions(+), 60 deletions(-) diff --git a/src/include/access/gist.h b/src/include/access/gist.h index 81e559b..33eb9d3 100644 --- a/src/include/access/gist.h +++ b/src/include/access/gist.h @@ -142,7 +142,7 @@ typedef struct GISTENTRY #define GistClearFollowRight(page) ( GistPageGetOpaque(page)->flags &= ~F_FOLLOW_RIGHT) #define GistPageGetNSN(page) ( PageXLogRecPtrGet(GistPageGetOpaque(page)->nsn)) -#define GistPageSetNSN(page, val) ( PageXLogRecPtrSet(GistPageGetOpaque(page)->nsn, val)) +#define GistPageSetNSN(page, val) ( PageXLogRecPtrSet(&GistPageGetOpaque(page)->nsn, val)) /* * Vector of GISTENTRY structs; user-defined methods union and picksplit diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index ec0a254..235264c 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -96,11 +96,12 @@ extern PGDLLIMPORT int32 *LocalRefCount; * even in non-assert-enabled builds can be significant. Thus, we've * now demoted the range checks to assertions within the macro itself. */ -#define BufferIsValid(bufnum) \ -( \ - AssertMacro((bufnum) <= NBuffers && (bufnum) >= -NLocBuffer), \ - (bufnum) != InvalidBuffer \ -) +static inline bool +BufferIsValid(Buffer bufnum) +{ + AssertArg(bufnum <= NBuffers && bufnum >= -NLocBuffer); + return bufnum != InvalidBuffer; +} /* * BufferGetBlock @@ -109,14 +110,16 @@ extern PGDLLIMPORT int32 *LocalRefCount; * Note: * Assumes buffer is valid. */ -#define BufferGetBlock(buffer) \ -( \ - AssertMacro(BufferIsValid(buffer)), \ - BufferIsLocal(buffer) ? \ - LocalBufferBlockPointers[-(buffer) - 1] \ - : \ - (Block) (BufferBlocks + ((Size) ((buffer) - 1)) * BLCKSZ) \ -) +static inline Block +BufferGetBlock(Buffer buffer) +{ + AssertArg(BufferIsValid(buffer)); + + if (BufferIsLocal(buffer)) + return LocalBufferBlockPointers[-buffer - 1]; + else + return (Block) (BufferBlocks + ((Size) (buffer - 1)) * BLCKSZ); +} /* * BufferGetPageSize diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index a2f78ee..4d104b8 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -14,6 +14,7 @@ #ifndef BUFPAGE_H #define BUFPAGE_H +#include "access/transam.h" #include "access/xlogdefs.h" #include "storage/block.h" #include "storage/item.h" @@ -93,10 +94,18 @@ typedef struct uint32 xrecoff; /* low bits */ } PageXLogRecPtr; -#define PageXLogRecPtrGet(val) \ - ((uint64) (val).xlogid << 32 | (val).xrecoff) -#define PageXLogRecPtrSet(ptr, lsn) \ - ((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn)) +static inline XLogRecPtr +PageXLogRecPtrGet(PageXLogRecPtr pageptr) +{ + return ((uint64) pageptr.xlogid) << 32 | pageptr.xrecoff; +} + +static inline void +PageXLogRecPtrSet(PageXLogRecPtr *pageptr, XLogRecPtr lsn) +{ + pageptr->xlogid = (uint32) (lsn >> 32); + pageptr->xrecoff = (uint32) lsn; +} /* * disk page organization @@ -197,7 +206,7 @@ typedef PageHeaderData *PageHeader; #define PG_DATA_CHECKSUM_VERSION 1 /* ---------------------------------------------------------------- - * page support macros + * page support macros & inline functions * ---------------------------------------------------------------- */ @@ -279,12 +288,13 @@ typedef PageHeaderData *PageHeader; * We could support setting these two values separately, but there's * no real need for it at the moment. */ -#define PageSetPageSizeAndVersion(page, size, version) \ -( \ - AssertMacro(((size) & 0xFF00) == (size)), \ - AssertMacro(((version) & 0x00FF) == (version)), \ - ((PageHeader) (page))->pd_pagesize_version = (size) | (version) \ -) +static inline void +PageSetPageSizeAndVersion(Page page, Size size, int version) +{ + Assert((size & 0xFF00) == size); + Assert((version & 0x00FF) == version); + ((PageHeader) page)->pd_pagesize_version = size | version; +} /* ---------------- * page special data macros @@ -294,20 +304,26 @@ typedef PageHeaderData *PageHeader; * PageGetSpecialSize * Returns size of special space on a page. */ -#define PageGetSpecialSize(page) \ - ((uint16) (PageGetPageSize(page) - ((PageHeader)(page))->pd_special)) +static inline uint16 +PageGetSpecialSize(Page page) +{ + return (uint16) (PageGetPageSize(page) - ((PageHeader) page)->pd_special); +} /* * PageGetSpecialPointer * Returns pointer to special space on a page. */ -#define PageGetSpecialPointer(page) \ -( \ - AssertMacro(PageIsValid(page)), \ - AssertMacro(((PageHeader) (page))->pd_special <= BLCKSZ), \ - AssertMacro(((PageHeader) (page))->pd_special >= SizeOfPageHeaderData), \ - (char *) ((char *) (page) + ((PageHeader) (page))->pd_special) \ -) +static inline char * +PageGetSpecialPointer(Page page) +{ + PageHeader ph = (PageHeader) page; + + Assert(PageIsValid(page)); + Assert(ph->pd_special <= BLCKSZ); + Assert(ph->pd_special >= SizeOfPageHeaderData); + return ((char *) page) + ph->pd_special; +} /* * PageGetItem @@ -317,12 +333,13 @@ typedef PageHeaderData *PageHeader; * This does not change the status of any of the resources passed. * The semantics may change in the future. */ -#define PageGetItem(page, itemId) \ -( \ - AssertMacro(PageIsValid(page)), \ - AssertMacro(ItemIdHasStorage(itemId)), \ - (Item)(((char *)(page)) + ItemIdGetOffset(itemId)) \ -) +static inline Item +PageGetItem(Page page, ItemId itemId) +{ + Assert(PageIsValid(page)); + Assert(ItemIdHasStorage(itemId)); + return (Item)(((char *) page) + ItemIdGetOffset(itemId)); +} /* * PageGetMaxOffsetNumber @@ -334,19 +351,24 @@ typedef PageHeaderData *PageHeader; * return zero to ensure sane behavior. Accept double evaluation * of the argument so that we can ensure this. */ -#define PageGetMaxOffsetNumber(page) \ - (((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData ? 0 : \ - ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \ - / sizeof(ItemIdData))) +static inline LocationIndex +PageGetMaxOffsetNumber(Page page) +{ + PageHeader ph = (PageHeader) page; + + if (ph->pd_lower <= SizeOfPageHeaderData) + return 0; + else + return (ph->pd_lower - SizeOfPageHeaderData) / sizeof(ItemIdData); +} /* - * Additional macros for access to page headers. (Beware multiple evaluation - * of the arguments!) + * Additional page header accessors */ #define PageGetLSN(page) \ PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn) #define PageSetLSN(page, lsn) \ - PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn) + PageXLogRecPtrSet(&((PageHeader) (page))->pd_lsn, lsn) #define PageHasFreeLinePointers(page) \ (((PageHeader) (page))->pd_flags & PD_HAS_FREE_LINES) @@ -369,19 +391,29 @@ typedef PageHeaderData *PageHeader; #define PageClearAllVisible(page) \ (((PageHeader) (page))->pd_flags &= ~PD_ALL_VISIBLE) -#define PageIsPrunable(page, oldestxmin) \ -( \ - AssertMacro(TransactionIdIsNormal(oldestxmin)), \ - TransactionIdIsValid(((PageHeader) (page))->pd_prune_xid) && \ - TransactionIdPrecedes(((PageHeader) (page))->pd_prune_xid, oldestxmin) \ -) -#define PageSetPrunable(page, xid) \ -do { \ - Assert(TransactionIdIsNormal(xid)); \ - if (!TransactionIdIsValid(((PageHeader) (page))->pd_prune_xid) || \ - TransactionIdPrecedes(xid, ((PageHeader) (page))->pd_prune_xid)) \ - ((PageHeader) (page))->pd_prune_xid = (xid); \ -} while (0) +static inline bool +PageIsPrunable(Page page, TransactionId oldestxmin) +{ + TransactionId prune_xid = ((PageHeader) page)->pd_prune_xid; + + Assert(TransactionIdIsNormal(oldestxmin)); + + return TransactionIdIsValid(prune_xid) && + TransactionIdPrecedes(prune_xid, oldestxmin); +} + +static inline void +PageSetPrunable(Page page, TransactionId xid) +{ + PageHeader ph = (PageHeader) page; + + Assert(TransactionIdIsNormal(xid)); + + if (!TransactionIdIsValid(ph->pd_prune_xid) || + TransactionIdPrecedes(xid, ph->pd_prune_xid)) + ph->pd_prune_xid = xid; +} + #define PageClearPrunable(page) \ (((PageHeader) (page))->pd_prune_xid = InvalidTransactionId) -- 2.3.0.149.gf3f4077.dirty