On 2015-08-05 15:08:29 +0200, Andres Freund wrote:
> We might later want to change some of the harder to maintain macros to
> inline functions, but that seems better done separately.
Here's a conversion for fastgetattr() and heap_getattr(). Not only is
the resulting code significantly more readable, but the conversion also
shrinks the code size:
$ ls -l src/backend/postgres_stock src/backend/postgres
-rwxr-xr-x 1 andres andres 37054832 Aug 5 15:18 src/backend/postgres_stock
-rwxr-xr-x 1 andres andres 37209288 Aug 5 15:23 src/backend/postgres
$ size src/backend/postgres_stock src/backend/postgres
text data bss dec hex filename
7026843 49982 298584 7375409 708a31 src/backend/postgres_stock
7023851 49982 298584 7372417 707e81 src/backend/postgres
stock is the binary compiled without the conversion.
A lot of the size difference is debugging information (which now needs
less duplicated information on each callsite), but you can see that the
text section (the actual executable code) shrank by 3k.
stripping the binary shows exactly that:
-rwxr-xr-x 1 andres andres 7076760 Aug 5 15:44 src/backend/postgres_s
-rwxr-xr-x 1 andres andres 7079512 Aug 5 15:45 src/backend/postgres_stock_s
To be sure this doesn't cause problems I ran a readonly pgbench. There's
a very slight, but reproducible, performance benefit. I don't think
that's a reason for the change, I just wanted to make sure there's no
regressions.
Regards,
Andres
>From cca830c39a6c46caf0c68b340399cf60f04ad31f Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 5 Aug 2015 15:30:20 +0200
Subject: [PATCH] Convert fastgetattr() and heap_getattr() into inline
functions.
The current macro is very hard to read. Now that we can unconditionally
use inline functions there's no reason to keep them that way.
In my gcc 5 based environment this shaves of nearly 200k of the final
binary size. A lot of that is debugging information, but the .text
section alone shrinks by 3k.
---
src/backend/access/heap/heapam.c | 46 -----------
src/include/access/htup_details.h | 157 ++++++++++++++++++--------------------
2 files changed, 75 insertions(+), 128 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 3701d8e..ff93b3c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -983,52 +983,6 @@ heapgettup_pagemode(HeapScanDesc scan,
}
-#if defined(DISABLE_COMPLEX_MACRO)
-/*
- * This is formatted so oddly so that the correspondence to the macro
- * definition in access/htup_details.h is maintained.
- */
-Datum
-fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
- bool *isnull)
-{
- return (
- (attnum) > 0 ?
- (
- (*(isnull) = false),
- HeapTupleNoNulls(tup) ?
- (
- (tupleDesc)->attrs[(attnum) - 1]->attcacheoff >= 0 ?
- (
- fetchatt((tupleDesc)->attrs[(attnum) - 1],
- (char *) (tup)->t_data + (tup)->t_data->t_hoff +
- (tupleDesc)->attrs[(attnum) - 1]->attcacheoff)
- )
- :
- nocachegetattr((tup), (attnum), (tupleDesc))
- )
- :
- (
- att_isnull((attnum) - 1, (tup)->t_data->t_bits) ?
- (
- (*(isnull) = true),
- (Datum) NULL
- )
- :
- (
- nocachegetattr((tup), (attnum), (tupleDesc))
- )
- )
- )
- :
- (
- (Datum) NULL
- )
- );
-}
-#endif /* defined(DISABLE_COMPLEX_MACRO) */
-
-
/* ----------------------------------------------------------------
* heap access method interface
* ----------------------------------------------------------------
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 8dd530bd..9d49c5e 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -673,6 +673,38 @@ struct MinimalTupleData
#define HeapTupleSetOid(tuple, oid) \
HeapTupleHeaderSetOid((tuple)->t_data, (oid))
+/* prototypes for functions in common/heaptuple.c */
+extern Size heap_compute_data_size(TupleDesc tupleDesc,
+ Datum *values, bool *isnull);
+extern void heap_fill_tuple(TupleDesc tupleDesc,
+ Datum *values, bool *isnull,
+ char *data, Size data_size,
+ uint16 *infomask, bits8 *bit);
+extern bool heap_attisnull(HeapTuple tup, int attnum);
+extern Datum nocachegetattr(HeapTuple tup, int attnum,
+ TupleDesc att);
+extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
+ bool *isnull);
+extern HeapTuple heap_copytuple(HeapTuple tuple);
+extern void heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest);
+extern Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc);
+extern HeapTuple heap_form_tuple(TupleDesc tupleDescriptor,
+ Datum *values, bool *isnull);
+extern HeapTuple heap_modify_tuple(HeapTuple tuple,
+ TupleDesc tupleDesc,
+ Datum *replValues,
+ bool *replIsnull,
+ bool *doReplace);
+extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
+ Datum *values, bool *isnull);
+extern void heap_freetuple(HeapTuple htup);
+extern MinimalTuple heap_form_minimal_tuple(TupleDesc tupleDescriptor,
+ Datum *values, bool *isnull);
+extern void heap_free_minimal_tuple(MinimalTuple mtup);
+extern MinimalTuple heap_copy_minimal_tuple(MinimalTuple mtup);
+extern HeapTuple heap_tuple_from_minimal_tuple(MinimalTuple mtup);
+extern MinimalTuple minimal_tuple_from_heap_tuple(HeapTuple htup);
+
/* ----------------
* fastgetattr
@@ -688,43 +720,34 @@ struct MinimalTupleData
* lookups, and call nocachegetattr() for the rest.
* ----------------
*/
+static inline Datum
+fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
+ bool *isnull)
+{
+ AssertArg(attnum > 0);
-#if !defined(DISABLE_COMPLEX_MACRO)
-
-#define fastgetattr(tup, attnum, tupleDesc, isnull) \
-( \
- AssertMacro((attnum) > 0), \
- (*(isnull) = false), \
- HeapTupleNoNulls(tup) ? \
- ( \
- (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
- ( \
- fetchatt((tupleDesc)->attrs[(attnum)-1], \
- (char *) (tup)->t_data + (tup)->t_data->t_hoff + \
- (tupleDesc)->attrs[(attnum)-1]->attcacheoff) \
- ) \
- : \
- nocachegetattr((tup), (attnum), (tupleDesc)) \
- ) \
- : \
- ( \
- att_isnull((attnum)-1, (tup)->t_data->t_bits) ? \
- ( \
- (*(isnull) = true), \
- (Datum)NULL \
- ) \
- : \
- ( \
- nocachegetattr((tup), (attnum), (tupleDesc)) \
- ) \
- ) \
-)
-#else /* defined(DISABLE_COMPLEX_MACRO) */
-
-extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
- bool *isnull);
-#endif /* defined(DISABLE_COMPLEX_MACRO) */
+ *isnull = false;
+
+ if (HeapTupleNoNulls(tup))
+ {
+ Form_pg_attribute attr = tupleDesc->attrs[attnum - 1];
+ if (attr->attcacheoff >= 0)
+ {
+ return fetchatt(attr,
+ (char *) tup->t_data +
+ tup->t_data->t_hoff +
+ attr->attcacheoff);
+ }
+ /* fall through to nocachegetattr */
+ }
+ else if (att_isnull(attnum - 1, tup->t_data->t_bits))
+ {
+ *isnull = true;
+ return (Datum) NULL;
+ }
+ return nocachegetattr(tup, attnum, tupleDesc);
+}
/* ----------------
* heap_getattr
@@ -741,53 +764,23 @@ extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
* pointer to the structure describing the row and all its fields.
* ----------------
*/
-#define heap_getattr(tup, attnum, tupleDesc, isnull) \
- ( \
- ((attnum) > 0) ? \
- ( \
- ((attnum) > (int) HeapTupleHeaderGetNatts((tup)->t_data)) ? \
- ( \
- (*(isnull) = true), \
- (Datum)NULL \
- ) \
- : \
- fastgetattr((tup), (attnum), (tupleDesc), (isnull)) \
- ) \
- : \
- heap_getsysattr((tup), (attnum), (tupleDesc), (isnull)) \
- )
-
+static inline Datum
+heap_getattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
+ bool *isnull)
+{
+ if (attnum > 0)
+ {
+ if (attnum > HeapTupleHeaderGetNatts((tup)->t_data))
+ {
+ *isnull = true;
+ return (Datum) NULL;
+ }
+ else
+ return fastgetattr(tup, attnum, tupleDesc, isnull);
+ }
+
+ return heap_getsysattr(tup, attnum, tupleDesc, isnull);
+}
-/* prototypes for functions in common/heaptuple.c */
-extern Size heap_compute_data_size(TupleDesc tupleDesc,
- Datum *values, bool *isnull);
-extern void heap_fill_tuple(TupleDesc tupleDesc,
- Datum *values, bool *isnull,
- char *data, Size data_size,
- uint16 *infomask, bits8 *bit);
-extern bool heap_attisnull(HeapTuple tup, int attnum);
-extern Datum nocachegetattr(HeapTuple tup, int attnum,
- TupleDesc att);
-extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
- bool *isnull);
-extern HeapTuple heap_copytuple(HeapTuple tuple);
-extern void heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest);
-extern Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc);
-extern HeapTuple heap_form_tuple(TupleDesc tupleDescriptor,
- Datum *values, bool *isnull);
-extern HeapTuple heap_modify_tuple(HeapTuple tuple,
- TupleDesc tupleDesc,
- Datum *replValues,
- bool *replIsnull,
- bool *doReplace);
-extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
- Datum *values, bool *isnull);
-extern void heap_freetuple(HeapTuple htup);
-extern MinimalTuple heap_form_minimal_tuple(TupleDesc tupleDescriptor,
- Datum *values, bool *isnull);
-extern void heap_free_minimal_tuple(MinimalTuple mtup);
-extern MinimalTuple heap_copy_minimal_tuple(MinimalTuple mtup);
-extern HeapTuple heap_tuple_from_minimal_tuple(MinimalTuple mtup);
-extern MinimalTuple minimal_tuple_from_heap_tuple(HeapTuple htup);
#endif /* HTUP_DETAILS_H */
--
2.3.0.149.gf3f4077.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
