On 2015-05-10 12:09:41 -0400, Tom Lane wrote:
> > * I find the ARRAY_ITER_VARS/ARRAY_ITER_NEXT macros rather ugly. I don't
> > buy the argument that turning them into functions will be slower. I'd
> > bet the contrary on common platforms.
> Perhaps; do you want to do some testing and see?
I've added new iterator functions using a on-stack state variable and
array_iter_setup/next functions pretty analogous to the macros. And then
converted arrayfuncs.c to use them.
Codesize before introducing inline functions:
assert:
text data bss dec hex filename
8142400 50562 295952 8488914 8187d2 src/backend/postgres
optimize:
text data bss dec hex filename
6892928 50022 295920 7238870 6e74d6 src/backend/postgres
After:
assert:
text data bss dec hex filename
8133040 50562 295952 8479554 816342 src/backend/postgres
optimize:
text data bss dec hex filename
6890256 50022 295920 7236198 6e6a66 src/backend/postgres
That's a small decrease.
I'm not sure what exactly to use as a performance benchmark
here. For now I chose
SELECT * FROM (SELECT ARRAY(SELECT generate_series(1, 10000))) d,
generate_series(1, 1000) repeat(i);
that'll hit array_out, which uses iterators.
pgbench -P 10 -h /tmp -p 5440 postgres -n -f /tmp/bench.sql -c 4 -T 60
(I chose parallel because it'll show icache efficiency differences)
before, best of four:
tps = 4.921260 (including connections establishing)
after, best of four:
tps = 5.046437 (including connections establishing)
That's a relatively small difference. I'm not surprised, I'd not have
expected anything major.
Personally I think something roughly along those lines is both more
robust and easier to maintain. Even if possibly need to protect against
inlines not being available.
Similarly using inline funcs for AARR_NDIMS/HASNULL does not appear to
hamper performance and gets rid of the multiple evaluation risk.
These patches are obviously WIP. Especially with the iter stuff it's
possible that the concept could be extended a bit further.
Greetings,
Andres Freund
>From 1fe208cda8df5341794ef6b76e11578ecd46cdd8 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 11 May 2015 02:43:06 +0200
Subject: [PATCH 1/2] WIP: Use inline functions for iteration.
---
src/backend/utils/adt/arrayfuncs.c | 68 +++++++++++++++++----------------
src/include/c.h | 2 +
src/include/utils/array.h | 78 +++++++++++++++++++++++++++++++++++++-
3 files changed, 115 insertions(+), 33 deletions(-)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 26fa648..287aca9 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1037,7 +1037,7 @@ array_out(PG_FUNCTION_ARGS)
int ndim,
*dims,
*lb;
- ARRAY_ITER ARRAY_ITER_VARS(iter);
+ array_iter iter;
ArrayMetaState *my_extra;
/*
@@ -1105,7 +1105,7 @@ array_out(PG_FUNCTION_ARGS)
needquotes = (bool *) palloc(nitems * sizeof(bool));
overall_length = 1; /* don't forget to count \0 at end. */
- ARRAY_ITER_SETUP(iter, v);
+ array_iter_setup(&iter, v);
for (i = 0; i < nitems; i++)
{
@@ -1114,7 +1114,8 @@ array_out(PG_FUNCTION_ARGS)
bool needquote;
/* Get source element, checking for NULL */
- ARRAY_ITER_NEXT(iter, i, itemvalue, isnull, typlen, typbyval, typalign);
+ itemvalue = array_iter_next(&iter, i, &isnull,
+ typlen, typbyval, typalign);
if (isnull)
{
@@ -1542,7 +1543,7 @@ array_send(PG_FUNCTION_ARGS)
*dim,
*lb;
StringInfoData buf;
- ARRAY_ITER ARRAY_ITER_VARS(iter);
+ array_iter iter;
ArrayMetaState *my_extra;
/*
@@ -1597,7 +1598,7 @@ array_send(PG_FUNCTION_ARGS)
}
/* Send the array elements using the element's own sendproc */
- ARRAY_ITER_SETUP(iter, v);
+ array_iter_setup(&iter, v);
for (i = 0; i < nitems; i++)
{
@@ -1605,7 +1606,8 @@ array_send(PG_FUNCTION_ARGS)
bool isnull;
/* Get source element, checking for NULL */
- ARRAY_ITER_NEXT(iter, i, itemvalue, isnull, typlen, typbyval, typalign);
+ itemvalue = array_iter_next(&iter, i, &isnull,
+ typlen, typbyval, typalign);
if (isnull)
{
@@ -3105,7 +3107,7 @@ array_map(FunctionCallInfo fcinfo, Oid retType, ArrayMapState *amstate)
int typlen;
bool typbyval;
char typalign;
- ARRAY_ITER ARRAY_ITER_VARS(iter);
+ array_iter iter;
ArrayMetaState *inp_extra;
ArrayMetaState *ret_extra;
@@ -3165,7 +3167,7 @@ array_map(FunctionCallInfo fcinfo, Oid retType, ArrayMapState *amstate)
nulls = (bool *) palloc(nitems * sizeof(bool));
/* Loop over source data */
- ARRAY_ITER_SETUP(iter, v);
+ array_iter_setup(&iter, v);
hasnulls = false;
for (i = 0; i < nitems; i++)
@@ -3173,8 +3175,8 @@ array_map(FunctionCallInfo fcinfo, Oid retType, ArrayMapState *amstate)
bool callit = true;
/* Get source element, checking for NULL */
- ARRAY_ITER_NEXT(iter, i, fcinfo->arg[0], fcinfo->argnull[0],
- inp_typlen, inp_typbyval, inp_typalign);
+ fcinfo->arg[0] = array_iter_next(&iter, i, &fcinfo->argnull[0],
+ inp_typlen, inp_typbyval, inp_typalign);
/*
* Apply the given function to source elt and extra args.
@@ -3572,8 +3574,8 @@ array_eq(PG_FUNCTION_ARGS)
int typlen;
bool typbyval;
char typalign;
- ARRAY_ITER ARRAY_ITER_VARS(it1);
- ARRAY_ITER ARRAY_ITER_VARS(it2);
+ array_iter it1;
+ array_iter it2;
int i;
FunctionCallInfoData locfcinfo;
@@ -3620,8 +3622,8 @@ array_eq(PG_FUNCTION_ARGS)
/* Loop over source data */
nitems = ArrayGetNItems(ndims1, dims1);
- ARRAY_ITER_SETUP(it1, array1);
- ARRAY_ITER_SETUP(it2, array2);
+ array_iter_setup(&it1, array1);
+ array_iter_setup(&it2, array2);
for (i = 0; i < nitems; i++)
{
@@ -3632,8 +3634,10 @@ array_eq(PG_FUNCTION_ARGS)
bool oprresult;
/* Get elements, checking for NULL */
- ARRAY_ITER_NEXT(it1, i, elt1, isnull1, typlen, typbyval, typalign);
- ARRAY_ITER_NEXT(it2, i, elt2, isnull2, typlen, typbyval, typalign);
+ elt1 = array_iter_next(&it1, i, &isnull1,
+ typlen, typbyval, typalign);
+ elt2 = array_iter_next(&it2, i, &isnull2,
+ typlen, typbyval, typalign);
/*
* We consider two NULLs equal; NULL and not-NULL are unequal.
@@ -3741,8 +3745,8 @@ array_cmp(FunctionCallInfo fcinfo)
bool typbyval;
char typalign;
int min_nitems;
- ARRAY_ITER ARRAY_ITER_VARS(it1);
- ARRAY_ITER ARRAY_ITER_VARS(it2);
+ array_iter it1;
+ array_iter it2;
int i;
FunctionCallInfoData locfcinfo;
@@ -3782,8 +3786,8 @@ array_cmp(FunctionCallInfo fcinfo)
/* Loop over source data */
min_nitems = Min(nitems1, nitems2);
- ARRAY_ITER_SETUP(it1, array1);
- ARRAY_ITER_SETUP(it2, array2);
+ array_iter_setup(&it1, array1);
+ array_iter_setup(&it2, array2);
for (i = 0; i < min_nitems; i++)
{
@@ -3794,8 +3798,8 @@ array_cmp(FunctionCallInfo fcinfo)
int32 cmpresult;
/* Get elements, checking for NULL */
- ARRAY_ITER_NEXT(it1, i, elt1, isnull1, typlen, typbyval, typalign);
- ARRAY_ITER_NEXT(it2, i, elt2, isnull2, typlen, typbyval, typalign);
+ elt1 = array_iter_next(&it1, i, &isnull1, typlen, typbyval, typalign);
+ elt2 = array_iter_next(&it2, i, &isnull2, typlen, typbyval, typalign);
/*
* We consider two NULLs equal; NULL > not-NULL.
@@ -3907,7 +3911,7 @@ hash_array(PG_FUNCTION_ARGS)
bool typbyval;
char typalign;
int i;
- ARRAY_ITER ARRAY_ITER_VARS(iter);
+ array_iter iter;
FunctionCallInfoData locfcinfo;
/*
@@ -3941,7 +3945,7 @@ hash_array(PG_FUNCTION_ARGS)
/* Loop over source data */
nitems = ArrayGetNItems(ndims, dims);
- ARRAY_ITER_SETUP(iter, array);
+ array_iter_setup(&iter, array);
for (i = 0; i < nitems; i++)
{
@@ -3950,7 +3954,7 @@ hash_array(PG_FUNCTION_ARGS)
uint32 elthash;
/* Get element, checking for NULL */
- ARRAY_ITER_NEXT(iter, i, elt, isnull, typlen, typbyval, typalign);
+ elt = array_iter_next(&iter, i, &isnull, typlen, typbyval, typalign);
if (isnull)
{
@@ -4017,7 +4021,7 @@ array_contain_compare(AnyArrayType *array1, AnyArrayType *array2, Oid collation,
char typalign;
int i;
int j;
- ARRAY_ITER ARRAY_ITER_VARS(it1);
+ array_iter it1;
FunctionCallInfoData locfcinfo;
if (element_type != AARR_ELEMTYPE(array2))
@@ -4074,7 +4078,7 @@ array_contain_compare(AnyArrayType *array1, AnyArrayType *array2, Oid collation,
/* Loop over source data */
nelems1 = ArrayGetNItems(AARR_NDIM(array1), AARR_DIMS(array1));
- ARRAY_ITER_SETUP(it1, array1);
+ array_iter_setup(&it1, array1);
for (i = 0; i < nelems1; i++)
{
@@ -4082,7 +4086,7 @@ array_contain_compare(AnyArrayType *array1, AnyArrayType *array2, Oid collation,
bool isnull1;
/* Get element, checking for NULL */
- ARRAY_ITER_NEXT(it1, i, elt1, isnull1, typlen, typbyval, typalign);
+ elt1 = array_iter_next(&it1, i, &isnull1, typlen, typbyval, typalign);
/*
* We assume that the comparison operator is strict, so a NULL can't
@@ -5869,7 +5873,7 @@ array_unnest(PG_FUNCTION_ARGS)
{
typedef struct
{
- ARRAY_ITER ARRAY_ITER_VARS(iter);
+ array_iter iter;
int nextelem;
int numelems;
int16 elmlen;
@@ -5907,7 +5911,7 @@ array_unnest(PG_FUNCTION_ARGS)
fctx = (array_unnest_fctx *) palloc(sizeof(array_unnest_fctx));
/* initialize state */
- ARRAY_ITER_SETUP(fctx->iter, arr);
+ array_iter_setup(&fctx->iter, arr);
fctx->nextelem = 0;
fctx->numelems = ArrayGetNItems(AARR_NDIM(arr), AARR_DIMS(arr));
@@ -5937,8 +5941,8 @@ array_unnest(PG_FUNCTION_ARGS)
int offset = fctx->nextelem++;
Datum elem;
- ARRAY_ITER_NEXT(fctx->iter, offset, elem, fcinfo->isnull,
- fctx->elmlen, fctx->elmbyval, fctx->elmalign);
+ elem = array_iter_next(&fctx->iter, offset, &fcinfo->isnull,
+ fctx->elmlen, fctx->elmbyval, fctx->elmalign);
SRF_RETURN_NEXT(funcctx, elem);
}
diff --git a/src/include/c.h b/src/include/c.h
index e63fd2f..7b13d09 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -661,6 +661,8 @@ typedef NameData *Name;
#define AssertArg(condition) assert(condition)
#define AssertState(condition) assert(condition)
#define AssertPointerAlignment(ptr, bndr) ((void)true)
+#define Trap(condition, errorType) ((void)true)
+#define TrapMacro(condition, errorType) (true)
#else /* USE_ASSERT_CHECKING && !FRONTEND */
/*
diff --git a/src/include/utils/array.h b/src/include/utils/array.h
index f76443d..d89962c 100644
--- a/src/include/utils/array.h
+++ b/src/include/utils/array.h
@@ -62,6 +62,7 @@
#define ARRAY_H
#include "fmgr.h"
+#include "access/tupmacs.h"
#include "utils/expandeddatum.h"
@@ -318,7 +319,6 @@ typedef struct ArrayIteratorData *ArrayIterator;
(VARATT_IS_EXPANDED_HEADER(a) ? (a)->xpn.dims : ARR_DIMS(&(a)->flt))
#define AARR_LBOUND(a) \
(VARATT_IS_EXPANDED_HEADER(a) ? (a)->xpn.lbound : ARR_LBOUND(&(a)->flt))
-
/*
* Macros for iterating through elements of a flat or expanded array.
* Use "ARRAY_ITER ARRAY_ITER_VARS(name);" to declare the local variables
@@ -330,6 +330,7 @@ typedef struct ArrayIteratorData *ArrayIterator;
* element number; we make caller provide this since caller is generally
* counting the elements anyway.
*/
+
#define ARRAY_ITER /* dummy type name to keep pgindent happy */
#define ARRAY_ITER_VARS(iter) \
@@ -399,6 +400,81 @@ typedef struct ArrayIteratorData *ArrayIterator;
} \
} while (0)
+typedef struct array_iter
+{
+ Datum *datumptr;
+ bool *isnullptr;
+ char *dataptr;
+ bits8 *bitmapptr;
+ int bitmask;
+} array_iter;
+
+static inline void
+array_iter_setup(array_iter *it, AnyArrayType *a)
+{
+ if (VARATT_IS_EXPANDED_HEADER(a))
+ {
+ if (a->xpn.dvalues)
+ {
+ it->datumptr = a->xpn.dvalues;
+ it->isnullptr = a->xpn.dnulls;
+ it->dataptr = NULL;
+ it->bitmapptr = NULL;
+ }
+ else
+ {
+ it->datumptr = NULL;
+ it->isnullptr = NULL;
+ it->dataptr = ARR_DATA_PTR(a->xpn.fvalue);
+ it->bitmapptr = ARR_NULLBITMAP(a->xpn.fvalue);
+ }
+ }
+ else
+ {
+ it->datumptr = NULL;
+ it->isnullptr = NULL;
+ it->dataptr = ARR_DATA_PTR(&a->flt);
+ it->bitmapptr = ARR_NULLBITMAP(&a->flt);
+ }
+ it->bitmask = 1;
+}
+
+static inline Datum
+array_iter_next(array_iter *it, int i, bool *isnull,
+ int elmlen, bool elmbyval, char elmalign)
+{
+ Datum ret;
+
+ if (it->datumptr)
+ {
+ ret = it->datumptr[i];
+ *isnull = it->isnullptr ? it->isnullptr[i] : false;
+ }
+ else
+ {
+ if (it->bitmapptr && (*(it->bitmapptr) & it->bitmask) == 0)
+ {
+ *isnull = true;
+ ret = (Datum) 0;
+ }
+ else
+ {
+ *isnull = false;
+ ret = fetch_att(it->dataptr, elmbyval, elmlen);
+ it->dataptr = att_addlength_pointer(it->dataptr, elmlen, it->dataptr);
+ it->dataptr = (char *) att_align_nominal(it->dataptr, elmalign);
+ }
+ it->bitmask <<= 1;
+ if (it->bitmask == 0x100)
+ {
+ if (it->bitmapptr)
+ it->bitmapptr++;
+ it->bitmask = 1;
+ }
+ }
+
+ return ret;
+}
/*
* GUC parameter
--
2.4.0.rc2.1.g3d6bc9a
>From 100675cea6eef322ce1dffc3978bffd8222a8361 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 11 May 2015 02:54:00 +0200
Subject: [PATCH 2/2] WIP-AARR-inlining
---
src/include/utils/array.h | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/include/utils/array.h b/src/include/utils/array.h
index d89962c..764ac86 100644
--- a/src/include/utils/array.h
+++ b/src/include/utils/array.h
@@ -307,12 +307,28 @@ typedef struct ArrayIteratorData *ArrayIterator;
/*
* Macros for working with AnyArrayType inputs. Beware multiple references!
*/
-#define AARR_NDIM(a) \
- (VARATT_IS_EXPANDED_HEADER(a) ? (a)->xpn.ndims : ARR_NDIM(&(a)->flt))
-#define AARR_HASNULL(a) \
- (VARATT_IS_EXPANDED_HEADER(a) ? \
- ((a)->xpn.dvalues != NULL ? (a)->xpn.dnulls != NULL : ARR_HASNULL((a)->xpn.fvalue)) : \
- ARR_HASNULL(&(a)->flt))
+static inline int
+AARR_NDIM(AnyArrayType *a)
+{
+ if (VARATT_IS_EXPANDED_HEADER(a))
+ return a->xpn.ndims;
+ else
+ return ARR_NDIM(&(a)->flt);
+}
+
+static inline bool
+AARR_HASNULL(AnyArrayType *a)
+{
+ if (VARATT_IS_EXPANDED_HEADER(a))
+ {
+ if (a->xpn.dvalues != NULL)
+ return a->xpn.dnulls != NULL;
+ else
+ return ARR_HASNULL(a->xpn.fvalue);
+ }
+ else
+ return ARR_HASNULL(&(a)->flt);
+}
#define AARR_ELEMTYPE(a) \
(VARATT_IS_EXPANDED_HEADER(a) ? (a)->xpn.element_type : ARR_ELEMTYPE(&(a)->flt))
#define AARR_DIMS(a) \
--
2.4.0.rc2.1.g3d6bc9a
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
