Hi,
It seems quite a while since the last email, but I was taking a look at this
thread and I think that we can have another, but similar, approach for this
proposal.
Em ter., 14 de mai. de 2024 às 01:10, David Rowley
<dgrowle...@gmail.com> escreveu:
> Currently, during execution, ExecCreateExprSetupSteps() traverses the
> Node tree of the Expr to figure out the max varattno of for each slot.
> That's done so all of the tuple deforming happens at once rather than
> incrementally. Figuring out the max varattno is a price we have to pay
> for every execution of the query. I think we'd be better off doing
> that in the planner.
>
> To do this, I thought that setrefs.c could do this processing in
> fix_join_expr / fix_upper_expr and wrap up the expression in a new
> Node type that stores the max varattno for each special var type.
>
> This idea is related to this discussion because another thing that
> could be stored in the very same struct is the "num_exec" value. I
> feel the number of executions of an ExprState is a better gauge of how
> useful JIT will be than the cost of the plan node. Now, looking at
> set_join_references(), the execution estimates are not exactly
> perfect. For example;
>
The fix_join_expr and fix_upper_expr functions have been evolved that now both
receives a "num_exec" parameter which can be derived from plan->plan_rows via
NUM_EXEC_TLIST macro. After a plan is created on create_plan_recurse we have
both plan_rows and total_cost filled, so it seems to me that all information
needed to decide if a plan node should be compiled or not is already present on
Plan struct?
If that's the case, I was thinking, what if we add a new field jit on
Plan struct
and fill this value after a plan is created on create_plan_recurse? Similar
idea with the first patch:
static void
plan_consider_jit(Plan *plan)
{
plan->jit = false;
if (jit_enabled)
{
Cost total_cost;
total_cost = plan->total_cost * plan->plan_rows;
if (total_cost > jit_above_cost)
plan->jit = true;
}
}
And then when jit_compile_expr is called we can check if the jit is enabled for
ExprState->parent->plan node.
This make any kind of sense? I'm not sure but I've executed some benchmarks.
I've used the same test scenario used on [1], which is:
CREATE TABLE listp(a int, b int) PARTITION BY LIST(a);
SELECT 'CREATE TABLE listp'|| x || ' PARTITION OF listp FOR VALUES IN
('||x||');' FROM generate_Series(1,1000) x; \gexec
INSERT INTO listp SELECT 1,x FROM generate_series(1,10000000) x;
EXPLAIN (VERBOSE, ANALYZE) SELECT COUNT(*) FROM listp WHERE b < 0;
Results:
master jit=off:
Planning Time: 59.457
Execution Time: 457.000
master jit=on:
Planning Time: 59.529 ms
JIT:
Functions: 9008
Options: Inlining false, Optimization false, Expressions true, Deforming true
Timing: Generation 99.267 ms (Deform 37.434 ms), Inlining 0.000 ms,
Optimization 309.079 ms, Emission 517.495 ms, Total 925.841 ms
Execution Time: 674.756 ms
patch jit=off
Planning Time: 60.906 ms
Execution Time: 453.978 ms
patch jit=on:
Planning Time: 67.625 ms
JIT:
Functions: 17
Options: Inlining false, Optimization false, Expressions true, Deforming true
Timing: Generation 0.502 ms (Deform 0.073 ms), Inlining 0.000 ms,
Optimization 0.998 ms, Emission 3.915 ms, Total 5.415 ms
Execution Time: 328.239 ms
Note that I used the jit_above_cost default value for both tests.
I've executed the same EXPLAIN query 100 times on master and with the patch and
all results seem similar with the above.
jit=off
Master mean: 438.898
Patch mean: 436.036
jit=on
Master mean: 665.730
Patch mean: 347.758
I'm a bit concerned if it's a good idea to add the jit field on Plan node, but I
also I'm not sure if this approach makes sense. Attached a simple patch that
play with the idea.
WYT?
[1]
https://www.postgresql.org/message-id/cagpvpcsbn_-t3jvpmmhhsqns2nogo1tsbbyzng1cjggacqj...@mail.gmail.com
--
Matheus Alcantara
From 2577544cc8b93d9a08a872353e3d260c5999ccef Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Wed, 18 Dec 2024 16:54:01 -0300
Subject: [PATCH v1] JIT compilation per plan node
Introduce a new jit field on Planner struct to make it possible to
decide whether a plan node should be compiled or not.
Previously the cost of the entire query was used to decide whether it
should be compiled or not. This approach has a problem that it can exist
some expressions on plan tree that is not executed frequently, so it is
not worth to compile.
This commit change the consideration of JIT from plan level to
per-plan-node. The plan total cost and the number of times that the node
will be executed is used to decide if the plan node should be compiled
or not.
---
src/backend/jit/jit.c | 8 ++++++++
src/backend/optimizer/plan/createplan.c | 27 +++++++++++++++++++++++++
src/backend/optimizer/plan/planner.c | 3 +--
src/include/nodes/plannodes.h | 2 ++
4 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c
index 815b58f33c..dfe010392f 100644
--- a/src/backend/jit/jit.c
+++ b/src/backend/jit/jit.c
@@ -170,6 +170,14 @@ jit_compile_expr(struct ExprState *state)
if (!(state->parent->state->es_jit_flags & PGJIT_EXPR))
return false;
+ /* don't jit if the plan node is missing */
+ if (state->parent->plan == NULL)
+ return false;
+
+ /* don't jit if it's not enabled for this plan node */
+ if (!state->parent->plan->jit)
+ return false;
+
/* this also takes !jit_enabled into account */
if (provider_init())
return provider.compile_expr(state);
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 178c572b02..1e787c3437 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -25,6 +25,7 @@
#include "nodes/extensible.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+#include "jit/jit.h"
#include "optimizer/clauses.h"
#include "optimizer/cost.h"
#include "optimizer/optimizer.h"
@@ -320,6 +321,7 @@ static ModifyTable *make_modifytable(PlannerInfo *root, Plan *subplan,
static GatherMerge *create_gather_merge_plan(PlannerInfo *root,
GatherMergePath *best_path);
+static void plan_consider_jit(Plan *plan);
/*
* create_plan
@@ -551,9 +553,34 @@ create_plan_recurse(PlannerInfo *root, Path *best_path, int flags)
break;
}
+ plan_consider_jit(plan);
return plan;
}
+static void
+plan_consider_jit(Plan *plan)
+{
+ plan->jit = false;
+
+ /* Determine which JIT options to enable for this plan node */
+ if (jit_enabled)
+ {
+ Cost total_cost;
+
+ /*
+ * Take into account the number of times that we expect to rescan a
+ * given plan node. For example, subplans being invoked under the
+ * inside of a Nested Loop may be rescanned many times. JITing these
+ * may be more worthwhile.
+ */
+ total_cost = plan->total_cost * plan->plan_rows;
+
+ if (total_cost > jit_above_cost)
+ plan->jit = true;
+ }
+}
+
+
/*
* create_scan_plan
* Create a scan plan for the parent relation of 'best_path'.
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index f3856c519f..e0b0e2f1ae 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -571,8 +571,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
result->stmt_len = parse->stmt_len;
result->jitFlags = PGJIT_NONE;
- if (jit_enabled && jit_above_cost >= 0 &&
- top_plan->total_cost > jit_above_cost)
+ if (jit_enabled && jit_above_cost >= 0 && top_plan->jit)
{
result->jitFlags |= PGJIT_PERFORM;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 52f29bcdb6..c4c6cd057f 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -135,6 +135,8 @@ typedef struct Plan
Cardinality plan_rows; /* number of rows plan is expected to emit */
int plan_width; /* average row width in bytes */
+ bool jit; /* true if the plan node is worth to compile */
+
/*
* information needed for parallel query
*/
--
2.39.3 (Apple Git-146)
