Subject: [PATCH v1 05/12] jit: explain: remove backend lifetime module count from function name.

From: Andres Freund <[email-masked]>
Date: Thu, 26 Sep 2019 14:05:08 -0700

Also expand function name to include in which module the function is -
without that it's harder to analyze which functions were emitted
separately (a performance concern).

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/commands/explain.c | 65 +++++++++++++++++++++++++++++-----
 src/backend/jit/llvm/llvmjit.c | 18 +++++++---
 src/include/jit/llvmjit.h      |  5 ++-
 3 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 3ccb76bdfd1..02455865d9f 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2228,6 +2228,43 @@ show_expression(Node *node, ExprState *expr, const char *qlabel,
 	}
 }
 
+/*
+ * To make JIT explain output reproducible, remove the module generation from
+ * function names. That makes it a bit harder to correlate with profiles etc,
+ * but reproducability is more important.
+ */
+static char *
+jit_funcname_for_display(const char *funcname)
+{
+	int			func_counter; /* nth function in query */
+	size_t		mod_num; /* nth module in query */
+	size_t		mod_generation; /* nth module in backend */
+	int			basename_end;
+	int			matchcount = 0;
+
+	/*
+	 * The pattern we need to match, see llvm_expand_funcname, is
+	 * "%s_%zu_%d_mod_%zu". Find the fourth _ from the end, so a _ in the name
+	 * is OK.
+	 */
+	for (basename_end = strlen(funcname); basename_end >= 0; basename_end--)
+	{
+		if (funcname[basename_end] == '_' && ++matchcount == 4)
+			break;
+	}
+
+	/* couldn't parse, bail out */
+	if (matchcount != 4)
+		return pstrdup(funcname);
+
+	/* couldn't parse, bail out */
+	if (sscanf(funcname + basename_end, "_%zu_%d_mod_%zu",
+			   &mod_num, &func_counter, &mod_generation) != 3)
+		return pstrdup(funcname);
+
+	return psprintf("%s_%zu_%d", pnstrdup(funcname, basename_end), mod_num, func_counter);
+}
+
 static void
 show_jit_expr_details(ExprState *expr, ExplainState *es)
 {
@@ -2239,7 +2276,8 @@ show_jit_expr_details(ExprState *expr, ExplainState *es)
 	if (es->format == EXPLAIN_FORMAT_TEXT)
 	{
 		if (expr->flags & EEO_FLAG_JIT_EXPR)
-			appendStringInfo(es->str, "JIT-Expr: %s", expr->expr_funcname);
+			appendStringInfo(es->str, "JIT-Expr: %s",
+							 jit_funcname_for_display(expr->expr_funcname));
 		else
 			appendStringInfoString(es->str, "JIT-Expr: false");
 
@@ -2250,19 +2288,22 @@ show_jit_expr_details(ExprState *expr, ExplainState *es)
 		 */
 
 		if (expr->scan_funcname)
-			appendStringInfo(es->str, ", JIT-Deform-Scan: %s", expr->scan_funcname);
+			appendStringInfo(es->str, ", JIT-Deform-Scan: %s",
+							 jit_funcname_for_display(expr->scan_funcname));
 		else if (expr->flags & EEO_FLAG_JIT_EXPR &&
 				 expr->flags & EEO_FLAG_DEFORM_SCAN)
 			appendStringInfo(es->str, ", JIT-Deform-Scan: false");
 
 		if (expr->outer_funcname)
-			appendStringInfo(es->str, ", JIT-Deform-Outer: %s", expr->outer_funcname);
+			appendStringInfo(es->str, ", JIT-Deform-Outer: %s",
+							 jit_funcname_for_display(expr->outer_funcname));
 		else if (expr->flags & EEO_FLAG_JIT_EXPR &&
 				 expr->flags & EEO_FLAG_DEFORM_OUTER)
 			appendStringInfo(es->str, ", JIT-Deform-Outer: false");
 
 		if (expr->inner_funcname)
-			appendStringInfo(es->str, ", JIT-Deform-Inner: %s", expr->inner_funcname);
+			appendStringInfo(es->str, ", JIT-Deform-Inner: %s",
+							 jit_funcname_for_display(expr->inner_funcname));
 		else if (expr->flags & EEO_FLAG_JIT_EXPR &&
 				 expr->flags & (EEO_FLAG_DEFORM_INNER))
 			appendStringInfo(es->str, ", JIT-Deform-Inner: false");
@@ -2270,26 +2311,34 @@ show_jit_expr_details(ExprState *expr, ExplainState *es)
 	else
 	{
 		if (expr->flags & EEO_FLAG_JIT_EXPR)
-			ExplainPropertyText("JIT-Expr", expr->expr_funcname, es);
+			ExplainPropertyText("JIT-Expr",
+								jit_funcname_for_display(expr->expr_funcname),
+								es);
 		else
 			ExplainPropertyBool("JIT-Expr", false, es);
 
 		if (expr->scan_funcname)
-			ExplainProperty("JIT-Deform-Scan", NULL, expr->scan_funcname, false, es);
+			ExplainProperty("JIT-Deform-Scan", NULL,
+							jit_funcname_for_display(expr->scan_funcname),
+							false, es);
 		else if (expr->flags & EEO_FLAG_DEFORM_SCAN)
 			ExplainProperty("JIT-Deform-Scan", NULL, "false", true, es);
 		else
 			ExplainProperty("JIT-Deform-Scan", NULL, "null", true, es);
 
 		if (expr->outer_funcname)
-			ExplainProperty("JIT-Deform-Outer", NULL, expr->outer_funcname, false, es);
+			ExplainProperty("JIT-Deform-Outer", NULL,
+							jit_funcname_for_display(expr->outer_funcname),
+							false, es);
 		else if (expr->flags & EEO_FLAG_DEFORM_OUTER)
 			ExplainProperty("JIT-Deform-Outer", NULL, "false", true, es);
 		else
 			ExplainProperty("JIT-Deform-Outer", NULL, "null", true, es);
 
 		if (expr->inner_funcname)
-			ExplainProperty("JIT-Deform-Inner", NULL, expr->inner_funcname, false, es);
+			ExplainProperty("JIT-Deform-Inner", NULL,
+							jit_funcname_for_display(expr->inner_funcname),
+							false, es);
 		else if (expr->flags & EEO_FLAG_DEFORM_INNER)
 			ExplainProperty("JIT-Deform-Inner", NULL, "false", true, es);
 		else
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 5489e118041..177a00f3826 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -227,6 +227,8 @@ llvm_mutable_module(LLVMJitContext *context)
 char *
 llvm_expand_funcname(struct LLVMJitContext *context, const char *basename)
 {
+	char *funcname;
+
 	Assert(context->module != NULL);
 
 	context->base.instr.created_functions++;
@@ -234,11 +236,19 @@ llvm_expand_funcname(struct LLVMJitContext *context, const char *basename)
 	/*
 	 * Previously we used dots to separate, but turns out some tools, e.g.
 	 * GDB, don't like that and truncate name.
+	 *
+	 * Append the backend-lifetime module count to the end, so it's easier for
+	 * humans and machines to compare the generated function names across
+	 * queries, the prefix will be the same from query execution to query
+	 * execution.
 	 */
-	return psprintf("%s_%zu_%d",
-					basename,
-					context->module_generation,
-					context->counter++);
+	funcname = psprintf("%s_%zu_%d_mod_%zu",
+					  basename,
+					  context->base.instr.created_modules - 1,
+					  context->counter++,
+					  context->module_generation);
+
+	return funcname;
 }
 
 /*
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 6178864b2e6..e45ff99194f 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -41,7 +41,10 @@ typedef struct LLVMJitContext
 {
 	JitContext	base;
 
-	/* number of modules created */
+	/*
+	 * llvm_generation when ->module was created, monotonically increasing
+	 * within the lifetime of a backend.
+	 */
 	size_t		module_generation;
 
 	/* current, "open for write", module */
-- 
2.23.0.162.gf1d4a28250

--kgppz6muakthis74
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0006-WIP-explain-Show-per-phase-information-about-aggr.patch"