]> granicus.if.org Git - postgresql/commitdiff
jit: Re-allow JIT compilation of execGrouping.c hashtable comparisons.
authorAndres Freund <andres@anarazel.de>
Sun, 29 Sep 2019 23:27:08 +0000 (16:27 -0700)
committerAndres Freund <andres@anarazel.de>
Sun, 29 Sep 2019 23:27:08 +0000 (16:27 -0700)
In the course of 5567d12ce03356687bd8 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

index c651084b5739033b4857980fe4f9e6b883ae2092..e6a876ded8bdce22f771de1ef3db53b93f66e5ed 100644 (file)
@@ -164,6 +164,7 @@ BuildTupleHashTableExt(PlanState *parent,
        TupleHashTable hashtable;
        Size            entrysize = sizeof(TupleHashEntryData) + additionalsize;
        MemoryContext oldcontext;
+       bool            allow_jit;
 
        Assert(nbuckets > 0);
 
@@ -206,11 +207,21 @@ BuildTupleHashTableExt(PlanState *parent,
         */
        hashtable->tableslot = MakeSingleTupleTableSlot(CreateTupleDescCopy(inputDesc));
 
+       /*
+        * 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 */
        hashtable->tab_eq_func = ExecBuildGroupingEqual(inputDesc, inputDesc,
                                                                                                        numCols,
                                                                                                        keyColIdx, eqfuncoids,
-                                                                                                       NULL);
+                                                                                                       allow_jit ? parent : NULL);
 
        /*
         * While not pretty, it's ok to not shut down this context, but instead