Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 200f610

Browse filesBrowse files
jeff-davisjeff-davis
committed
Fix LookupTupleHashEntryHash() pipeline-stall issue.
Refactor hash lookups in nodeAgg.c to improve performance. Author: Andres Freund and Jeff Davis Discussion: https://postgr.es/m/20200612213715.op4ye4q7gktqvpuo%40alap3.anarazel.de Backpatch-through: 13
1 parent 56788d2 commit 200f610
Copy full SHA for 200f610

File tree

Expand file treeCollapse file tree

6 files changed

+105
-101
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+105
-101
lines changed

‎src/backend/executor/execGrouping.c

Copy file name to clipboardExpand all lines: src/backend/executor/execGrouping.c
+19-10Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
#include "utils/memutils.h"
2323

2424
static int TupleHashTableMatch(struct tuplehash_hash *tb, const MinimalTuple tuple1, const MinimalTuple tuple2);
25-
static uint32 TupleHashTableHash_internal(struct tuplehash_hash *tb,
26-
const MinimalTuple tuple);
27-
static TupleHashEntry LookupTupleHashEntry_internal(TupleHashTable hashtable,
28-
TupleTableSlot *slot,
29-
bool *isnew, uint32 hash);
25+
static inline uint32 TupleHashTableHash_internal(struct tuplehash_hash *tb,
26+
const MinimalTuple tuple);
27+
static inline TupleHashEntry LookupTupleHashEntry_internal(TupleHashTable hashtable,
28+
TupleTableSlot *slot,
29+
bool *isnew, uint32 hash);
3030

3131
/*
3232
* Define parameters for tuple hash table code generation. The interface is
@@ -291,18 +291,21 @@ ResetTupleHashTable(TupleHashTable hashtable)
291291
* If isnew is NULL, we do not create new entries; we return NULL if no
292292
* match is found.
293293
*
294+
* If hash is not NULL, we set it to the calculated hash value. This allows
295+
* callers access to the hash value even if no entry is returned.
296+
*
294297
* If isnew isn't NULL, then a new entry is created if no existing entry
295298
* matches. On return, *isnew is true if the entry is newly created,
296299
* false if it existed already. ->additional_data in the new entry has
297300
* been zeroed.
298301
*/
299302
TupleHashEntry
300303
LookupTupleHashEntry(TupleHashTable hashtable, TupleTableSlot *slot,
301-
bool *isnew)
304+
bool *isnew, uint32 *hash)
302305
{
303306
TupleHashEntry entry;
304307
MemoryContext oldContext;
305-
uint32 hash;
308+
uint32 local_hash;
306309

307310
/* Need to run the hash functions in short-lived context */
308311
oldContext = MemoryContextSwitchTo(hashtable->tempcxt);
@@ -312,8 +315,13 @@ LookupTupleHashEntry(TupleHashTable hashtable, TupleTableSlot *slot,
312315
hashtable->in_hash_funcs = hashtable->tab_hash_funcs;
313316
hashtable->cur_eq_func = hashtable->tab_eq_func;
314317

315-
hash = TupleHashTableHash_internal(hashtable->hashtab, NULL);
316-
entry = LookupTupleHashEntry_internal(hashtable, slot, isnew, hash);
318+
local_hash = TupleHashTableHash_internal(hashtable->hashtab, NULL);
319+
entry = LookupTupleHashEntry_internal(hashtable, slot, isnew, local_hash);
320+
321+
if (hash != NULL)
322+
*hash = local_hash;
323+
324+
Assert(entry == NULL || entry->hash == local_hash);
317325

318326
MemoryContextSwitchTo(oldContext);
319327

@@ -362,6 +370,7 @@ LookupTupleHashEntryHash(TupleHashTable hashtable, TupleTableSlot *slot,
362370
hashtable->cur_eq_func = hashtable->tab_eq_func;
363371

364372
entry = LookupTupleHashEntry_internal(hashtable, slot, isnew, hash);
373+
Assert(entry == NULL || entry->hash == hash);
365374

366375
MemoryContextSwitchTo(oldContext);
367376

@@ -480,7 +489,7 @@ TupleHashTableHash_internal(struct tuplehash_hash *tb,
480489
* NB: This function may or may not change the memory context. Caller is
481490
* expected to change it back.
482491
*/
483-
static TupleHashEntry
492+
static inline TupleHashEntry
484493
LookupTupleHashEntry_internal(TupleHashTable hashtable, TupleTableSlot *slot,
485494
bool *isnew, uint32 hash)
486495
{

‎src/backend/executor/nodeAgg.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeAgg.c
+79-84Lines changed: 79 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,9 @@ static void finalize_partialaggregate(AggState *aggstate,
391391
AggStatePerAgg peragg,
392392
AggStatePerGroup pergroupstate,
393393
Datum *resultVal, bool *resultIsNull);
394-
static void prepare_hash_slot(AggState *aggstate);
394+
static inline void prepare_hash_slot(AggStatePerHash perhash,
395+
TupleTableSlot *inputslot,
396+
TupleTableSlot *hashslot);
395397
static void prepare_projection_slot(AggState *aggstate,
396398
TupleTableSlot *slot,
397399
int currentSet);
@@ -413,8 +415,9 @@ static int hash_choose_num_partitions(uint64 input_groups,
413415
double hashentrysize,
414416
int used_bits,
415417
int *log2_npartittions);
416-
static AggStatePerGroup lookup_hash_entry(AggState *aggstate, uint32 hash,
417-
bool *in_hash_table);
418+
static void initialize_hash_entry(AggState *aggstate,
419+
TupleHashTable hashtable,
420+
TupleHashEntry entry);
418421
static void lookup_hash_entries(AggState *aggstate);
419422
static TupleTableSlot *agg_retrieve_direct(AggState *aggstate);
420423
static void agg_fill_hash_table(AggState *aggstate);
@@ -1207,12 +1210,11 @@ finalize_partialaggregate(AggState *aggstate,
12071210
* Extract the attributes that make up the grouping key into the
12081211
* hashslot. This is necessary to compute the hash or perform a lookup.
12091212
*/
1210-
static void
1211-
prepare_hash_slot(AggState *aggstate)
1213+
static inline void
1214+
prepare_hash_slot(AggStatePerHash perhash,
1215+
TupleTableSlot *inputslot,
1216+
TupleTableSlot *hashslot)
12121217
{
1213-
TupleTableSlot *inputslot = aggstate->tmpcontext->ecxt_outertuple;
1214-
AggStatePerHash perhash = &aggstate->perhash[aggstate->current_set];
1215-
TupleTableSlot *hashslot = perhash->hashslot;
12161218
int i;
12171219

12181220
/* transfer just the needed columns into hashslot */
@@ -2013,75 +2015,39 @@ hash_choose_num_partitions(uint64 input_groups, double hashentrysize,
20132015
}
20142016

20152017
/*
2016-
* Find or create a hashtable entry for the tuple group containing the current
2017-
* tuple (already set in tmpcontext's outertuple slot), in the current grouping
2018-
* set (which the caller must have selected - note that initialize_aggregate
2019-
* depends on this).
2020-
*
2021-
* When called, CurrentMemoryContext should be the per-query context. The
2022-
* already-calculated hash value for the tuple must be specified.
2023-
*
2024-
* If in "spill mode", then only find existing hashtable entries; don't create
2025-
* new ones. If a tuple's group is not already present in the hash table for
2026-
* the current grouping set, assign *in_hash_table=false and the caller will
2027-
* spill it to disk.
2018+
* Initialize a freshly-created TupleHashEntry.
20282019
*/
2029-
static AggStatePerGroup
2030-
lookup_hash_entry(AggState *aggstate, uint32 hash, bool *in_hash_table)
2020+
static void
2021+
initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable,
2022+
TupleHashEntry entry)
20312023
{
2032-
AggStatePerHash perhash = &aggstate->perhash[aggstate->current_set];
2033-
TupleTableSlot *hashslot = perhash->hashslot;
2034-
TupleHashEntryData *entry;
2035-
bool isnew = false;
2036-
bool *p_isnew;
2037-
2038-
/* if hash table already spilled, don't create new entries */
2039-
p_isnew = aggstate->hash_spill_mode ? NULL : &isnew;
2040-
2041-
/* find or create the hashtable entry using the filtered tuple */
2042-
entry = LookupTupleHashEntryHash(perhash->hashtable, hashslot, p_isnew,
2043-
hash);
2044-
2045-
if (entry == NULL)
2046-
{
2047-
*in_hash_table = false;
2048-
return NULL;
2049-
}
2050-
else
2051-
*in_hash_table = true;
2052-
2053-
if (isnew)
2054-
{
2055-
AggStatePerGroup pergroup;
2056-
int transno;
2024+
AggStatePerGroup pergroup;
2025+
int transno;
20572026

2058-
aggstate->hash_ngroups_current++;
2059-
hash_agg_check_limits(aggstate);
2027+
aggstate->hash_ngroups_current++;
2028+
hash_agg_check_limits(aggstate);
20602029

2061-
/* no need to allocate or initialize per-group state */
2062-
if (aggstate->numtrans == 0)
2063-
return NULL;
2030+
/* no need to allocate or initialize per-group state */
2031+
if (aggstate->numtrans == 0)
2032+
return;
20642033

2065-
pergroup = (AggStatePerGroup)
2066-
MemoryContextAlloc(perhash->hashtable->tablecxt,
2067-
sizeof(AggStatePerGroupData) * aggstate->numtrans);
2034+
pergroup = (AggStatePerGroup)
2035+
MemoryContextAlloc(hashtable->tablecxt,
2036+
sizeof(AggStatePerGroupData) * aggstate->numtrans);
20682037

2069-
entry->additional = pergroup;
2038+
entry->additional = pergroup;
20702039

2071-
/*
2072-
* Initialize aggregates for new tuple group, lookup_hash_entries()
2073-
* already has selected the relevant grouping set.
2074-
*/
2075-
for (transno = 0; transno < aggstate->numtrans; transno++)
2076-
{
2077-
AggStatePerTrans pertrans = &aggstate->pertrans[transno];
2078-
AggStatePerGroup pergroupstate = &pergroup[transno];
2040+
/*
2041+
* Initialize aggregates for new tuple group, lookup_hash_entries()
2042+
* already has selected the relevant grouping set.
2043+
*/
2044+
for (transno = 0; transno < aggstate->numtrans; transno++)
2045+
{
2046+
AggStatePerTrans pertrans = &aggstate->pertrans[transno];
2047+
AggStatePerGroup pergroupstate = &pergroup[transno];
20792048

2080-
initialize_aggregate(aggstate, pertrans, pergroupstate);
2081-
}
2049+
initialize_aggregate(aggstate, pertrans, pergroupstate);
20822050
}
2083-
2084-
return entry->additional;
20852051
}
20862052

20872053
/*
@@ -2106,21 +2072,37 @@ static void
21062072
lookup_hash_entries(AggState *aggstate)
21072073
{
21082074
AggStatePerGroup *pergroup = aggstate->hash_pergroup;
2075+
TupleTableSlot *outerslot = aggstate->tmpcontext->ecxt_outertuple;
21092076
int setno;
21102077

21112078
for (setno = 0; setno < aggstate->num_hashes; setno++)
21122079
{
21132080
AggStatePerHash perhash = &aggstate->perhash[setno];
2081+
TupleHashTable hashtable = perhash->hashtable;
2082+
TupleTableSlot *hashslot = perhash->hashslot;
2083+
TupleHashEntry entry;
21142084
uint32 hash;
2115-
bool in_hash_table;
2085+
bool isnew = false;
2086+
bool *p_isnew;
2087+
2088+
/* if hash table already spilled, don't create new entries */
2089+
p_isnew = aggstate->hash_spill_mode ? NULL : &isnew;
21162090

21172091
select_current_set(aggstate, setno, true);
2118-
prepare_hash_slot(aggstate);
2119-
hash = TupleHashTableHash(perhash->hashtable, perhash->hashslot);
2120-
pergroup[setno] = lookup_hash_entry(aggstate, hash, &in_hash_table);
2092+
prepare_hash_slot(perhash,
2093+
outerslot,
2094+
hashslot);
2095+
2096+
entry = LookupTupleHashEntry(hashtable, hashslot,
2097+
p_isnew, &hash);
21212098

2122-
/* check to see if we need to spill the tuple for this grouping set */
2123-
if (!in_hash_table)
2099+
if (entry != NULL)
2100+
{
2101+
if (isnew)
2102+
initialize_hash_entry(aggstate, hashtable, entry);
2103+
pergroup[setno] = entry->additional;
2104+
}
2105+
else
21242106
{
21252107
HashAggSpill *spill = &aggstate->hash_spills[setno];
21262108
TupleTableSlot *slot = aggstate->tmpcontext->ecxt_outertuple;
@@ -2131,6 +2113,7 @@ lookup_hash_entries(AggState *aggstate)
21312113
aggstate->hashentrysize);
21322114

21332115
hashagg_spill_tuple(aggstate, spill, slot, hash);
2116+
pergroup[setno] = NULL;
21342117
}
21352118
}
21362119
}
@@ -2588,6 +2571,7 @@ static bool
25882571
agg_refill_hash_table(AggState *aggstate)
25892572
{
25902573
HashAggBatch *batch;
2574+
AggStatePerHash perhash;
25912575
HashAggSpill spill;
25922576
HashTapeInfo *tapeinfo = aggstate->hash_tapeinfo;
25932577
uint64 ngroups_estimate;
@@ -2639,6 +2623,8 @@ agg_refill_hash_table(AggState *aggstate)
26392623

26402624
select_current_set(aggstate, batch->setno, true);
26412625

2626+
perhash = &aggstate->perhash[aggstate->current_set];
2627+
26422628
/*
26432629
* Spilled tuples are always read back as MinimalTuples, which may be
26442630
* different from the outer plan, so recompile the aggregate expressions.
@@ -2652,27 +2638,34 @@ agg_refill_hash_table(AggState *aggstate)
26522638
HASHAGG_READ_BUFFER_SIZE);
26532639
for (;;)
26542640
{
2655-
TupleTableSlot *slot = aggstate->hash_spill_rslot;
2641+
TupleTableSlot *spillslot = aggstate->hash_spill_rslot;
2642+
TupleTableSlot *hashslot = perhash->hashslot;
2643+
TupleHashEntry entry;
26562644
MinimalTuple tuple;
26572645
uint32 hash;
2658-
bool in_hash_table;
2646+
bool isnew = false;
2647+
bool *p_isnew = aggstate->hash_spill_mode ? NULL : &isnew;
26592648

26602649
CHECK_FOR_INTERRUPTS();
26612650

26622651
tuple = hashagg_batch_read(batch, &hash);
26632652
if (tuple == NULL)
26642653
break;
26652654

2666-
ExecStoreMinimalTuple(tuple, slot, true);
2667-
aggstate->tmpcontext->ecxt_outertuple = slot;
2655+
ExecStoreMinimalTuple(tuple, spillslot, true);
2656+
aggstate->tmpcontext->ecxt_outertuple = spillslot;
26682657

2669-
prepare_hash_slot(aggstate);
2670-
aggstate->hash_pergroup[batch->setno] =
2671-
lookup_hash_entry(aggstate, hash, &in_hash_table);
2658+
prepare_hash_slot(perhash,
2659+
aggstate->tmpcontext->ecxt_outertuple,
2660+
hashslot);
2661+
entry = LookupTupleHashEntryHash(
2662+
perhash->hashtable, hashslot, p_isnew, hash);
26722663

2673-
if (in_hash_table)
2664+
if (entry != NULL)
26742665
{
2675-
/* Advance the aggregates (or combine functions) */
2666+
if (isnew)
2667+
initialize_hash_entry(aggstate, perhash->hashtable, entry);
2668+
aggstate->hash_pergroup[batch->setno] = entry->additional;
26762669
advance_aggregates(aggstate);
26772670
}
26782671
else
@@ -2688,7 +2681,9 @@ agg_refill_hash_table(AggState *aggstate)
26882681
ngroups_estimate, aggstate->hashentrysize);
26892682
}
26902683
/* no memory for a new group, spill */
2691-
hashagg_spill_tuple(aggstate, &spill, slot, hash);
2684+
hashagg_spill_tuple(aggstate, &spill, spillslot, hash);
2685+
2686+
aggstate->hash_pergroup[batch->setno] = NULL;
26922687
}
26932688

26942689
/*

‎src/backend/executor/nodeRecursiveunion.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeRecursiveunion.c
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ ExecRecursiveUnion(PlanState *pstate)
9494
if (plan->numCols > 0)
9595
{
9696
/* Find or build hashtable entry for this tuple's group */
97-
LookupTupleHashEntry(node->hashtable, slot, &isnew);
97+
LookupTupleHashEntry(node->hashtable, slot, &isnew, NULL);
9898
/* Must reset temp context after each hashtable lookup */
9999
MemoryContextReset(node->tempContext);
100100
/* Ignore tuple if already seen */
@@ -141,7 +141,7 @@ ExecRecursiveUnion(PlanState *pstate)
141141
if (plan->numCols > 0)
142142
{
143143
/* Find or build hashtable entry for this tuple's group */
144-
LookupTupleHashEntry(node->hashtable, slot, &isnew);
144+
LookupTupleHashEntry(node->hashtable, slot, &isnew, NULL);
145145
/* Must reset temp context after each hashtable lookup */
146146
MemoryContextReset(node->tempContext);
147147
/* Ignore tuple if already seen */

‎src/backend/executor/nodeSetOp.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeSetOp.c
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ setop_fill_hash_table(SetOpState *setopstate)
381381

382382
/* Find or build hashtable entry for this tuple's group */
383383
entry = LookupTupleHashEntry(setopstate->hashtable, outerslot,
384-
&isnew);
384+
&isnew, NULL);
385385

386386
/* If new tuple group, initialize counts */
387387
if (isnew)
@@ -402,7 +402,7 @@ setop_fill_hash_table(SetOpState *setopstate)
402402

403403
/* For tuples not seen previously, do not make hashtable entry */
404404
entry = LookupTupleHashEntry(setopstate->hashtable, outerslot,
405-
NULL);
405+
NULL, NULL);
406406

407407
/* Advance the counts if entry is already present */
408408
if (entry)

‎src/backend/executor/nodeSubplan.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeSubplan.c
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -595,12 +595,12 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
595595
*/
596596
if (slotNoNulls(slot))
597597
{
598-
(void) LookupTupleHashEntry(node->hashtable, slot, &isnew);
598+
(void) LookupTupleHashEntry(node->hashtable, slot, &isnew, NULL);
599599
node->havehashrows = true;
600600
}
601601
else if (node->hashnulls)
602602
{
603-
(void) LookupTupleHashEntry(node->hashnulls, slot, &isnew);
603+
(void) LookupTupleHashEntry(node->hashnulls, slot, &isnew, NULL);
604604
node->havenullrows = true;
605605
}
606606

‎src/include/executor/executor.h

Copy file name to clipboardExpand all lines: src/include/executor/executor.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ extern TupleHashTable BuildTupleHashTableExt(PlanState *parent,
139139
MemoryContext tempcxt, bool use_variable_hash_iv);
140140
extern TupleHashEntry LookupTupleHashEntry(TupleHashTable hashtable,
141141
TupleTableSlot *slot,
142-
bool *isnew);
142+
bool *isnew, uint32 *hash);
143143
extern uint32 TupleHashTableHash(TupleHashTable hashtable,
144144
TupleTableSlot *slot);
145145
extern TupleHashEntry LookupTupleHashEntryHash(TupleHashTable hashtable,

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.