Currently, nodeMemoize.c builds the hashtable for the cache during
executor startup. This is not what is done in hash joins. I think we
should make the two behave the same way.
Per [1] and the corresponding discussion leading to that, making a
possibly large allocation at executor startup can lead to excessively
long EXPLAIN (not EXPLAIN ANALYZE) times. This can confuse users as
we don't mention in EXPLAIN where the time is being spent.
Although there's not yet any conclusion that Memoize is to blame,
there's another report from someone confused about where this time is
being spent in [2].
Working on the Memoize code, I originally created the hash table
during executor startup to save on having to check we have a table
each time the node is executed. However, the branch for this should
be quite predictable and I doubt it'll add any overhead that we would
notice.
The patch to do this is attached.
David
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1e731ed12aa
[2] https://postgr.es/m/61e642df-5f48-4e4e-b4c3-58936f90d...@thefreecat.org
diff --git a/src/backend/executor/nodeMemoize.c
b/src/backend/executor/nodeMemoize.c
index 0722e47777..1075178340 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -278,11 +278,14 @@ MemoizeHash_equal(struct memoize_hash *tb, const
MemoizeKey *key1,
}
/*
- * Initialize the hash table to empty.
+ * Initialize the hash table to empty. The MemoizeState's hashtable field
+ * must point to NULL.
*/
static void
build_hash_table(MemoizeState *mstate, uint32 size)
{
+ Assert(mstate->hashtable == NULL);
+
/* Make a guess at a good size when we're not given a valid size. */
if (size == 0)
size = 1024;
@@ -400,9 +403,12 @@ remove_cache_entry(MemoizeState *mstate, MemoizeEntry
*entry)
static void
cache_purge_all(MemoizeState *mstate)
{
- uint64 evictions = mstate->hashtable->members;
+ uint64 evictions = 0;
PlanState *pstate = (PlanState *) mstate;
+ if (mstate->hashtable != NULL)
+ evictions = mstate->hashtable->members;
+
/*
* Likely the most efficient way to remove all items is to just reset
the
* memory context for the cache and then rebuild a fresh hash table.
This
@@ -410,8 +416,8 @@ cache_purge_all(MemoizeState *mstate)
*/
MemoryContextReset(mstate->tableContext);
- /* Make the hash table the same size as the original size */
- build_hash_table(mstate, ((Memoize *) pstate->plan)->est_entries);
+ /* NULLify so we recreate the table on the next call */
+ mstate->hashtable = NULL;
/* reset the LRU list */
dlist_init(&mstate->lru_list);
@@ -707,6 +713,10 @@ ExecMemoize(PlanState *pstate)
Assert(node->entry == NULL);
+ /* first call? we'll need a hash table. */
+ if (unlikely(node->hashtable == NULL))
+ build_hash_table(node, ((Memoize *)
pstate->plan)->est_entries);
+
/*
* We're only ever in this state for the first
call of the
* scan. Here we have a look to see if we've
already seen the
@@ -1051,8 +1061,11 @@ ExecInitMemoize(Memoize *node, EState *estate, int
eflags)
/* Zero the statistics counters */
memset(&mstate->stats, 0, sizeof(MemoizeInstrumentation));
- /* Allocate and set up the actual cache */
- build_hash_table(mstate, node->est_entries);
+ /*
+ * Because it may require a large allocation we delay building of the
hash
+ * table until executor run.
+ */
+ mstate->hashtable = NULL;
return mstate;
}
@@ -1062,6 +1075,7 @@ ExecEndMemoize(MemoizeState *node)
{
#ifdef USE_ASSERT_CHECKING
/* Validate the memory accounting code is correct in assert builds. */
+ if (node->hashtable != NULL)
{
int count;
uint64 mem = 0;
