From 356687bd825e5ca7230d43c1bffe7a59ad2e77bd Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sat, 9 Feb 2019 00:35:57 -0800 Subject: Reset, not recreate, execGrouping.c style hashtables. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the facility added in the preceding commit to fix performance issues caused by rebuilding the hashtable (with its comparator expression being the most expensive bit), after every reset. That's especially important when the comparator is JIT compiled. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5 --- src/backend/executor/nodeAgg.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) (limited to 'src/backend/executor/nodeAgg.c') diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 263a0f81279..bae7989a422 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1246,7 +1246,7 @@ find_unaggregated_cols_walker(Node *node, Bitmapset **colnos) } /* - * Initialize the hash table(s) to empty. + * (Re-)initialize the hash table(s) to empty. * * To implement hashed aggregation, we need a hashtable that stores a * representative tuple and an array of AggStatePerGroup structs for each @@ -1257,9 +1257,9 @@ find_unaggregated_cols_walker(Node *node, Bitmapset **colnos) * We have a separate hashtable and associated perhash data structure for each * grouping set for which we're doing hashing. * - * The hash tables always live in the hashcontext's per-tuple memory context - * (there is only one of these for all tables together, since they are all - * reset at the same time). + * The contents of the hash tables always live in the hashcontext's per-tuple + * memory context (there is only one of these for all tables together, since + * they are all reset at the same time). */ static void build_hash_table(AggState *aggstate) @@ -1278,17 +1278,21 @@ build_hash_table(AggState *aggstate) Assert(perhash->aggnode->numGroups > 0); - perhash->hashtable = BuildTupleHashTable(&aggstate->ss.ps, - perhash->hashslot->tts_tupleDescriptor, - perhash->numCols, - perhash->hashGrpColIdxHash, - perhash->eqfuncoids, - perhash->hashfunctions, - perhash->aggnode->numGroups, - additionalsize, - aggstate->hashcontext->ecxt_per_tuple_memory, - tmpmem, - DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit)); + if (perhash->hashtable) + ResetTupleHashTable(perhash->hashtable); + else + perhash->hashtable = BuildTupleHashTableExt(&aggstate->ss.ps, + perhash->hashslot->tts_tupleDescriptor, + perhash->numCols, + perhash->hashGrpColIdxHash, + perhash->eqfuncoids, + perhash->hashfunctions, + perhash->aggnode->numGroups, + additionalsize, + aggstate->ss.ps.state->es_query_cxt, + aggstate->hashcontext->ecxt_per_tuple_memory, + tmpmem, + DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit)); } } -- cgit v1.2.3