On Tue, 10 May 2022 at 14:22, Justin Pryzby <pry...@telsasoft.com> wrote:
> On Fri, May 06, 2022 at 09:27:57PM +1200, David Rowley wrote:
> > I'm very tempted to change the EXPLAIN output in at least master to
> > display the initial and final (maximum) hash table sizes. Wondering if
> > anyone would object to that?
>
> No objection to add it to v15.
>
> I'll point out that "Cache Mode" was added to EXPLAIN between 11.1 and 11.2
> without controversy, so this could conceivably be backpatched to v14, too.
>
> commit 6c32c0977783fae217b5eaa1d22d26c96e5b0085
This is seemingly a good point, but I don't really think it's a case
of just keeping the EXPLAIN output stable in minor versions, it's more
about adding new fields to structs.
I just went and wrote the patch and the fundamental difference seems
to be that what I did in 6c32c0977 managed to only add a new field in
the empty padding between two fields. That resulted in no fields in
the struct being pushed up in their address offset. The idea here is
not to break any extension that's already been compiled that
references some field that comes after that.
In the patch I've just written, I've had to add some fields which
causes sizeof(MemoizeState) to go up resulting in the offsets of some
later fields changing.
One thing I'll say about this patch is that I found it annoying that I
had to add code to cache_lookup() when we failed to find an entry.
That's probably not the end of the world as that's only for cache
misses. Ideally, I'd just be looking at the size of the hash table at
the end of execution, however, naturally, we must show the EXPLAIN
output before we shut down the executor.
I just copied the Hash Join output. It looks like:
# alter table tt alter column a set (n_distinct=4);
ALTER TABLE
# analyze tt;
# explain (analyze, costs off, timing off) select * from tt inner join
t2 on tt.a=t2.a;
QUERY PLAN
---------------------------------------------------------------------------------
Nested Loop (actual rows=1000000 loops=1)
-> Seq Scan on tt (actual rows=1000000 loops=1)
-> Memoize (actual rows=1 loops=1000000)
Cache Key: tt.a
Cache Mode: logical
Hits: 999990 Misses: 10 Evictions: 0 Overflows: 0 Memory Usage: 2kB
Hash Buckets: 16 (originally 4)
-> Index Only Scan using t2_pkey on t2 (actual rows=1 loops=10)
Index Cond: (a = tt.a)
Heap Fetches: 0
Planning Time: 0.483 ms
Execution Time: 862.860 ms
(12 rows)
Does anyone have any views about the attached patch going into v15?
David
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index d2a2479822..c535d63883 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3177,6 +3177,11 @@ show_memoize_info(MemoizeState *mstate, List *ancestors,
ExplainState *es)
ExplainPropertyInteger("Cache Evictions", NULL,
mstate->stats.cache_evictions, es);
ExplainPropertyInteger("Cache Overflows", NULL,
mstate->stats.cache_overflows, es);
ExplainPropertyInteger("Peak Memory Usage", "kB",
memPeakKb, es);
+ ExplainPropertyInteger("Maximum Hash Buckets", NULL,
+ mstate->stats.hashsize_max, es);
+ ExplainPropertyInteger("Original Hash Buckets", NULL,
+ mstate->stats.hashsize_orig, es);
+
}
else
{
@@ -3188,6 +3193,14 @@ show_memoize_info(MemoizeState *mstate, List *ancestors,
ExplainState *es)
mstate->stats.cache_evictions,
mstate->stats.cache_overflows,
memPeakKb);
+ ExplainIndentText(es);
+ if (mstate->stats.hashsize_max !=
mstate->stats.hashsize_orig)
+ appendStringInfo(es->str, "Hash Buckets: %u
(originally %u)\n",
+
mstate->stats.hashsize_max,
+
mstate->stats.hashsize_orig);
+ else
+ appendStringInfo(es->str, "Hash Buckets: %u\n",
+
mstate->stats.hashsize_max);
}
}
@@ -3227,6 +3240,14 @@ show_memoize_info(MemoizeState *mstate, List *ancestors,
ExplainState *es)
si->cache_hits,
si->cache_misses,
si->cache_evictions,
si->cache_overflows,
memPeakKb);
+ ExplainIndentText(es);
+ if (si->hashsize_max != si->hashsize_orig)
+ appendStringInfo(es->str, "Hash Buckets: %u
(originally %u)\n",
+
si->hashsize_max,
+
si->hashsize_orig);
+ else
+ appendStringInfo(es->str, "Hash Buckets: %u\n",
+
si->hashsize_max);
}
else
{
@@ -3240,6 +3261,11 @@ show_memoize_info(MemoizeState *mstate, List *ancestors,
ExplainState *es)
si->cache_overflows, es);
ExplainPropertyInteger("Peak Memory Usage", "kB",
memPeakKb,
es);
+ ExplainPropertyInteger("Maximum Hash Buckets", NULL,
+
si->hashsize_max, es);
+ ExplainPropertyInteger("Original Hash Buckets", NULL,
+
si->hashsize_orig, es);
+
}
if (es->workers_state)
diff --git a/src/backend/executor/nodeMemoize.c
b/src/backend/executor/nodeMemoize.c
index 23441e33ca..e2a17a615f 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -270,6 +270,10 @@ build_hash_table(MemoizeState *mstate, uint32 size)
/* memoize_create will convert the size to a power of 2 */
mstate->hashtable = memoize_create(mstate->tableContext, size, mstate);
+
+ /* Record the initial size of the hash table */
+ if (mstate->stats.hashsize_orig == 0)
+ mstate->stats.hashsize_orig = mstate->hashtable->size;
}
/*
@@ -527,6 +531,10 @@ cache_lookup(MemoizeState *mstate, bool *found)
/* Update the total cache memory utilization */
mstate->mem_used += EMPTY_ENTRY_MEMORY_BYTES(entry);
+ /* Record the maximum size of the hash table */
+ mstate->stats.hashsize_max = Max(mstate->stats.hashsize_max,
+
mstate->hashtable->size);
+
/* Initialize this entry */
entry->complete = false;
entry->tuplehead = NULL;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 94b191f8ae..1e7012d6e8 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -2109,6 +2109,8 @@ typedef struct MemoizeInstrumentation
* able
to free enough space to store the
*
current scan's tuples. */
uint64 mem_peak; /* peak memory usage in bytes */
+ uint32 hashsize_orig; /* Original hash bucket count */
+ uint32 hashsize_max; /* Maximum hash table bucket count */
} MemoizeInstrumentation;
/* ----------------
