From 5fc7b472ca6e335a83fab81ca767d074279ff969 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 27 Sep 2019 00:17:38 -0700 Subject: [PATCH v1 10/12] jit: Fix pessimization of execGrouping.c comparisons. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/executor/execGrouping.c | 13 ++++++++++++- src/test/regress/expected/jit.out | 8 +++----- src/test/regress/sql/jit.sql | 2 -- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index 14ee8db3f98..6349c11e1d5 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -166,6 +166,7 @@ BuildTupleHashTableExt(PlanState *parent, TupleHashTable hashtable; Size entrysize = sizeof(TupleHashEntryData) + additionalsize; MemoryContext oldcontext; + bool allow_jit; Assert(nbuckets > 0); @@ -210,13 +211,23 @@ BuildTupleHashTableExt(PlanState *parent, hashtable->tableslot = MakeSingleTupleTableSlot(CreateTupleDescCopy(inputDesc), &TTSOpsMinimalTuple); + /* + * If the old reset interface is used (i.e. BuildTupleHashTable, rather + * than BuildTupleHashTableExt), allowing JIT would lead to the generated + * functions to a) live longer than the query b) be re-generated each time + * the table is being reset. Therefore prevent JIT from being used in that + * case, by not providing a parent node (which prevents accessing the + * JitContext in the EState). + */ + allow_jit = metacxt != tablecxt; + /* build comparator for all columns */ /* XXX: should we support non-minimal tuples for the inputslot? */ hashtable->tab_eq_func = ExecBuildGroupingEqual(inputDesc, inputDesc, &TTSOpsMinimalTuple, &TTSOpsMinimalTuple, numCols, keyColIdx, eqfuncoids, collations, - NULL); + allow_jit ? parent : NULL); /* * While not pretty, it's ok to not shut down this context, but instead diff --git a/src/test/regress/expected/jit.out b/src/test/regress/expected/jit.out index 4db4ae6d352..151faaa2fde 100644 --- a/src/test/regress/expected/jit.out +++ b/src/test/regress/expected/jit.out @@ -418,8 +418,6 @@ SELECT count(*), count(data), string_agg(data, ':') FROM jittest_simple; -- Check that the equality hash-table function in a hash-aggregate can -- be accelerated. --- --- XXX: Unfortunately this is currently broken BEGIN; SET LOCAL enable_hashagg = true; SET LOCAL enable_sort = false; @@ -429,12 +427,12 @@ EXPLAIN (VERBOSE, COSTS OFF, JIT_DETAILS) SELECT data, string_agg(id::text, ', ' HashAggregate Project: data, string_agg((id)::text, ', '::text); JIT-Expr: evalexpr_0_0, JIT-Deform-Outer: deform_0_1 Phase 0 using strategy "Hash": - Transition Function: string_agg_transfn(TRANS, (id)::text, ', '::text); JIT-Expr: evalexpr_0_2, JIT-Deform-Outer: deform_0_3 - Hash Group: jittest_simple.data; JIT-Expr: false + Transition Function: string_agg_transfn(TRANS, (id)::text, ', '::text); JIT-Expr: evalexpr_0_5, JIT-Deform-Outer: deform_0_6 + Hash Group: jittest_simple.data; JIT-Expr: evalexpr_0_2, JIT-Deform-Outer: deform_0_4, JIT-Deform-Inner: deform_0_3 -> Seq Scan on public.jittest_simple Output: id, data JIT: - Functions: 4 (2 for expression evaluation, 2 for tuple deforming) + Functions: 7 (3 for expression evaluation, 4 for tuple deforming) Options: Inlining false, Optimization false, Expressions true, Deforming true (10 rows) diff --git a/src/test/regress/sql/jit.sql b/src/test/regress/sql/jit.sql index f3b9a352cf1..eb617c0ca58 100644 --- a/src/test/regress/sql/jit.sql +++ b/src/test/regress/sql/jit.sql @@ -144,8 +144,6 @@ SELECT count(*), count(data), string_agg(data, ':') FROM jittest_simple; -- Check that the equality hash-table function in a hash-aggregate can -- be accelerated. --- --- XXX: Unfortunately this is currently broken BEGIN; SET LOCAL enable_hashagg = true; SET LOCAL enable_sort = false; -- 2.23.0.162.gf1d4a28250