postgres.git / summary / log / commit / refs

commit    2ce9bca36458a1f1907c82eb2e9bd642cb1fca26
Author:   Andres Freund <andres@anarazel.de>
Date:     Sun Sep 29 23:27:18 2019 +0000
First released in: 13.0, 11.6, 12.0

    jit: Re-allow JIT compilation of execGrouping.c hashtable comparisons.
    
    In the course of 5567d12ce03, 356687bd8 and 317ffdfeaac, I changed
    BuildTupleHashTable[Ext]'s call to ExecBuildGroupingEqual to not pass
    in the parent node, but NULL. Which in turn prevents the tuple
    equality comparator from being JIT compiled.  While that fixes
    bug #15486, it is not actually necessary after all of the above commits,
    as we don't re-build the comparator when using the new
    BuildTupleHashTableExt() interface (as the content of the hashtable
    are reset, but the TupleHashTable itself is not).
    
    Therefore re-allow jit compilation for callers that use
    BuildTupleHashTableExt with a separate context for "metadata" and
    content.
    
    As in the previous commit, there's ongoing work to make this easier to
    test to prevent such regressions in the future, but that
    infrastructure is not going to be backpatchable.
    
    The performance impact of not JIT compiling hashtable equality
    comparators can be substantial e.g. for aggregation queries that
    aggregate a lot of input rows to few output rows (when there are a lot
    of output groups, there will be fewer comparisons).
    
    Author: Andres Freund
    Discussion: https://postgr.es/m/20190927072053.njf6prdl3vb7y7qb@alap3.anarazel.de
    Backpatch: 11, just as 5567d12ce03


 src/backend/executor/execGrouping.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

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


[parent: 6cf74f1bbd65]