The ilist patch from Andres Freund introduces a cute trick for defining maybe-inline functions, which works regardless of whether the compiler supports inlining, and eliminates the need to write the code twice (first in the header and also the .c file.) It's really quite a simple thing, but the whole topic on inlining has generated a lot of debate.
So to remove that controversy out of said patch, here's a preliminary one: get rid of duplicate definitions in sortsupport.c, list.c, mcxt.c (which are currently defined as static inline in their respective headers for compilers that support it, and as regular functions in the .c files for those that don't). What's being done in this patch is exactly what would be done in the ilist code. So if there are problems with this, please speak up. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/nodes/list.c --- b/src/backend/nodes/list.c *************** *** 15,20 **** --- 15,23 ---- */ #include "postgres.h" + /* see pg_list.h */ + #define PG_LIST_INCLUDE_DEFINITIONS + #include "nodes/pg_list.h" *************** *** 1224,1254 **** list_copy_tail(const List *oldlist, int nskip) } /* - * pg_list.h defines inline versions of these functions if allowed by the - * compiler; in which case the definitions below are skipped. - */ - #ifndef USE_INLINE - - ListCell * - list_head(const List *l) - { - return l ? l->head : NULL; - } - - ListCell * - list_tail(List *l) - { - return l ? l->tail : NULL; - } - - int - list_length(const List *l) - { - return l ? l->length : 0; - } - #endif /* ! USE_INLINE */ - - /* * Temporary compatibility functions * * In order to avoid warnings for these function definitions, we need --- 1227,1232 ---- *** a/src/backend/utils/mmgr/mcxt.c --- b/src/backend/utils/mmgr/mcxt.c *************** *** 21,26 **** --- 21,29 ---- #include "postgres.h" + /* see palloc.h */ + #define MCXT_INCLUDE_DEFINITIONS + #include "utils/memutils.h" *************** *** 696,723 **** repalloc(void *pointer, Size size) } /* - * MemoryContextSwitchTo - * Returns the current context; installs the given context. - * - * palloc.h defines an inline version of this function if allowed by the - * compiler; in which case the definition below is skipped. - */ - #ifndef USE_INLINE - - MemoryContext - MemoryContextSwitchTo(MemoryContext context) - { - MemoryContext old; - - AssertArg(MemoryContextIsValid(context)); - - old = CurrentMemoryContext; - CurrentMemoryContext = context; - return old; - } - #endif /* ! USE_INLINE */ - - /* * MemoryContextStrdup * Like strdup(), but allocate from the specified context */ --- 699,704 ---- *** a/src/backend/utils/sort/sortsupport.c --- b/src/backend/utils/sort/sortsupport.c *************** *** 15,20 **** --- 15,23 ---- #include "postgres.h" + /* See sortsupport.h */ + #define SORTSUPPORT_INCLUDE_DEFINITIONS + #include "fmgr.h" #include "utils/lsyscache.h" #include "utils/sortsupport.h" *************** *** 29,78 **** typedef struct /* - * sortsupport.h defines inline versions of these functions if allowed by the - * compiler; in which case the definitions below are skipped. - */ - #ifndef USE_INLINE - - /* - * Apply a sort comparator function and return a 3-way comparison result. - * This takes care of handling reverse-sort and NULLs-ordering properly. - */ - int - ApplySortComparator(Datum datum1, bool isNull1, - Datum datum2, bool isNull2, - SortSupport ssup) - { - int compare; - - if (isNull1) - { - if (isNull2) - compare = 0; /* NULL "=" NULL */ - else if (ssup->ssup_nulls_first) - compare = -1; /* NULL "<" NOT_NULL */ - else - compare = 1; /* NULL ">" NOT_NULL */ - } - else if (isNull2) - { - if (ssup->ssup_nulls_first) - compare = 1; /* NOT_NULL ">" NULL */ - else - compare = -1; /* NOT_NULL "<" NULL */ - } - else - { - compare = (*ssup->comparator) (datum1, datum2, ssup); - if (ssup->ssup_reverse) - compare = -compare; - } - - return compare; - } - #endif /* ! USE_INLINE */ - - /* * Shim function for calling an old-style comparator * * This is essentially an inlined version of FunctionCall2Coll(), except --- 32,37 ---- *** a/src/include/c.h --- b/src/include/c.h *************** *** 744,749 **** typedef NameData *Name; --- 744,764 ---- #endif /* HAVE__BUILTIN_TYPES_COMPATIBLE_P */ + /* + * Inlining support + * + * Allow modules to declare functions that are inlined if the compiler supports + * it. The functions must be defined in its header, protected by a special + * #ifdef that the .c file defines. If the compiler does not support inlines, + * the function definitions are pulled in by the .c file as regular (not + * inline) symbols; the prototype must also exist in the header in that case. + */ + #ifdef USE_INLINE + #define STATIC_IF_INLINE static inline + #else + #define STATIC_IF_INLINE + #endif /* USE_INLINE */ + /* ---------------------------------------------------------------- * Section 7: random stuff * ---------------------------------------------------------------- *** a/src/include/nodes/pg_list.h --- b/src/include/nodes/pg_list.h *************** *** 73,104 **** struct ListCell * them as macros, since we want to avoid double-evaluation of macro * arguments. Therefore, we implement them using static inline functions * if supported by the compiler, or as regular functions otherwise. */ ! #ifdef USE_INLINE ! ! static inline ListCell * list_head(const List *l) { return l ? l->head : NULL; } ! static inline ListCell * list_tail(List *l) { return l ? l->tail : NULL; } ! static inline int list_length(const List *l) { return l ? l->length : 0; } ! #else ! ! extern ListCell *list_head(const List *l); ! extern ListCell *list_tail(List *l); ! extern int list_length(const List *l); ! #endif /* USE_INLINE */ /* * NB: There is an unfortunate legacy from a previous incarnation of --- 73,104 ---- * them as macros, since we want to avoid double-evaluation of macro * arguments. Therefore, we implement them using static inline functions * if supported by the compiler, or as regular functions otherwise. + * See STATIC_IF_INLINE in c.h. */ ! #ifndef USE_INLINE ! extern ListCell *list_head(const List *l); ! extern ListCell *list_tail(List *l); ! extern int list_length(const List *l); ! #endif /* USE_INLINE */ ! #if defined(USE_INLINE) || defined(PG_LIST_INCLUDE_DEFINITIONS) ! STATIC_IF_INLINE ListCell * list_head(const List *l) { return l ? l->head : NULL; } ! STATIC_IF_INLINE ListCell * list_tail(List *l) { return l ? l->tail : NULL; } ! STATIC_IF_INLINE int list_length(const List *l) { return l ? l->length : 0; } ! #endif /* USE_INLINE || PG_LIST_INCLUDE_DEFINITIONS */ /* * NB: There is an unfortunate legacy from a previous incarnation of *** a/src/include/utils/palloc.h --- b/src/include/utils/palloc.h *************** *** 73,86 **** extern void *repalloc(void *pointer, Size size); /* * MemoryContextSwitchTo can't be a macro in standard C compilers. * But we can make it an inline function if the compiler supports it. * * This file has to be includable by some non-backend code such as * pg_resetxlog, so don't expose the CurrentMemoryContext reference * if FRONTEND is defined. */ ! #if defined(USE_INLINE) && !defined(FRONTEND) ! ! static inline MemoryContext MemoryContextSwitchTo(MemoryContext context) { MemoryContext old = CurrentMemoryContext; --- 73,87 ---- /* * MemoryContextSwitchTo can't be a macro in standard C compilers. * But we can make it an inline function if the compiler supports it. + * See STATIC_IF_INLINE in c.h. * * This file has to be includable by some non-backend code such as * pg_resetxlog, so don't expose the CurrentMemoryContext reference * if FRONTEND is defined. */ ! #ifndef FRONTEND ! #if defined(USE_INLINE) || defined(MCXT_INCLUDE_DEFINITIONS) ! STATIC_IF_INLINE MemoryContext MemoryContextSwitchTo(MemoryContext context) { MemoryContext old = CurrentMemoryContext; *************** *** 88,97 **** MemoryContextSwitchTo(MemoryContext context) CurrentMemoryContext = context; return old; } ! #else extern MemoryContext MemoryContextSwitchTo(MemoryContext context); ! #endif /* USE_INLINE && !FRONTEND */ /* * These are like standard strdup() except the copied string is --- 89,101 ---- CurrentMemoryContext = context; return old; } ! #endif + #ifndef USE_INLINE extern MemoryContext MemoryContextSwitchTo(MemoryContext context); ! #endif /* !USE_INLINE */ ! ! #endif /* !FRONTEND */ /* * These are like standard strdup() except the copied string is *** a/src/include/utils/sortsupport.h --- b/src/include/utils/sortsupport.h *************** *** 101,114 **** typedef struct SortSupportData } SortSupportData; ! /* ApplySortComparator should be inlined if possible */ ! #ifdef USE_INLINE ! /* * Apply a sort comparator function and return a 3-way comparison result. * This takes care of handling reverse-sort and NULLs-ordering properly. */ ! static inline int ApplySortComparator(Datum datum1, bool isNull1, Datum datum2, bool isNull2, SortSupport ssup) --- 101,121 ---- } SortSupportData; ! /* ! * ApplySortComparator should be inlined if possible. See STATIC_IF_INLINE ! * in c.h. ! */ ! #ifndef USE_INLINE ! extern int ApplySortComparator(Datum datum1, bool isNull1, ! Datum datum2, bool isNull2, ! SortSupport ssup); ! #endif /* !USE_INLINE */ ! #if defined(USE_INLINE) || defined(SORTSUPPORT_INCLUDE_DEFINITIONS) /* * Apply a sort comparator function and return a 3-way comparison result. * This takes care of handling reverse-sort and NULLs-ordering properly. */ ! STATIC_IF_INLINE int ApplySortComparator(Datum datum1, bool isNull1, Datum datum2, bool isNull2, SortSupport ssup) *************** *** 140,151 **** ApplySortComparator(Datum datum1, bool isNull1, return compare; } ! #else ! ! extern int ApplySortComparator(Datum datum1, bool isNull1, ! Datum datum2, bool isNull2, ! SortSupport ssup); ! #endif /* USE_INLINE */ /* Other functions in utils/sort/sortsupport.c */ extern void PrepareSortSupportComparisonShim(Oid cmpFunc, SortSupport ssup); --- 147,153 ---- return compare; } ! #endif /* USE_INLINE || SORTSUPPORT_INCLUDE_DEFINITIONS */ /* Other functions in utils/sort/sortsupport.c */ extern void PrepareSortSupportComparisonShim(Oid cmpFunc, SortSupport ssup);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers