On 16.05.22 15:23, Amul Sul wrote:
+static inline OffsetNumber +PageGetMaxOffsetNumber(Page page) +{ + if (((PageHeader) page)->pd_lower <= SizeOfPageHeaderData) + return 0; + else + return ((((PageHeader) page)->pd_lower - SizeOfPageHeaderData) / sizeof(ItemIdData)); +}The "else" is not necessary, we can have the return statement directly which would save some indentation as well. The Similar pattern can be considered for 0004 and 0007 patches as well.
I kind of like it better this way. It preserves the functional style of the original macro.
+static inline void +XLogFromFileName(const char *fname, TimeLineID *tli, XLogSegNo *logSegNo, int wal_segsz_bytes) +{ + uint32 log; + uint32 seg; + sscanf(fname, "%08X%08X%08X", tli, &log, &seg); + *logSegNo = (uint64) log * XLogSegmentsPerXLogId(wal_segsz_bytes) + seg; +} Can we have a blank line after variable declarations that we usually have?
done
0006 patch: +static inline Datum +fetch_att(const void *T, bool attbyval, int attlen) +{ + if (attbyval) + { +#if SIZEOF_DATUM == 8 + if (attlen == sizeof(Datum)) + return *((const Datum *) T); + else +#endif Can we have a switch case like store_att_byval() instead of if-else, code would look more symmetric, IMO.
done
From 75cc12a425a58340799376bc1e2dfdebfcb7ea0a Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (block.h) Remove BlockIdIsValid(), which wasn't used and is unnecessary. Remove BlockIdCopy(), which wasn't used and can be done by struct assignment. (BlockIdEquals() also isn't used, but seems reasonable to keep around.) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/storage/block.h | 53 ++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/src/include/storage/block.h b/src/include/storage/block.h index d756e3fda5..c726142f96 100644 --- a/src/include/storage/block.h +++ b/src/include/storage/block.h @@ -59,7 +59,7 @@ typedef struct BlockIdData typedef BlockIdData *BlockId; /* block identifier */ /* ---------------- - * support macros + * support functions * ---------------- */ @@ -67,49 +67,42 @@ typedef BlockIdData *BlockId; /* block identifier */ * BlockNumberIsValid * True iff blockNumber is valid. */ -#define BlockNumberIsValid(blockNumber) \ - ((BlockNumber) (blockNumber) != InvalidBlockNumber) - -/* - * BlockIdIsValid - * True iff the block identifier is valid. - */ -#define BlockIdIsValid(blockId) \ - PointerIsValid(blockId) +static inline bool +BlockNumberIsValid(BlockNumber blockNumber) +{ + return blockNumber != InvalidBlockNumber; +} /* * BlockIdSet * Sets a block identifier to the specified value. */ -#define BlockIdSet(blockId, blockNumber) \ -( \ - (blockId)->bi_hi = (blockNumber) >> 16, \ - (blockId)->bi_lo = (blockNumber) & 0xffff \ -) - -/* - * BlockIdCopy - * Copy a block identifier. - */ -#define BlockIdCopy(toBlockId, fromBlockId) \ -( \ - (toBlockId)->bi_hi = (fromBlockId)->bi_hi, \ - (toBlockId)->bi_lo = (fromBlockId)->bi_lo \ -) +static inline void +BlockIdSet(BlockIdData *blockId, BlockNumber blockNumber) +{ + blockId->bi_hi = blockNumber >> 16; + blockId->bi_lo = blockNumber & 0xffff; +} /* * BlockIdEquals * Check for block number equality. */ -#define BlockIdEquals(blockId1, blockId2) \ - ((blockId1)->bi_hi == (blockId2)->bi_hi && \ - (blockId1)->bi_lo == (blockId2)->bi_lo) +static inline bool +BlockIdEquals(const BlockIdData *blockId1, const BlockIdData *blockId2) +{ + return (blockId1->bi_hi == blockId2->bi_hi && + blockId1->bi_lo == blockId2->bi_lo); +} /* * BlockIdGetBlockNumber * Retrieve the block number from a block identifier. */ -#define BlockIdGetBlockNumber(blockId) \ - ((((BlockNumber) (blockId)->bi_hi) << 16) | ((BlockNumber) (blockId)->bi_lo)) +static inline BlockNumber +BlockIdGetBlockNumber(const BlockIdData *blockId) +{ + return (((BlockNumber) (blockId)->bi_hi) << 16) | ((BlockNumber) (blockId)->bi_lo); +} #endif /* BLOCK_H */ -- 2.36.1
From 103f6b446cc2efa0b6a21bf181171cbad176d968 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (bufpage.h) rawpage.c needs some adjustments because it was previously playing loose with the Page vs. PageHeader types, which is no longer possible with the functions instead of macros. Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- contrib/pageinspect/rawpage.c | 24 ++++--- src/include/storage/bufpage.h | 131 ++++++++++++++++++++-------------- 2 files changed, 92 insertions(+), 63 deletions(-) diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index 730a46b1d8..90942be71e 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -254,7 +254,8 @@ page_header(PG_FUNCTION_ARGS) Datum values[9]; bool nulls[9]; - PageHeader page; + Page page; + PageHeader pageheader; XLogRecPtr lsn; if (!superuser()) @@ -262,7 +263,8 @@ page_header(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to use raw page functions"))); - page = (PageHeader) get_page_from_raw(raw_page); + page = get_page_from_raw(raw_page); + pageheader = (PageHeader) page; /* Build a tuple descriptor for our result type */ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) @@ -282,8 +284,8 @@ page_header(PG_FUNCTION_ARGS) } else values[0] = LSNGetDatum(lsn); - values[1] = UInt16GetDatum(page->pd_checksum); - values[2] = UInt16GetDatum(page->pd_flags); + values[1] = UInt16GetDatum(pageheader->pd_checksum); + values[2] = UInt16GetDatum(pageheader->pd_flags); /* pageinspect >= 1.10 uses int4 instead of int2 for those fields */ switch (TupleDescAttr(tupdesc, 3)->atttypid) @@ -292,18 +294,18 @@ page_header(PG_FUNCTION_ARGS) Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT2OID && TupleDescAttr(tupdesc, 5)->atttypid == INT2OID && TupleDescAttr(tupdesc, 6)->atttypid == INT2OID); - values[3] = UInt16GetDatum(page->pd_lower); - values[4] = UInt16GetDatum(page->pd_upper); - values[5] = UInt16GetDatum(page->pd_special); + values[3] = UInt16GetDatum(pageheader->pd_lower); + values[4] = UInt16GetDatum(pageheader->pd_upper); + values[5] = UInt16GetDatum(pageheader->pd_special); values[6] = UInt16GetDatum(PageGetPageSize(page)); break; case INT4OID: Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT4OID && TupleDescAttr(tupdesc, 5)->atttypid == INT4OID && TupleDescAttr(tupdesc, 6)->atttypid == INT4OID); - values[3] = Int32GetDatum(page->pd_lower); - values[4] = Int32GetDatum(page->pd_upper); - values[5] = Int32GetDatum(page->pd_special); + values[3] = Int32GetDatum(pageheader->pd_lower); + values[4] = Int32GetDatum(pageheader->pd_upper); + values[5] = Int32GetDatum(pageheader->pd_special); values[6] = Int32GetDatum(PageGetPageSize(page)); break; default: @@ -312,7 +314,7 @@ page_header(PG_FUNCTION_ARGS) } values[7] = UInt16GetDatum(PageGetPageLayoutVersion(page)); - values[8] = TransactionIdGetDatum(page->pd_prune_xid); + values[8] = TransactionIdGetDatum(pageheader->pd_prune_xid); /* Build and return the tuple. */ diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index e9f253f2c8..23a93ebda0 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" @@ -97,8 +98,12 @@ typedef struct uint32 xrecoff; /* low bits */ } PageXLogRecPtr; -#define PageXLogRecPtrGet(val) \ - ((uint64) (val).xlogid << 32 | (val).xrecoff) + +static inline XLogRecPtr +PageXLogRecPtrGet(PageXLogRecPtr val) +{ + return (uint64) val.xlogid << 32 | val.xrecoff; +} #define PageXLogRecPtrSet(ptr, lsn) \ ((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn)) @@ -200,7 +205,7 @@ typedef PageHeaderData *PageHeader; #define PG_DATA_CHECKSUM_VERSION 1 /* ---------------------------------------------------------------- - * page support macros + * page support macros and functions * ---------------------------------------------------------------- */ @@ -247,7 +252,7 @@ typedef PageHeaderData *PageHeader; ((char *) (page) + MAXALIGN(SizeOfPageHeaderData)) /* ---------------- - * macros to access page size info + * macros and functions to access page size info * ---------------- */ @@ -265,15 +270,21 @@ typedef PageHeaderData *PageHeader; * BufferGetPageSize, which can be called on an unformatted page). * however, it can be called on a page that is not stored in a buffer. */ -#define PageGetPageSize(page) \ - ((Size) (((PageHeader) (page))->pd_pagesize_version & (uint16) 0xFF00)) +static inline Size +PageGetPageSize(Page page) +{ + return (Size) (((PageHeader) page)->pd_pagesize_version & (uint16) 0xFF00); +} /* * PageGetPageLayoutVersion * Returns the page layout version of a page. */ -#define PageGetPageLayoutVersion(page) \ - (((PageHeader) (page))->pd_pagesize_version & 0x00FF) +static inline uint8 +PageGetPageLayoutVersion(Page page) +{ + return (((PageHeader) page)->pd_pagesize_version & 0x00FF); +} /* * PageSetPageSizeAndVersion @@ -282,52 +293,52 @@ 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, uint8 version) +{ + Assert((size & 0xFF00) == size); + Assert((version & 0x00FF) == version); + + ((PageHeader) page)->pd_pagesize_version = size | version; +} /* ---------------- - * page special data macros + * page special data macros and functions * ---------------- */ /* * 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)); +} /* * Using assertions, validate that the page special pointer is OK. * * This is intended to catch use of the pointer before page initialization. - * It is implemented as a function due to the limitations of the MSVC - * compiler, which choked on doing all these tests within another macro. We - * return true so that AssertMacro() can be used while still getting the - * specifics from the macro failure within this function. */ -static inline bool +static inline void PageValidateSpecialPointer(Page page) { Assert(PageIsValid(page)); Assert(((PageHeader) (page))->pd_special <= BLCKSZ); Assert(((PageHeader) (page))->pd_special >= SizeOfPageHeaderData); - - return true; } /* * PageGetSpecialPointer * Returns pointer to special space on a page. */ -#define PageGetSpecialPointer(page) \ -( \ - AssertMacro(PageValidateSpecialPointer(page)), \ - (char *) ((char *) (page) + ((PageHeader) (page))->pd_special) \ -) +static inline char * +PageGetSpecialPointer(Page page) +{ + PageValidateSpecialPointer(page); + return (char *) page + ((PageHeader) page)->pd_special; +} /* * PageGetItem @@ -337,12 +348,14 @@ PageValidateSpecialPointer(Page page) * 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 @@ -351,17 +364,21 @@ PageValidateSpecialPointer(Page page) * of items on the page. * * NOTE: if the page is not initialized (pd_lower == 0), we must - * return zero to ensure sane behavior. Accept double evaluation - * of the argument so that we can ensure this. + * return zero to ensure sane behavior. */ -#define PageGetMaxOffsetNumber(page) \ - (((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData ? 0 : \ - ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \ - / sizeof(ItemIdData))) +static inline OffsetNumber +PageGetMaxOffsetNumber(Page page) +{ + PageHeader pageheader = (PageHeader) page; + + if (pageheader->pd_lower <= SizeOfPageHeaderData) + return 0; + else + return (pageheader->pd_lower - SizeOfPageHeaderData) / sizeof(ItemIdData); +} /* - * Additional macros for access to page headers. (Beware multiple evaluation - * of the arguments!) + * Additional macros and functions for access to page headers. */ #define PageGetLSN(page) \ PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn) @@ -389,15 +406,25 @@ PageValidateSpecialPointer(Page page) #define PageClearAllVisible(page) \ (((PageHeader) (page))->pd_flags &= ~PD_ALL_VISIBLE) -#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) -#define PageClearPrunable(page) \ - (((PageHeader) (page))->pd_prune_xid = InvalidTransactionId) +static inline void +PageSetPrunable(Page page, TransactionId xid) +{ + PageHeader pageheader = (PageHeader) page; + + Assert(TransactionIdIsNormal(xid)); + + if (!TransactionIdIsValid(pageheader->pd_prune_xid) || + TransactionIdPrecedes(xid, pageheader->pd_prune_xid)) + pageheader->pd_prune_xid = xid; +} + +static inline void +PageClearPrunable(Page page) +{ + PageHeader pageheader = (PageHeader) page; + + pageheader->pd_prune_xid = InvalidTransactionId; +} /* ---------------------------------------------------------------- -- 2.36.1
From 667f5e233c5a563f62bfd5b6a94871599ab7a8c3 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (itemptr.h) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/storage/itemptr.h | 129 +++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 56 deletions(-) diff --git a/src/include/storage/itemptr.h b/src/include/storage/itemptr.h index 81947bc657..3ec88a68df 100644 --- a/src/include/storage/itemptr.h +++ b/src/include/storage/itemptr.h @@ -71,7 +71,7 @@ typedef ItemPointerData *ItemPointer; /* ---------------- - * support macros + * support functions * ---------------- */ @@ -79,77 +79,87 @@ typedef ItemPointerData *ItemPointer; * ItemPointerIsValid * True iff the disk item pointer is not NULL. */ -#define ItemPointerIsValid(pointer) \ - ((bool) (PointerIsValid(pointer) && ((pointer)->ip_posid != 0))) +static inline bool +ItemPointerIsValid(const ItemPointerData *pointer) +{ + return PointerIsValid(pointer) && pointer->ip_posid != 0; +} /* * ItemPointerGetBlockNumberNoCheck * Returns the block number of a disk item pointer. */ -#define ItemPointerGetBlockNumberNoCheck(pointer) \ -( \ - BlockIdGetBlockNumber(&(pointer)->ip_blkid) \ -) +static inline BlockNumber +ItemPointerGetBlockNumberNoCheck(const ItemPointerData *pointer) +{ + return BlockIdGetBlockNumber(&pointer->ip_blkid); +} /* * ItemPointerGetBlockNumber * As above, but verifies that the item pointer looks valid. */ -#define ItemPointerGetBlockNumber(pointer) \ -( \ - AssertMacro(ItemPointerIsValid(pointer)), \ - ItemPointerGetBlockNumberNoCheck(pointer) \ -) +static inline BlockNumber +ItemPointerGetBlockNumber(const ItemPointerData *pointer) +{ + Assert(ItemPointerIsValid(pointer)); + return ItemPointerGetBlockNumberNoCheck(pointer); +} /* * ItemPointerGetOffsetNumberNoCheck * Returns the offset number of a disk item pointer. */ -#define ItemPointerGetOffsetNumberNoCheck(pointer) \ -( \ - (pointer)->ip_posid \ -) +static inline OffsetNumber +ItemPointerGetOffsetNumberNoCheck(const ItemPointerData *pointer) +{ + return pointer->ip_posid; +} /* * ItemPointerGetOffsetNumber * As above, but verifies that the item pointer looks valid. */ -#define ItemPointerGetOffsetNumber(pointer) \ -( \ - AssertMacro(ItemPointerIsValid(pointer)), \ - ItemPointerGetOffsetNumberNoCheck(pointer) \ -) +static inline OffsetNumber +ItemPointerGetOffsetNumber(const ItemPointerData *pointer) +{ + Assert(ItemPointerIsValid(pointer)); + return ItemPointerGetOffsetNumberNoCheck(pointer); +} /* * ItemPointerSet * Sets a disk item pointer to the specified block and offset. */ -#define ItemPointerSet(pointer, blockNumber, offNum) \ -( \ - AssertMacro(PointerIsValid(pointer)), \ - BlockIdSet(&((pointer)->ip_blkid), blockNumber), \ - (pointer)->ip_posid = offNum \ -) +static inline void +ItemPointerSet(ItemPointerData *pointer, BlockNumber blockNumber, OffsetNumber offNum) +{ + Assert(PointerIsValid(pointer)); + BlockIdSet(&pointer->ip_blkid, blockNumber); + pointer->ip_posid = offNum; +} /* * ItemPointerSetBlockNumber * Sets a disk item pointer to the specified block. */ -#define ItemPointerSetBlockNumber(pointer, blockNumber) \ -( \ - AssertMacro(PointerIsValid(pointer)), \ - BlockIdSet(&((pointer)->ip_blkid), blockNumber) \ -) +static inline void +ItemPointerSetBlockNumber(ItemPointerData *pointer, BlockNumber blockNumber) +{ + Assert(PointerIsValid(pointer)); + BlockIdSet(&pointer->ip_blkid, blockNumber); +} /* * ItemPointerSetOffsetNumber * Sets a disk item pointer to the specified offset. */ -#define ItemPointerSetOffsetNumber(pointer, offsetNumber) \ -( \ - AssertMacro(PointerIsValid(pointer)), \ - (pointer)->ip_posid = (offsetNumber) \ -) +static inline void +ItemPointerSetOffsetNumber(ItemPointerData *pointer, OffsetNumber offsetNumber) +{ + Assert(PointerIsValid(pointer)); + pointer->ip_posid = offsetNumber; +} /* * ItemPointerCopy @@ -158,42 +168,49 @@ typedef ItemPointerData *ItemPointer; * Should there ever be padding in an ItemPointer this would need to be handled * differently as it's used as hash key. */ -#define ItemPointerCopy(fromPointer, toPointer) \ -( \ - AssertMacro(PointerIsValid(toPointer)), \ - AssertMacro(PointerIsValid(fromPointer)), \ - *(toPointer) = *(fromPointer) \ -) +static inline void +ItemPointerCopy(const ItemPointerData *fromPointer, ItemPointerData *toPointer) +{ + Assert(PointerIsValid(toPointer)); + Assert(PointerIsValid(fromPointer)); + *toPointer = *fromPointer; +} /* * ItemPointerSetInvalid * Sets a disk item pointer to be invalid. */ -#define ItemPointerSetInvalid(pointer) \ -( \ - AssertMacro(PointerIsValid(pointer)), \ - BlockIdSet(&((pointer)->ip_blkid), InvalidBlockNumber), \ - (pointer)->ip_posid = InvalidOffsetNumber \ -) +static inline void +ItemPointerSetInvalid(ItemPointerData *pointer) +{ + Assert(PointerIsValid(pointer)); + BlockIdSet(&pointer->ip_blkid, InvalidBlockNumber); + pointer->ip_posid = InvalidOffsetNumber; +} /* * ItemPointerIndicatesMovedPartitions * True iff the block number indicates the tuple has moved to another * partition. */ -#define ItemPointerIndicatesMovedPartitions(pointer) \ -( \ - ItemPointerGetOffsetNumber(pointer) == MovedPartitionsOffsetNumber && \ - ItemPointerGetBlockNumberNoCheck(pointer) == MovedPartitionsBlockNumber \ -) +static inline bool +ItemPointerIndicatesMovedPartitions(const ItemPointerData *pointer) +{ + return + ItemPointerGetOffsetNumber(pointer) == MovedPartitionsOffsetNumber && + ItemPointerGetBlockNumberNoCheck(pointer) == MovedPartitionsBlockNumber; +} /* * ItemPointerSetMovedPartitions * Indicate that the item referenced by the itempointer has moved into a * different partition. */ -#define ItemPointerSetMovedPartitions(pointer) \ - ItemPointerSet((pointer), MovedPartitionsBlockNumber, MovedPartitionsOffsetNumber) +static inline void +ItemPointerSetMovedPartitions(ItemPointerData *pointer) +{ + ItemPointerSet(pointer, MovedPartitionsBlockNumber, MovedPartitionsOffsetNumber); +} /* ---------------- * externs -- 2.36.1
From 9dd948206d3f8bd3c1b19c00af7e73c8070dae52 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (bufmgr.h) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/storage/bufmgr.h | 50 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 58391406f6..ce725afa5e 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -98,7 +98,7 @@ extern PGDLLIMPORT int32 *LocalRefCount; #define BUFFER_LOCK_EXCLUSIVE 2 /* - * These routines are beaten on quite heavily, hence the macroization. + * These routines are beaten on quite heavily, hence inline. */ /* @@ -120,11 +120,14 @@ 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) +{ + Assert(bufnum <= NBuffers); + Assert(bufnum >= -NLocBuffer); + + return bufnum != InvalidBuffer; +} /* * BufferGetBlock @@ -133,14 +136,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) +{ + Assert(BufferIsValid(buffer)); + + if (BufferIsLocal(buffer)) + return LocalBufferBlockPointers[-buffer - 1]; + else + return (Block) (BufferBlocks + ((Size) (buffer - 1)) * BLCKSZ); +} /* * BufferGetPageSize @@ -153,11 +158,12 @@ extern PGDLLIMPORT int32 *LocalRefCount; * (formatted) disk page. */ /* XXX should dig out of buffer descriptor */ -#define BufferGetPageSize(buffer) \ -( \ - AssertMacro(BufferIsValid(buffer)), \ - (Size)BLCKSZ \ -) +static inline Size +BufferGetPageSize(Buffer buffer) +{ + AssertMacro(BufferIsValid(buffer)); + return (Size) BLCKSZ; +} /* * BufferGetPage @@ -166,7 +172,11 @@ extern PGDLLIMPORT int32 *LocalRefCount; * When this is called as part of a scan, there may be a need for a nearby * call to TestForOldSnapshot(). See the definition of that for details. */ -#define BufferGetPage(buffer) ((Page)BufferGetBlock(buffer)) +static inline Page +BufferGetPage(Buffer buffer) +{ + return (Page) BufferGetBlock(buffer); +} /* * prototypes for functions in bufmgr.c -- 2.36.1
From 02caccee2c51237157bb0ced29e714eb422fb489 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (xlog_internal.h) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/access/xlog_internal.h | 155 ++++++++++++++++++----------- 1 file changed, 97 insertions(+), 58 deletions(-) diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index fae0bef8f5..51c91cb279 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -159,74 +159,113 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader; #define XLOG_FNAME_LEN 24 /* - * Generate a WAL segment file name. Do not use this macro in a helper + * Generate a WAL segment file name. Do not use this function in a helper * function allocating the result generated. */ -#define XLogFileName(fname, tli, logSegNo, wal_segsz_bytes) \ - snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli, \ - (uint32) ((logSegNo) / XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) ((logSegNo) % XLogSegmentsPerXLogId(wal_segsz_bytes))) +static inline void +XLogFileName(char *fname, TimeLineID tli, XLogSegNo logSegNo, int wal_segsz_bytes) +{ + snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli, + (uint32) (logSegNo / XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (logSegNo % XLogSegmentsPerXLogId(wal_segsz_bytes))); +} -#define XLogFileNameById(fname, tli, log, seg) \ - snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli, log, seg) +// FIXME: This is only used by pg_archivecleanup and could be gotten rid of. +static inline void +XLogFileNameById(char *fname, TimeLineID tli, uint32 log, uint32 seg) +{ + snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli, log, seg); +} -#define IsXLogFileName(fname) \ - (strlen(fname) == XLOG_FNAME_LEN && \ - strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN) +static inline bool +IsXLogFileName(const char *fname) +{ + return (strlen(fname) == XLOG_FNAME_LEN && \ + strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN); +} /* * XLOG segment with .partial suffix. Used by pg_receivewal and at end of * archive recovery, when we want to archive a WAL segment but it might not * be complete yet. */ -#define IsPartialXLogFileName(fname) \ - (strlen(fname) == XLOG_FNAME_LEN + strlen(".partial") && \ - strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN && \ - strcmp((fname) + XLOG_FNAME_LEN, ".partial") == 0) - -#define XLogFromFileName(fname, tli, logSegNo, wal_segsz_bytes) \ - do { \ - uint32 log; \ - uint32 seg; \ - sscanf(fname, "%08X%08X%08X", tli, &log, &seg); \ - *logSegNo = (uint64) log * XLogSegmentsPerXLogId(wal_segsz_bytes) + seg; \ - } while (0) - -#define XLogFilePath(path, tli, logSegNo, wal_segsz_bytes) \ - snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X", tli, \ - (uint32) ((logSegNo) / XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) ((logSegNo) % XLogSegmentsPerXLogId(wal_segsz_bytes))) - -#define TLHistoryFileName(fname, tli) \ - snprintf(fname, MAXFNAMELEN, "%08X.history", tli) - -#define IsTLHistoryFileName(fname) \ - (strlen(fname) == 8 + strlen(".history") && \ - strspn(fname, "0123456789ABCDEF") == 8 && \ - strcmp((fname) + 8, ".history") == 0) - -#define TLHistoryFilePath(path, tli) \ - snprintf(path, MAXPGPATH, XLOGDIR "/%08X.history", tli) - -#define StatusFilePath(path, xlog, suffix) \ - snprintf(path, MAXPGPATH, XLOGDIR "/archive_status/%s%s", xlog, suffix) - -#define BackupHistoryFileName(fname, tli, logSegNo, startpoint, wal_segsz_bytes) \ - snprintf(fname, MAXFNAMELEN, "%08X%08X%08X.%08X.backup", tli, \ - (uint32) ((logSegNo) / XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) ((logSegNo) % XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) (XLogSegmentOffset(startpoint, wal_segsz_bytes))) - -#define IsBackupHistoryFileName(fname) \ - (strlen(fname) > XLOG_FNAME_LEN && \ - strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN && \ - strcmp((fname) + strlen(fname) - strlen(".backup"), ".backup") == 0) - -#define BackupHistoryFilePath(path, tli, logSegNo, startpoint, wal_segsz_bytes) \ - snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.backup", tli, \ - (uint32) ((logSegNo) / XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) ((logSegNo) % XLogSegmentsPerXLogId(wal_segsz_bytes)), \ - (uint32) (XLogSegmentOffset((startpoint), wal_segsz_bytes))) +static inline bool +IsPartialXLogFileName(const char *fname) +{ + return (strlen(fname) == XLOG_FNAME_LEN + strlen(".partial") && + strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN && + strcmp((fname) + XLOG_FNAME_LEN, ".partial") == 0); +} + +static inline void +XLogFromFileName(const char *fname, TimeLineID *tli, XLogSegNo *logSegNo, int wal_segsz_bytes) +{ + uint32 log; + uint32 seg; + + sscanf(fname, "%08X%08X%08X", tli, &log, &seg); + *logSegNo = (uint64) log * XLogSegmentsPerXLogId(wal_segsz_bytes) + seg; +} + +static inline void +XLogFilePath(char *path, TimeLineID tli, XLogSegNo logSegNo, int wal_segsz_bytes) +{ + snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X", tli, + (uint32) (logSegNo / XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (logSegNo % XLogSegmentsPerXLogId(wal_segsz_bytes))); +} + +static inline void +TLHistoryFileName(char *fname, TimeLineID tli) +{ + snprintf(fname, MAXFNAMELEN, "%08X.history", tli); +} + +static inline bool +IsTLHistoryFileName(const char *fname) +{ + return (strlen(fname) == 8 + strlen(".history") && + strspn(fname, "0123456789ABCDEF") == 8 && + strcmp((fname) + 8, ".history") == 0); +} + +static inline void +TLHistoryFilePath(char *path, TimeLineID tli) +{ + snprintf(path, MAXPGPATH, XLOGDIR "/%08X.history", tli); +} + +static inline void +StatusFilePath(char *path, const char *xlog, const char *suffix) +{ + snprintf(path, MAXPGPATH, XLOGDIR "/archive_status/%s%s", xlog, suffix); +} + +static inline void +BackupHistoryFileName(char *fname, TimeLineID tli, XLogSegNo logSegNo, XLogRecPtr startpoint, int wal_segsz_bytes) +{ + snprintf(fname, MAXFNAMELEN, "%08X%08X%08X.%08X.backup", tli, + (uint32) (logSegNo / XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (logSegNo % XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (XLogSegmentOffset(startpoint, wal_segsz_bytes))); +} + +static inline bool +IsBackupHistoryFileName(const char *fname) +{ + return (strlen(fname) > XLOG_FNAME_LEN && + strspn(fname, "0123456789ABCDEF") == XLOG_FNAME_LEN && + strcmp((fname) + strlen(fname) - strlen(".backup"), ".backup") == 0); +} + +static inline void +BackupHistoryFilePath(char *path, TimeLineID tli, XLogSegNo logSegNo, XLogRecPtr startpoint, int wal_segsz_bytes) +{ + snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.backup", tli, + (uint32) (logSegNo / XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (logSegNo % XLogSegmentsPerXLogId(wal_segsz_bytes)), + (uint32) (XLogSegmentOffset((startpoint), wal_segsz_bytes))); +} /* * Information logged when we detect a change in one of the parameters -- 2.36.1
From 41cfab80cb71453ee48bebe914ce3814d8696ec1 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (tupmacs.h) XXX The renaming in gistutil.c is unnecessary if the subsequent itup.h patch is also included. Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/backend/access/gist/gistutil.c | 16 +-- src/include/access/itup.h | 2 +- src/include/access/tupmacs.h | 161 +++++++++++------------------ 3 files changed, 70 insertions(+), 109 deletions(-) diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index d4bf0c7563..24b2c493e7 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -667,7 +667,7 @@ HeapTuple gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple) { MemoryContext oldcxt = MemoryContextSwitchTo(giststate->tempCxt); - Datum fetchatt[INDEX_MAX_KEYS]; + Datum att[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; int i; @@ -680,9 +680,9 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple) if (giststate->fetchFn[i].fn_oid != InvalidOid) { if (!isnull[i]) - fetchatt[i] = gistFetchAtt(giststate, i, datum, r); + att[i] = gistFetchAtt(giststate, i, datum, r); else - fetchatt[i] = (Datum) 0; + att[i] = (Datum) 0; } else if (giststate->compressFn[i].fn_oid == InvalidOid) { @@ -691,9 +691,9 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple) * original value, att is necessarily stored in original form. */ if (!isnull[i]) - fetchatt[i] = datum; + att[i] = datum; else - fetchatt[i] = (Datum) 0; + att[i] = (Datum) 0; } else { @@ -703,7 +703,7 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple) * in this column, and we can replace it with a NULL. */ isnull[i] = true; - fetchatt[i] = (Datum) 0; + att[i] = (Datum) 0; } } @@ -712,12 +712,12 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple) */ for (; i < r->rd_att->natts; i++) { - fetchatt[i] = index_getattr(tuple, i + 1, giststate->leafTupdesc, + att[i] = index_getattr(tuple, i + 1, giststate->leafTupdesc, &isnull[i]); } MemoryContextSwitchTo(oldcxt); - return heap_form_tuple(giststate->fetchTupdesc, fetchatt, isnull); + return heap_form_tuple(giststate->fetchTupdesc, att, isnull); } float diff --git a/src/include/access/itup.h b/src/include/access/itup.h index 2c8877e991..6e6aec1043 100644 --- a/src/include/access/itup.h +++ b/src/include/access/itup.h @@ -114,7 +114,7 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap; ) \ : \ ( \ - (att_isnull((attnum)-1, (char *)(tup) + sizeof(IndexTupleData))) ? \ + (att_isnull((attnum)-1, (bits8 *)(tup) + sizeof(IndexTupleData))) ? \ ( \ *(isnull) = true, \ (Datum)NULL \ diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h index 16c74a581e..3f6e9d146f 100644 --- a/src/include/access/tupmacs.h +++ b/src/include/access/tupmacs.h @@ -14,6 +14,7 @@ #ifndef TUPMACS_H #define TUPMACS_H +#include "catalog/pg_attribute.h" #include "catalog/pg_type_d.h" /* for TYPALIGN macros */ @@ -22,10 +23,14 @@ * Note that a 0 in the null bitmap indicates a null, while 1 indicates * non-null. */ -#define att_isnull(ATT, BITS) (!((BITS)[(ATT) >> 3] & (1 << ((ATT) & 0x07)))) +static inline bool +att_isnull(int ATT, const bits8 *BITS) +{ + return !(BITS[ATT >> 3] & (1 << (ATT & 0x07))); +} /* - * Given a Form_pg_attribute and a pointer into a tuple's data area, + * Given byval/len parameters and a pointer into a tuple's data area, * return the correct value or pointer. * * We return a Datum value in all cases. If the attribute has "byval" false, @@ -38,61 +43,40 @@ * * Note that T must already be properly aligned for this to work correctly. */ -#define fetchatt(A,T) fetch_att(T, (A)->attbyval, (A)->attlen) +static inline Datum +fetch_att(const void *T, bool attbyval, int attlen) +{ + if (attbyval) + { + switch (attlen) + { + case sizeof(char): + return CharGetDatum(*((const char *) T)); + case sizeof(int16): + return Int16GetDatum(*((const int16 *) T)); + case sizeof(int32): + return Int32GetDatum(*((const int32 *) T)); +#if SIZEOF_DATUM == 8 + case sizeof(Datum): + return *((const Datum *) T); +#endif + default: + elog(ERROR, "unsupported byval length: %d", attlen); + return 0; + } + } + else + return PointerGetDatum(T); +} /* - * Same, but work from byval/len parameters rather than Form_pg_attribute. + * Same, but work from a Form_pg_attribute rather than byval/len parameters. */ -#if SIZEOF_DATUM == 8 - -#define fetch_att(T,attbyval,attlen) \ -( \ - (attbyval) ? \ - ( \ - (attlen) == (int) sizeof(Datum) ? \ - *((Datum *)(T)) \ - : \ - ( \ - (attlen) == (int) sizeof(int32) ? \ - Int32GetDatum(*((int32 *)(T))) \ - : \ - ( \ - (attlen) == (int) sizeof(int16) ? \ - Int16GetDatum(*((int16 *)(T))) \ - : \ - ( \ - AssertMacro((attlen) == 1), \ - CharGetDatum(*((char *)(T))) \ - ) \ - ) \ - ) \ - ) \ - : \ - PointerGetDatum((char *) (T)) \ -) -#else /* SIZEOF_DATUM != 8 */ - -#define fetch_att(T,attbyval,attlen) \ -( \ - (attbyval) ? \ - ( \ - (attlen) == (int) sizeof(int32) ? \ - Int32GetDatum(*((int32 *)(T))) \ - : \ - ( \ - (attlen) == (int) sizeof(int16) ? \ - Int16GetDatum(*((int16 *)(T))) \ - : \ - ( \ - AssertMacro((attlen) == 1), \ - CharGetDatum(*((char *)(T))) \ - ) \ - ) \ - ) \ - : \ - PointerGetDatum((char *) (T)) \ -) -#endif /* SIZEOF_DATUM == 8 */ +static inline Datum +fetchatt(const FormData_pg_attribute *A, const void *T) +{ + return fetch_att(T, A->attbyval, A->attlen); +} /* * att_align_datum aligns the given offset as needed for a datum of alignment @@ -194,54 +178,31 @@ * store_att_byval is a partial inverse of fetch_att: store a given Datum * value into a tuple data area at the specified address. However, it only * handles the byval case, because in typical usage the caller needs to - * distinguish by-val and by-ref cases anyway, and so a do-it-all macro + * distinguish by-val and by-ref cases anyway, and so a do-it-all function * wouldn't be convenient. */ +static inline void +store_att_byval(void *T, Datum newdatum, int attlen) +{ + switch (attlen) + { + case sizeof(char): + *(char *) T = DatumGetChar(newdatum); + break; + case sizeof(int16): + *(int16 *) T = DatumGetInt16(newdatum); + break; + case sizeof(int32): + *(int32 *) T = DatumGetInt32(newdatum); + break; #if SIZEOF_DATUM == 8 - -#define store_att_byval(T,newdatum,attlen) \ - do { \ - switch (attlen) \ - { \ - case sizeof(char): \ - *(char *) (T) = DatumGetChar(newdatum); \ - break; \ - case sizeof(int16): \ - *(int16 *) (T) = DatumGetInt16(newdatum); \ - break; \ - case sizeof(int32): \ - *(int32 *) (T) = DatumGetInt32(newdatum); \ - break; \ - case sizeof(Datum): \ - *(Datum *) (T) = (newdatum); \ - break; \ - default: \ - elog(ERROR, "unsupported byval length: %d", \ - (int) (attlen)); \ - break; \ - } \ - } while (0) -#else /* SIZEOF_DATUM != 8 */ - -#define store_att_byval(T,newdatum,attlen) \ - do { \ - switch (attlen) \ - { \ - case sizeof(char): \ - *(char *) (T) = DatumGetChar(newdatum); \ - break; \ - case sizeof(int16): \ - *(int16 *) (T) = DatumGetInt16(newdatum); \ - break; \ - case sizeof(int32): \ - *(int32 *) (T) = DatumGetInt32(newdatum); \ - break; \ - default: \ - elog(ERROR, "unsupported byval length: %d", \ - (int) (attlen)); \ - break; \ - } \ - } while (0) -#endif /* SIZEOF_DATUM == 8 */ + case sizeof(Datum): + *(Datum *) T = newdatum; + break; +#endif + default: + elog(ERROR, "unsupported byval length: %d", attlen); + } +} #endif -- 2.36.1
From 4e065f7a2097534801f299649fc69993e1b81a6d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (itup.h) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/access/itup.h | 107 +++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 54 deletions(-) diff --git a/src/include/access/itup.h b/src/include/access/itup.h index 6e6aec1043..36e5e87b9c 100644 --- a/src/include/access/itup.h +++ b/src/include/access/itup.h @@ -73,21 +73,33 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap; #define IndexTupleHasVarwidths(itup) ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK)) +/* routines in indextuple.c */ +extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor, + Datum *values, bool *isnull); +extern Datum nocache_index_getattr(IndexTuple tup, int attnum, + TupleDesc tupleDesc); +extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor, + Datum *values, bool *isnull); +extern void index_deform_tuple_internal(TupleDesc tupleDescriptor, + Datum *values, bool *isnull, + char *tp, bits8 *bp, int hasnulls); +extern IndexTuple CopyIndexTuple(IndexTuple source); +extern IndexTuple index_truncate_tuple(TupleDesc sourceDescriptor, + IndexTuple source, int leavenatts); + + /* * Takes an infomask as argument (primarily because this needs to be usable * at index_form_tuple time so enough space is allocated). */ -#define IndexInfoFindDataOffset(t_info) \ -( \ - (!((t_info) & INDEX_NULL_MASK)) ? \ - ( \ - (Size)MAXALIGN(sizeof(IndexTupleData)) \ - ) \ - : \ - ( \ - (Size)MAXALIGN(sizeof(IndexTupleData) + sizeof(IndexAttributeBitMapData)) \ - ) \ -) +static inline Size +IndexInfoFindDataOffset(unsigned short t_info) +{ + if (!(t_info & INDEX_NULL_MASK)) + return MAXALIGN(sizeof(IndexTupleData)); + else + return MAXALIGN(sizeof(IndexTupleData) + sizeof(IndexAttributeBitMapData)); +} /* ---------------- * index_getattr @@ -97,34 +109,36 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap; * * ---------------- */ -#define index_getattr(tup, attnum, tupleDesc, isnull) \ -( \ - AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \ - *(isnull) = false, \ - !IndexTupleHasNulls(tup) ? \ - ( \ - TupleDescAttr((tupleDesc), (attnum)-1)->attcacheoff >= 0 ? \ - ( \ - fetchatt(TupleDescAttr((tupleDesc), (attnum)-1), \ - (char *) (tup) + IndexInfoFindDataOffset((tup)->t_info) \ - + TupleDescAttr((tupleDesc), (attnum)-1)->attcacheoff) \ - ) \ - : \ - nocache_index_getattr((tup), (attnum), (tupleDesc)) \ - ) \ - : \ - ( \ - (att_isnull((attnum)-1, (bits8 *)(tup) + sizeof(IndexTupleData))) ? \ - ( \ - *(isnull) = true, \ - (Datum)NULL \ - ) \ - : \ - ( \ - nocache_index_getattr((tup), (attnum), (tupleDesc)) \ - ) \ - ) \ -) +static inline Datum +index_getattr(IndexTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull) +{ + Assert(PointerIsValid(isnull)); + Assert(attnum > 0); + + *isnull = false; + + if (!IndexTupleHasNulls(tup)) + { + if (TupleDescAttr(tupleDesc, attnum - 1)->attcacheoff >= 0) + { + return fetchatt(TupleDescAttr(tupleDesc, attnum - 1), + (char *) tup + IndexInfoFindDataOffset(tup->t_info) + + TupleDescAttr(tupleDesc, attnum-1)->attcacheoff); + } + else + return nocache_index_getattr(tup, attnum, tupleDesc); + } + else + { + if (att_isnull(attnum - 1, (bits8 *) tup + sizeof(IndexTupleData))) + { + *isnull = true; + return (Datum) NULL; + } + else + return nocache_index_getattr(tup, attnum, tupleDesc); + } +} /* * MaxIndexTuplesPerPage is an upper bound on the number of tuples that can @@ -146,19 +160,4 @@ typedef IndexAttributeBitMapData * IndexAttributeBitMap; ((int) ((BLCKSZ - SizeOfPageHeaderData) / \ (MAXALIGN(sizeof(IndexTupleData) + 1) + sizeof(ItemIdData)))) - -/* routines in indextuple.c */ -extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor, - Datum *values, bool *isnull); -extern Datum nocache_index_getattr(IndexTuple tup, int attnum, - TupleDesc tupleDesc); -extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor, - Datum *values, bool *isnull); -extern void index_deform_tuple_internal(TupleDesc tupleDescriptor, - Datum *values, bool *isnull, - char *tp, bits8 *bp, int hasnulls); -extern IndexTuple CopyIndexTuple(IndexTuple source); -extern IndexTuple index_truncate_tuple(TupleDesc sourceDescriptor, - IndexTuple source, int leavenatts); - #endif /* ITUP_H */ -- 2.36.1
From c8a2beac1c278cfc0daafffc7f678697970ecd18 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 23 May 2022 07:28:13 +0200 Subject: [PATCH v2] Convert macros to static inline functions (pgstat.h) Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- src/include/pgstat.h | 227 ++++++++++++++++++++++++------------------- 1 file changed, 125 insertions(+), 102 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index ac28f813b4..e7a122dcdb 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -16,6 +16,7 @@ #include "postmaster/pgarch.h" /* for MAX_XFN_CHARS */ #include "utils/backend_progress.h" /* for backward compatibility */ #include "utils/backend_status.h" /* for backward compatibility */ +#include "utils/rel.h" #include "utils/relcache.h" #include "utils/wait_event.h" /* for backward compatibility */ @@ -396,6 +397,62 @@ typedef struct PgStat_WalStats } PgStat_WalStats; +/* + * Variables in pgstat.c + */ + +/* GUC parameters */ +extern PGDLLIMPORT bool pgstat_track_counts; +extern PGDLLIMPORT int pgstat_track_functions; +extern PGDLLIMPORT int pgstat_fetch_consistency; + + +/* + * Variables in pgstat_bgwriter.c + */ + +/* updated directly by bgwriter and bufmgr */ +extern PGDLLIMPORT PgStat_BgWriterStats PendingBgWriterStats; + + +/* + * Variables in pgstat_checkpointer.c + */ + +/* + * Checkpointer statistics counters are updated directly by checkpointer and + * bufmgr. + */ +extern PGDLLIMPORT PgStat_CheckpointerStats PendingCheckpointerStats; + + +/* + * Variables in pgstat_database.c + */ + +/* Updated by pgstat_count_buffer_*_time macros */ +extern PGDLLIMPORT PgStat_Counter pgStatBlockReadTime; +extern PGDLLIMPORT PgStat_Counter pgStatBlockWriteTime; + +/* + * Updated by pgstat_count_conn_*_time macros, called by + * pgstat_report_activity(). + */ +extern PGDLLIMPORT PgStat_Counter pgStatActiveTime; +extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime; + +/* updated by the traffic cop and in errfinish() */ +extern PGDLLIMPORT SessionEndType pgStatSessionEndCause; + + +/* + * Variables in pgstat_wal.c + */ + +/* updated directly by backends and background processes */ +extern PGDLLIMPORT PgStat_WalStats PendingWalStats; + + /* * Functions in pgstat.c */ @@ -465,14 +522,26 @@ extern void pgstat_report_checksum_failures_in_db(Oid dboid, int failurecount); extern void pgstat_report_checksum_failure(void); extern void pgstat_report_connect(Oid dboid); -#define pgstat_count_buffer_read_time(n) \ - (pgStatBlockReadTime += (n)) -#define pgstat_count_buffer_write_time(n) \ - (pgStatBlockWriteTime += (n)) -#define pgstat_count_conn_active_time(n) \ - (pgStatActiveTime += (n)) -#define pgstat_count_conn_txn_idle_time(n) \ - (pgStatTransactionIdleTime += (n)) +static inline void +pgstat_count_buffer_read_time(PgStat_Counter n) +{ + pgStatBlockReadTime += n; +} +static inline void +pgstat_count_buffer_write_time(PgStat_Counter n) +{ + pgStatBlockWriteTime += n; +} +static inline void +pgstat_count_conn_active_time(PgStat_Counter n) +{ + pgStatActiveTime += n; +} +static inline void +pgstat_count_conn_txn_idle_time(PgStat_Counter n) +{ + pgStatTransactionIdleTime += n; +} extern PgStat_StatDBEntry *pgstat_fetch_stat_dbentry(Oid dbid); @@ -516,47 +585,57 @@ extern void pgstat_report_analyze(Relation rel, * pgstat_assoc_relation() to do so. See its comment for why this is done * separately from pgstat_init_relation(). */ -#define pgstat_should_count_relation(rel) \ - (likely((rel)->pgstat_info != NULL) ? true : \ - ((rel)->pgstat_enabled ? pgstat_assoc_relation(rel), true : false)) +static inline bool +pgstat_should_count_relation(Relation rel) +{ + return (likely(rel->pgstat_info != NULL) ? true : + (rel->pgstat_enabled ? pgstat_assoc_relation(rel), true : false)); +} /* nontransactional event counts are simple enough to inline */ -#define pgstat_count_heap_scan(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_numscans++; \ - } while (0) -#define pgstat_count_heap_getnext(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_tuples_returned++; \ - } while (0) -#define pgstat_count_heap_fetch(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_tuples_fetched++; \ - } while (0) -#define pgstat_count_index_scan(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_numscans++; \ - } while (0) -#define pgstat_count_index_tuples(rel, n) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_tuples_returned += (n); \ - } while (0) -#define pgstat_count_buffer_read(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_blocks_fetched++; \ - } while (0) -#define pgstat_count_buffer_hit(rel) \ - do { \ - if (pgstat_should_count_relation(rel)) \ - (rel)->pgstat_info->t_counts.t_blocks_hit++; \ - } while (0) +static inline void +pgstat_count_heap_scan(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_numscans++; +} +static inline void +pgstat_count_heap_getnext(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_tuples_returned++; +} +static inline void +pgstat_count_heap_fetch(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_tuples_fetched++; +} +static inline void +pgstat_count_index_scan(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_numscans++; +} +static inline void +pgstat_count_index_tuples(Relation rel, PgStat_Counter n) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_tuples_returned += n; +} +static inline void +pgstat_count_buffer_read(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_blocks_fetched++; +} +static inline void +pgstat_count_buffer_hit(Relation rel) +{ + if (pgstat_should_count_relation(rel)) + rel->pgstat_info->t_counts.t_blocks_hit++; +} extern void pgstat_count_heap_insert(Relation rel, PgStat_Counter n); extern void pgstat_count_heap_update(Relation rel, bool hot); @@ -636,60 +715,4 @@ extern void pgstat_report_wal(bool force); extern PgStat_WalStats *pgstat_fetch_stat_wal(void); -/* - * Variables in pgstat.c - */ - -/* GUC parameters */ -extern PGDLLIMPORT bool pgstat_track_counts; -extern PGDLLIMPORT int pgstat_track_functions; -extern PGDLLIMPORT int pgstat_fetch_consistency; - - -/* - * Variables in pgstat_bgwriter.c - */ - -/* updated directly by bgwriter and bufmgr */ -extern PGDLLIMPORT PgStat_BgWriterStats PendingBgWriterStats; - - -/* - * Variables in pgstat_checkpointer.c - */ - -/* - * Checkpointer statistics counters are updated directly by checkpointer and - * bufmgr. - */ -extern PGDLLIMPORT PgStat_CheckpointerStats PendingCheckpointerStats; - - -/* - * Variables in pgstat_database.c - */ - -/* Updated by pgstat_count_buffer_*_time macros */ -extern PGDLLIMPORT PgStat_Counter pgStatBlockReadTime; -extern PGDLLIMPORT PgStat_Counter pgStatBlockWriteTime; - -/* - * Updated by pgstat_count_conn_*_time macros, called by - * pgstat_report_activity(). - */ -extern PGDLLIMPORT PgStat_Counter pgStatActiveTime; -extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime; - -/* updated by the traffic cop and in errfinish() */ -extern PGDLLIMPORT SessionEndType pgStatSessionEndCause; - - -/* - * Variables in pgstat_wal.c - */ - -/* updated directly by backends and background processes */ -extern PGDLLIMPORT PgStat_WalStats PendingWalStats; - - #endif /* PGSTAT_H */ -- 2.36.1