Skip to content

Commit b4a07f5

Browse files
jeff-davisjeff-davis
committed
Revert "TupleHashTable: store additional data along with tuple."
This reverts commit e0ece2a due to performance regressions. Reported-by: David Rowley
1 parent af23176 commit b4a07f5

File tree

6 files changed

+31
-78
lines changed

6 files changed

+31
-78
lines changed

‎src/backend/executor/execGrouping.c

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@
2020
#include "miscadmin.h"
2121
#include "utils/lsyscache.h"
2222

23-
struct TupleHashEntryData
24-
{
25-
MinimalTuple firstTuple; /* copy of first tuple in this group */
26-
uint32 status; /* hash status */
27-
uint32 hash; /* hash value (cached) */
28-
};
29-
3023
static int TupleHashTableMatch(struct tuplehash_hash *tb, const MinimalTuple tuple1, const MinimalTuple tuple2);
3124
static inline uint32 TupleHashTableHash_internal(struct tuplehash_hash *tb,
3225
const MinimalTuple tuple);
@@ -203,7 +196,6 @@ BuildTupleHashTable(PlanState *parent,
203196
hashtable->tab_collations = collations;
204197
hashtable->tablecxt = tablecxt;
205198
hashtable->tempcxt = tempcxt;
206-
hashtable->additionalsize = additionalsize;
207199
hashtable->tableslot = NULL; /* will be made on first lookup */
208200
hashtable->inputslot = NULL;
209201
hashtable->in_hash_expr = NULL;
@@ -281,15 +273,6 @@ ResetTupleHashTable(TupleHashTable hashtable)
281273
tuplehash_reset(hashtable->hashtab);
282274
}
283275

284-
/*
285-
* Return size of the hash bucket. Useful for estimating memory usage.
286-
*/
287-
size_t
288-
TupleHashEntrySize(void)
289-
{
290-
return sizeof(TupleHashEntryData);
291-
}
292-
293276
/*
294277
* Find or create a hashtable entry for the tuple group containing the
295278
* given tuple. The tuple must be the same type as the hashtable entries.
@@ -356,24 +339,6 @@ TupleHashTableHash(TupleHashTable hashtable, TupleTableSlot *slot)
356339
return hash;
357340
}
358341

359-
MinimalTuple
360-
TupleHashEntryGetTuple(TupleHashEntry entry)
361-
{
362-
return entry->firstTuple;
363-
}
364-
365-
/*
366-
* Get a pointer into the additional space allocated for this entry. The
367-
* amount of space available is the additionalsize specified to
368-
* BuildTupleHashTable(). If additionalsize was specified as zero, no
369-
* additional space is available and this function should not be called.
370-
*/
371-
void *
372-
TupleHashEntryGetAdditional(TupleHashEntry entry)
373-
{
374-
return (char *) entry->firstTuple + MAXALIGN(entry->firstTuple->t_len);
375-
}
376-
377342
/*
378343
* A variant of LookupTupleHashEntry for callers that have already computed
379344
* the hash value.
@@ -512,31 +477,13 @@ LookupTupleHashEntry_internal(TupleHashTable hashtable, TupleTableSlot *slot,
512477
}
513478
else
514479
{
515-
MinimalTuple firstTuple;
516-
size_t totalsize; /* including alignment and additionalsize */
517-
518480
/* created new entry */
519481
*isnew = true;
520482
/* zero caller data */
483+
entry->additional = NULL;
521484
MemoryContextSwitchTo(hashtable->tablecxt);
522-
523485
/* Copy the first tuple into the table context */
524-
firstTuple = ExecCopySlotMinimalTuple(slot);
525-
526-
/*
527-
* Allocate additional space right after the MinimalTuple of size
528-
* additionalsize. The caller can get a pointer to this data with
529-
* TupleHashEntryGetAdditional(), and store arbitrary data there.
530-
*
531-
* This avoids the need to store an extra pointer or allocate an
532-
* additional chunk, which would waste memory.
533-
*/
534-
totalsize = MAXALIGN(firstTuple->t_len) + hashtable->additionalsize;
535-
firstTuple = repalloc(firstTuple, totalsize);
536-
memset((char *) firstTuple + firstTuple->t_len, 0,
537-
totalsize - firstTuple->t_len);
538-
539-
entry->firstTuple = firstTuple;
486+
entry->firstTuple = ExecCopySlotMinimalTuple(slot);
540487
}
541488
}
542489
else

‎src/backend/executor/nodeAgg.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,7 +1713,7 @@ hash_agg_entry_size(int numTrans, Size tupleWidth, Size transitionSpace)
17131713
transitionChunkSize = 0;
17141714

17151715
return
1716-
TupleHashEntrySize() +
1716+
sizeof(TupleHashEntryData) +
17171717
tupleChunkSize +
17181718
pergroupChunkSize +
17191719
transitionChunkSize;
@@ -1954,7 +1954,7 @@ hash_agg_update_metrics(AggState *aggstate, bool from_tape, int npartitions)
19541954
if (aggstate->hash_ngroups_current > 0)
19551955
{
19561956
aggstate->hashentrysize =
1957-
TupleHashEntrySize() +
1957+
sizeof(TupleHashEntryData) +
19581958
(hashkey_mem / (double) aggstate->hash_ngroups_current);
19591959
}
19601960
}
@@ -2055,7 +2055,11 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable,
20552055
if (aggstate->numtrans == 0)
20562056
return;
20572057

2058-
pergroup = (AggStatePerGroup) TupleHashEntryGetAdditional(entry);
2058+
pergroup = (AggStatePerGroup)
2059+
MemoryContextAlloc(hashtable->tablecxt,
2060+
sizeof(AggStatePerGroupData) * aggstate->numtrans);
2061+
2062+
entry->additional = pergroup;
20592063

20602064
/*
20612065
* Initialize aggregates for new tuple group, lookup_hash_entries()
@@ -2119,7 +2123,7 @@ lookup_hash_entries(AggState *aggstate)
21192123
{
21202124
if (isnew)
21212125
initialize_hash_entry(aggstate, hashtable, entry);
2122-
pergroup[setno] = TupleHashEntryGetAdditional(entry);
2126+
pergroup[setno] = entry->additional;
21232127
}
21242128
else
21252129
{
@@ -2677,7 +2681,7 @@ agg_refill_hash_table(AggState *aggstate)
26772681
{
26782682
if (isnew)
26792683
initialize_hash_entry(aggstate, perhash->hashtable, entry);
2680-
aggstate->hash_pergroup[batch->setno] = TupleHashEntryGetAdditional(entry);
2684+
aggstate->hash_pergroup[batch->setno] = entry->additional;
26812685
advance_aggregates(aggstate);
26822686
}
26832687
else
@@ -2769,7 +2773,7 @@ agg_retrieve_hash_table_in_memory(AggState *aggstate)
27692773
ExprContext *econtext;
27702774
AggStatePerAgg peragg;
27712775
AggStatePerGroup pergroup;
2772-
TupleHashEntry entry;
2776+
TupleHashEntryData *entry;
27732777
TupleTableSlot *firstSlot;
27742778
TupleTableSlot *result;
27752779
AggStatePerHash perhash;
@@ -2841,7 +2845,7 @@ agg_retrieve_hash_table_in_memory(AggState *aggstate)
28412845
* Transform representative tuple back into one with the right
28422846
* columns.
28432847
*/
2844-
ExecStoreMinimalTuple(TupleHashEntryGetTuple(entry), hashslot, false);
2848+
ExecStoreMinimalTuple(entry->firstTuple, hashslot, false);
28452849
slot_getallattrs(hashslot);
28462850

28472851
ExecClearTuple(firstSlot);
@@ -2857,7 +2861,7 @@ agg_retrieve_hash_table_in_memory(AggState *aggstate)
28572861
}
28582862
ExecStoreVirtualTuple(firstSlot);
28592863

2860-
pergroup = (AggStatePerGroup) TupleHashEntryGetAdditional(entry);
2864+
pergroup = (AggStatePerGroup) entry->additional;
28612865

28622866
/*
28632867
* Use the representative input tuple for any references to

‎src/backend/executor/nodeSetOp.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,6 @@ setop_fill_hash_table(SetOpState *setopstate)
425425
{
426426
TupleTableSlot *outerslot;
427427
TupleHashEntryData *entry;
428-
SetOpStatePerGroup pergroup;
429428
bool isnew;
430429

431430
outerslot = ExecProcNode(outerPlan);
@@ -438,16 +437,16 @@ setop_fill_hash_table(SetOpState *setopstate)
438437
outerslot,
439438
&isnew, NULL);
440439

441-
pergroup = TupleHashEntryGetAdditional(entry);
442440
/* If new tuple group, initialize counts to zero */
443441
if (isnew)
444442
{
445-
pergroup->numLeft = 0;
446-
pergroup->numRight = 0;
443+
entry->additional = (SetOpStatePerGroup)
444+
MemoryContextAllocZero(setopstate->hashtable->tablecxt,
445+
sizeof(SetOpStatePerGroupData));
447446
}
448447

449448
/* Advance the counts */
450-
pergroup->numLeft++;
449+
((SetOpStatePerGroup) entry->additional)->numLeft++;
451450

452451
/* Must reset expression context after each hashtable lookup */
453452
ResetExprContext(econtext);
@@ -479,7 +478,7 @@ setop_fill_hash_table(SetOpState *setopstate)
479478

480479
/* Advance the counts if entry is already present */
481480
if (entry)
482-
((SetOpStatePerGroup) TupleHashEntryGetAdditional(entry))->numRight++;
481+
((SetOpStatePerGroup) entry->additional)->numRight++;
483482

484483
/* Must reset expression context after each hashtable lookup */
485484
ResetExprContext(econtext);
@@ -497,7 +496,7 @@ setop_fill_hash_table(SetOpState *setopstate)
497496
static TupleTableSlot *
498497
setop_retrieve_hash_table(SetOpState *setopstate)
499498
{
500-
TupleHashEntry entry;
499+
TupleHashEntryData *entry;
501500
TupleTableSlot *resultTupleSlot;
502501

503502
/*
@@ -527,12 +526,12 @@ setop_retrieve_hash_table(SetOpState *setopstate)
527526
* See if we should emit any copies of this tuple, and if so return
528527
* the first copy.
529528
*/
530-
set_output_count(setopstate, (SetOpStatePerGroup) TupleHashEntryGetAdditional(entry));
529+
set_output_count(setopstate, (SetOpStatePerGroup) entry->additional);
531530

532531
if (setopstate->numOutput > 0)
533532
{
534533
setopstate->numOutput--;
535-
return ExecStoreMinimalTuple(TupleHashEntryGetTuple(entry),
534+
return ExecStoreMinimalTuple(entry->firstTuple,
536535
resultTupleSlot,
537536
false);
538537
}

‎src/backend/executor/nodeSubplan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ findPartialMatch(TupleHashTable hashtable, TupleTableSlot *slot,
753753
{
754754
CHECK_FOR_INTERRUPTS();
755755

756-
ExecStoreMinimalTuple(TupleHashEntryGetTuple(entry), hashtable->tableslot, false);
756+
ExecStoreMinimalTuple(entry->firstTuple, hashtable->tableslot, false);
757757
if (!execTuplesUnequal(slot, hashtable->tableslot,
758758
numCols, keyColIdx,
759759
eqfunctions,

‎src/include/executor/executor.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,9 @@ extern TupleHashEntry LookupTupleHashEntry(TupleHashTable hashtable,
148148
bool *isnew, uint32 *hash);
149149
extern uint32 TupleHashTableHash(TupleHashTable hashtable,
150150
TupleTableSlot *slot);
151-
extern size_t TupleHashEntrySize(void);
152151
extern TupleHashEntry LookupTupleHashEntryHash(TupleHashTable hashtable,
153152
TupleTableSlot *slot,
154153
bool *isnew, uint32 hash);
155-
extern MinimalTuple TupleHashEntryGetTuple(TupleHashEntry entry);
156-
extern void *TupleHashEntryGetAdditional(TupleHashEntry entry);
157154
extern TupleHashEntry FindTupleHashEntry(TupleHashTable hashtable,
158155
TupleTableSlot *slot,
159156
ExprState *eqcomp,

‎src/include/nodes/execnodes.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -806,10 +806,17 @@ typedef struct ExecAuxRowMark
806806
* point to tab_hash_expr and tab_eq_func respectively.
807807
* ----------------------------------------------------------------
808808
*/
809-
typedef struct TupleHashEntryData TupleHashEntryData;
810809
typedef struct TupleHashEntryData *TupleHashEntry;
811810
typedef struct TupleHashTableData *TupleHashTable;
812811

812+
typedef struct TupleHashEntryData
813+
{
814+
MinimalTuple firstTuple; /* copy of first tuple in this group */
815+
void *additional; /* user data */
816+
uint32 status; /* hash status */
817+
uint32 hash; /* hash value (cached) */
818+
} TupleHashEntryData;
819+
813820
/* define parameters necessary to generate the tuple hash table interface */
814821
#define SH_PREFIX tuplehash
815822
#define SH_ELEMENT_TYPE TupleHashEntryData
@@ -828,7 +835,6 @@ typedef struct TupleHashTableData
828835
Oid *tab_collations; /* collations for hash and comparison */
829836
MemoryContext tablecxt; /* memory context containing table */
830837
MemoryContext tempcxt; /* context for function evaluations */
831-
Size additionalsize; /* size of additional data */
832838
TupleTableSlot *tableslot; /* slot for referencing table entries */
833839
/* The following fields are set transiently for each table search: */
834840
TupleTableSlot *inputslot; /* current input tuple's slot */

0 commit comments

Comments
 (0)