public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v1 05/12] jit: explain: remove backend lifetime module count from function name.
2+ messages / 2 participants
[nested] [flat]
* [PATCH v1 05/12] jit: explain: remove backend lifetime module count from function name.
@ 2019-09-26 21:05 Andres Freund <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Andres Freund @ 2019-09-26 21:05 UTC (permalink / raw)
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"
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Add Pipelining support in psql
@ 2025-03-04 23:25 Michael Paquier <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Michael Paquier @ 2025-03-04 23:25 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Mar 04, 2025 at 06:37:09PM +0100, Anthonin Bonnefoy wrote:
> I do see the idea to make it easier to convert existing scripts into
> using pipelining. The main focus of the initial implementation was
> more on protocol regression tests with psql, so that's not necessarily
> something I had in mind. I have some reservation as it will push all
> parameters in the query string which may not be the desired behaviour.
> But on the other hand, if it is to convert existing psql scripts, then
> everything was already pushed as simple queries. Plus, this is similar
> to what pgbench is doing when using -Mextended or -Mprepared.
Hmm. Simplicity is tempting here because we know the status of the
pipeline when sending the query. If we do something like what you are
suggesting here, do we actually need the \sendpipeline at all? We
should still prevent \g, \gx and others from running in a pipeline
because of the format argument raised by Daniel so as we restrict the
use of meta-commands that can manipulate the output format, right?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2025-03-04 23:25 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 21:05 [PATCH v1 05/12] jit: explain: remove backend lifetime module count from function name. Andres Freund <[email protected]>
2025-03-04 23:25 Re: Add Pipelining support in psql Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox