Change function call information to be variable length.

Before this change FunctionCallInfoData, the struct arguments etc for
V1 function calls are stored in, always had space for
FUNC_MAX_ARGS/100 arguments, storing datums and their nullness in two
arrays.  For nearly every function call 100 arguments is far more than
needed, therefore wasting memory. Arg and argnull being two separate
arrays also guarantees that to access a single argument, two
cachelines have to be touched.

Change the layout so there's a single variable-length array with pairs
of value / isnull. That drastically reduces memory consumption for
most function calls (on x86-64 a two argument function now uses
64bytes, previously 936 bytes), and makes it very likely that argument
value and its nullness are on the same cacheline.

Arguments are stored in a new NullableDatum struct, which, due to
padding, needs more memory per argument than before. But as usually
far fewer arguments are stored, and individual arguments are cheaper
to access, that's still a clear win.  It's likely that there's other
places where conversion to NullableDatum arrays would make sense,
e.g. TupleTableSlots, but that's for another commit.

Because the function call information is now variable-length
allocations have to take the number of arguments into account. For
heap allocations that can be done with SizeForFunctionCallInfoData(),
for on-stack allocations there's a new LOCAL_FCINFO(name, nargs) macro
that helps to allocate an appropriately sized and aligned variable.

Some places with stack allocation function call information don't know
the number of arguments at compile time, and currently variably sized
stack allocations aren't allowed in postgres. Therefore allow for
FUNC_MAX_ARGS space in these cases. They're not that common, so for
now that seems acceptable.

Because of the need to allocate FunctionCallInfo of the appropriate
size, older extensions may need to update their code. To avoid subtle
breakages, the FunctionCallInfoData struct has been renamed to
FunctionCallInfoBaseData. Most code only references FunctionCallInfo,
so that shouldn't cause much collateral damage.

This change is also a prerequisite for more efficient expression JIT
compilation (by allocating the function call information on the stack,
allowing LLVM to optimize it away); previously the size of the call
information caused problems inside LLVM's optimizer.

Author: Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/20180605172952.x34m5uz6ju6enaem@alap3.anarazel.de

8 files changed, 136 insertions, 26 deletions

diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index b875bbad4fb..2c1697dd766 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -223,7 +223,8 @@ typedef enum ExprEvalOp
 	/* aggregation related nodes */
 	EEOP_AGG_STRICT_DESERIALIZE,
 	EEOP_AGG_DESERIALIZE,
-	EEOP_AGG_STRICT_INPUT_CHECK,
+	EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
+	EEOP_AGG_STRICT_INPUT_CHECK_NULLS,
 	EEOP_AGG_INIT_TRANS,
 	EEOP_AGG_STRICT_TRANS_CHECK,
 	EEOP_AGG_PLAIN_TRANS_BYVAL,
@@ -601,9 +602,21 @@ typedef struct ExprEvalStep
 			int			jumpnull;
 		}			agg_deserialize;
 
-		/* for EEOP_AGG_STRICT_INPUT_CHECK */
-		struct
-		{
+		/* for EEOP_AGG_STRICT_INPUT_CHECK_NULLS / STRICT_INPUT_CHECK_ARGS */
+		struct
+		{
+			/*
+			 * For EEOP_AGG_STRICT_INPUT_CHECK_ARGS args contains pointers to
+			 * the NullableDatums that need to be checked for NULLs.
+			 *
+			 * For EEOP_AGG_STRICT_INPUT_CHECK_NULLS nulls contains pointers
+			 * to booleans that need to be checked for NULLs.
+			 *
+			 * Both cases currently need to exist because sometimes the
+			 * to-be-checked nulls are in TupleTableSlot.isnull array, and
+			 * sometimes in FunctionCallInfoBaseData.args[i].isnull.
+			 */
+			NullableDatum *args;
 			bool	   *nulls;
 			int			nargs;
 			int			jumpnull;
diff --git a/src/include/executor/nodeAgg.h b/src/include/executor/nodeAgg.h
index 3d93439e5fc..30541361301 100644
--- a/src/include/executor/nodeAgg.h
+++ b/src/include/executor/nodeAgg.h
@@ -158,12 +158,12 @@ typedef struct AggStatePerTransData
 	 * re-initializing the unchanging fields; which isn't much, but it seems
 	 * worth the extra space consumption.
 	 */
-	FunctionCallInfoData transfn_fcinfo;
+	FunctionCallInfo transfn_fcinfo;
 
 	/* Likewise for serialization and deserialization functions */
-	FunctionCallInfoData serialfn_fcinfo;
+	FunctionCallInfo serialfn_fcinfo;
 
-	FunctionCallInfoData deserialfn_fcinfo;
+	FunctionCallInfo deserialfn_fcinfo;
 }			AggStatePerTransData;
 
 /*
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index ead17f0e442..ce652845253 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -35,7 +35,7 @@ typedef struct StringInfoData *fmStringInfo;
  * signature.)
  */
 
-typedef struct FunctionCallInfoData *FunctionCallInfo;
+typedef struct FunctionCallInfoBaseData *FunctionCallInfo;
 
 typedef Datum (*PGFunction) (FunctionCallInfo fcinfo);
 
@@ -46,8 +46,8 @@ typedef Datum (*PGFunction) (FunctionCallInfo fcinfo);
  * info struct saved for re-use.
  *
  * Note that fn_expr really is parse-time-determined information about the
- * arguments, rather than about the function itself.  But it's convenient
- * to store it here rather than in FunctionCallInfoData, where it might more
+ * arguments, rather than about the function itself.  But it's convenient to
+ * store it here rather than in FunctionCallInfoBaseData, where it might more
  * logically belong.
  *
  * fn_extra is available for use by the called function; all other fields
@@ -73,8 +73,16 @@ typedef struct FmgrInfo
  * fields in whatever resultinfo points to.  It should not change any other
  * fields.  (In particular, scribbling on the argument arrays is a bad idea,
  * since some callers assume they can re-call with the same arguments.)
+ *
+ * Note that enough space for arguments needs to be provided, either by using
+ * SizeForFunctionCallInfo() in dynamic allocations, or by using
+ * LOCAL_FCINFO() for on-stack allocations.
+ *
+ * This struct is named *BaseData, rather than *Data, to break pre v12 code
+ * that allocated FunctionCallInfoData itself, as it'd often silently break
+ * old code due to no space for arguments being provided.
  */
-typedef struct FunctionCallInfoData
+typedef struct FunctionCallInfoBaseData
 {
 	FmgrInfo   *flinfo;			/* ptr to lookup info used for this call */
 	fmNodePtr	context;		/* pass info about context of call */
@@ -83,11 +91,31 @@ typedef struct FunctionCallInfoData
 #define FIELDNO_FUNCTIONCALLINFODATA_ISNULL 4
 	bool		isnull;			/* function must set true if result is NULL */
 	short		nargs;			/* # arguments actually passed */
-#define FIELDNO_FUNCTIONCALLINFODATA_ARG 6
-	Datum		arg[FUNC_MAX_ARGS]; /* Arguments passed to function */
-#define FIELDNO_FUNCTIONCALLINFODATA_ARGNULL 7
-	bool		argnull[FUNC_MAX_ARGS]; /* T if arg[i] is actually NULL */
-} FunctionCallInfoData;
+#define FIELDNO_FUNCTIONCALLINFODATA_ARGS 6
+	NullableDatum args[FLEXIBLE_ARRAY_MEMBER];
+} FunctionCallInfoBaseData;
+
+/*
+ * Space needed for for a FunctionCallInfoBaseData struct with sufficient space
+ * for `nargs` arguments.
+ */
+#define SizeForFunctionCallInfo(nargs) \
+	(offsetof(FunctionCallInfoBaseData, args) + \
+	 sizeof(NullableDatum) * (nargs))
+
+/*
+ * This macro ensures that `name` points to a stack-allocated
+ * FunctionCallInfoBaseData struct with sufficient space for `nargs` arguments.
+ */
+#define LOCAL_FCINFO(name, nargs) \
+	/* use union with FunctionCallInfoBaseData to guarantee alignment */ \
+	union \
+	{ \
+		FunctionCallInfoBaseData fcinfo; \
+		/* ensure enough space for nargs args is available */ \
+		char fcinfo_data[SizeForFunctionCallInfo(nargs)]; \
+	} name##data; \
+	FunctionCallInfo name = &name##data.fcinfo
 
 /*
  * This routine fills a FmgrInfo struct, given the OID
@@ -116,11 +144,8 @@ extern void fmgr_info_copy(FmgrInfo *dstinfo, FmgrInfo *srcinfo,
 extern void fmgr_symbol(Oid functionId, char **mod, char **fn);
 
 /*
- * This macro initializes all the fields of a FunctionCallInfoData except
- * for the arg[] and argnull[] arrays.  Performance testing has shown that
- * the fastest way to set up argnull[] for small numbers of arguments is to
- * explicitly set each required element to false, so we don't try to zero
- * out the argnull[] array in the macro.
+ * This macro initializes all the fields of a FunctionCallInfoBaseData except
+ * for the args[] array.
  */
 #define InitFunctionCallInfoData(Fcinfo, Flinfo, Nargs, Collation, Context, Resultinfo) \
 	do { \
@@ -133,7 +158,7 @@ extern void fmgr_symbol(Oid functionId, char **mod, char **fn);
 	} while (0)
 
 /*
- * This macro invokes a function given a filled-in FunctionCallInfoData
+ * This macro invokes a function given a filled-in FunctionCallInfoBaseData
  * struct.  The macro result is the returned Datum --- but note that
  * caller must still check fcinfo->isnull!	Also, if function is strict,
  * it is caller's responsibility to verify that no null arguments are present
@@ -176,7 +201,7 @@ extern void fmgr_symbol(Oid functionId, char **mod, char **fn);
  * If function is not marked "proisstrict" in pg_proc, it must check for
  * null arguments using this macro.  Do not try to GETARG a null argument!
  */
-#define PG_ARGISNULL(n)  (fcinfo->argnull[n])
+#define PG_ARGISNULL(n)  (fcinfo->args[n].isnull)
 
 /*
  * Support for fetching detoasted copies of toastable datatypes (all of
@@ -235,7 +260,7 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena *datum);
 
 /* Macros for fetching arguments of standard types */
 
-#define PG_GETARG_DATUM(n)	 (fcinfo->arg[n])
+#define PG_GETARG_DATUM(n)	 (fcinfo->args[n].value)
 #define PG_GETARG_INT32(n)	 DatumGetInt32(PG_GETARG_DATUM(n))
 #define PG_GETARG_UINT32(n)  DatumGetUInt32(PG_GETARG_DATUM(n))
 #define PG_GETARG_INT16(n)	 DatumGetInt16(PG_GETARG_DATUM(n))
@@ -514,6 +539,7 @@ extern Datum CallerFInfoFunctionCall2(PGFunction func, FmgrInfo *flinfo,
  * directly-computed parameter list.  Note that neither arguments nor result
  * are allowed to be NULL.
  */
+extern Datum FunctionCall0Coll(FmgrInfo *flinfo, Oid collation);
 extern Datum FunctionCall1Coll(FmgrInfo *flinfo, Oid collation,
 				  Datum arg1);
 extern Datum FunctionCall2Coll(FmgrInfo *flinfo, Oid collation,
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 2545abeddd1..6af5fe74388 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -62,6 +62,7 @@ extern LLVMTypeRef TypePGFunction;
 extern LLVMTypeRef TypeSizeT;
 extern LLVMTypeRef TypeStorageBool;
 
+extern LLVMTypeRef StructNullableDatum;
 extern LLVMTypeRef StructTupleDescData;
 extern LLVMTypeRef StructHeapTupleData;
 extern LLVMTypeRef StructTupleTableSlot;
diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h
index 48a5c0f86cd..41145fc517a 100644
--- a/src/include/jit/llvmjit_emit.h
+++ b/src/include/jit/llvmjit_emit.h
@@ -208,4 +208,59 @@ l_mcxt_switch(LLVMModuleRef mod, LLVMBuilderRef b, LLVMValueRef nc)
 
 	return ret;
 }
+
+/*
+ * Return pointer to the the argno'th argument nullness.
+ */
+static inline LLVMValueRef
+l_funcnullp(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno)
+{
+	LLVMValueRef v_args;
+	LLVMValueRef v_argn;
+
+	v_args = LLVMBuildStructGEP(b,
+								v_fcinfo,
+								FIELDNO_FUNCTIONCALLINFODATA_ARGS,
+								"");
+	v_argn = LLVMBuildStructGEP(b, v_args, argno, "");
+
+	return LLVMBuildStructGEP(b, v_argn, FIELDNO_NULLABLE_DATUM_ISNULL, "");
+}
+
+/*
+ * Return pointer to the the argno'th argument datum.
+ */
+static inline LLVMValueRef
+l_funcvaluep(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno)
+{
+	LLVMValueRef v_args;
+	LLVMValueRef v_argn;
+
+	v_args = LLVMBuildStructGEP(b,
+								v_fcinfo,
+								FIELDNO_FUNCTIONCALLINFODATA_ARGS,
+								"");
+	v_argn = LLVMBuildStructGEP(b, v_args, argno, "");
+
+	return LLVMBuildStructGEP(b, v_argn, FIELDNO_NULLABLE_DATUM_DATUM, "");
+}
+
+/*
+ * Return argno'th argument nullness.
+ */
+static inline LLVMValueRef
+l_funcnull(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno)
+{
+	return LLVMBuildLoad(b, l_funcnullp(b, v_fcinfo, argno), "");
+}
+
+/*
+ * Return argno'th argument datum.
+ */
+static inline LLVMValueRef
+l_funcvalue(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno)
+{
+	return LLVMBuildLoad(b, l_funcvaluep(b, v_fcinfo, argno), "");
+}
+
 #endif
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 64e8ef37407..3b789ee7cf3 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -826,7 +826,7 @@ typedef struct SetExprState
 	 * (by InitFunctionCallInfoData) if func.fn_oid is valid.  It also saves
 	 * argument values between calls, when setArgsValid is true.
 	 */
-	FunctionCallInfoData fcinfo_data;
+	FunctionCallInfo fcinfo;
 } SetExprState;
 
 /* ----------------
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 313ca5f3c34..0ce79489dad 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -1321,7 +1321,7 @@ extern void pgstat_count_heap_delete(Relation rel);
 extern void pgstat_count_truncate(Relation rel);
 extern void pgstat_update_heap_dead_tuples(Relation rel, int delta);
 
-extern void pgstat_init_function_usage(FunctionCallInfoData *fcinfo,
+extern void pgstat_init_function_usage(FunctionCallInfo fcinfo,
 						   PgStat_FunctionCallUsage *fcu);
 extern void pgstat_end_function_usage(PgStat_FunctionCallUsage *fcu,
 						  bool finalize);
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 446bb44b711..057a3413acd 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -366,6 +366,21 @@ typedef struct
 
 typedef uintptr_t Datum;
 
+/*
+ * A NullableDatum is used in places where both a Datum and its nullness needs
+ * to be stored. This can be more efficient than storing datums and nullness
+ * in separate arrays, due to better spatial locality, even if more space may
+ * be wasted due to padding.
+ */
+typedef struct NullableDatum
+{
+#define FIELDNO_NULLABLE_DATUM_DATUM 0
+	Datum		value;
+#define FIELDNO_NULLABLE_DATUM_ISNULL 1
+	bool		isnull;
+	/* due to alignment padding this could be used for flags for free */
+} NullableDatum;
+
 #define SIZEOF_DATUM SIZEOF_VOID_P
 
 /*