On Fri, Jul 28, 2023 at 7:28 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com>
wrote:
> >
> > + bms_free(child_sjinfo->commute_above_l);
> > + bms_free(child_sjinfo->commute_above_r);
> > + bms_free(child_sjinfo->commute_below_l);
> > + bms_free(child_sjinfo->commute_below_r);
> >
> > These four members in SpecialJoinInfo only contain outer join relids.
> > They do not need to be translated. So they would reference the same
> > memory areas in child_sjinfo as in parent_sjinfo. We should not free
> > them, otherwise they would become dangling pointers in parent sjinfo.
> >
>
>
Attached patchset fixing those.
0001 - patch to report planning memory, with to explain.out regression
output fix. We may consider committing this as well.
0002 - with your comment addressed above.
>
> I think we should 1. add an assert to make sure that commute_above_*
> do not require any transations i.e. they do not contain any parent
> relids of the child rels.
Looking at the definitions of commute_above and commute_below relids and OJ
relids, I don't think these assertions are necessary. So didn't add those.
> 2. Do not free those.
Done
> 3. Add a comment about
> keeping the build and free functions in sync.
>
Done.
--
Best Wishes,
Ashutosh Bapat
From 553a147b4566e4f6da28e2f7dd2c9759371afd7b Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 24 Jul 2023 20:20:53 +0530
Subject: [PATCH 2/2] Reduce memory used by child SpecialJoinInfo
The SpecialJoinInfo applicable to a child join is computed by
translating the same applicable to the parent join in
try_partitionwise_join(). The child SpecialJoinInfo is not needed once
the child join RelOptInfo is created and paths are added to it. Use a
local variable to hold child SpecialJoinInfo so that it doesn't need
memory allocated separately.
Also free the memory allocated to various Bitmapsets in SpecialJoinInfo.
Those are not referenced anywhere. But we do not free the memory
allocated to expression trees of operator OID list since those may be
referenced in paths or other objects.
---
src/backend/optimizer/path/joinrels.c | 70 +++++++++++++++++++--------
1 file changed, 50 insertions(+), 20 deletions(-)
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 015a0b3cbe..5cf32f8b07 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -42,9 +42,11 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
List *parent_restrictlist);
-static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
- SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids);
+static void build_child_join_sjinfo(PlannerInfo *root,
+ SpecialJoinInfo *parent_sjinfo,
+ Relids left_relids, Relids right_relids,
+ SpecialJoinInfo *child_sjinfo);
+static void free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo);
static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
@@ -1540,7 +1542,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
RelOptInfo *child_rel2;
bool rel1_empty;
bool rel2_empty;
- SpecialJoinInfo *child_sjinfo;
+ SpecialJoinInfo child_sjinfo;
List *child_restrictlist;
RelOptInfo *child_joinrel;
AppendRelInfo **appinfos;
@@ -1635,9 +1637,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct SpecialJoinInfo from parent join relations's
* SpecialJoinInfo.
*/
- child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo,
- child_rel1->relids,
- child_rel2->relids);
+ build_child_join_sjinfo(root, parent_sjinfo, child_rel1->relids,
+ child_rel2->relids, &child_sjinfo);
/* Find the AppendRelInfo structures */
appinfos = find_appinfos_by_relids(root,
@@ -1660,7 +1661,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
{
child_joinrel = build_child_join_rel(root, child_rel1, child_rel2,
joinrel, child_restrictlist,
- child_sjinfo);
+ &child_sjinfo);
joinrel->part_rels[cnt_parts] = child_joinrel;
joinrel->live_parts = bms_add_member(joinrel->live_parts, cnt_parts);
joinrel->all_partrels = bms_add_members(joinrel->all_partrels,
@@ -1674,10 +1675,11 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
/* And make paths for the child join */
populate_joinrel_with_paths(root, child_rel1, child_rel2,
- child_joinrel, child_sjinfo,
+ child_joinrel, &child_sjinfo,
child_restrictlist);
pfree(appinfos);
+ free_child_sjinfo_members(&child_sjinfo);
}
}
@@ -1685,43 +1687,71 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct the SpecialJoinInfo for a child-join by translating
* SpecialJoinInfo for the join between parents. left_relids and right_relids
* are the relids of left and right side of the join respectively.
+ *
+ * If translations are added or removed from this function, consider freeing
+ * the translated objects in free_child_sjinfo_members() appropriately.
*/
-static SpecialJoinInfo *
+static void
build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids)
+ Relids left_relids, Relids right_relids,
+ SpecialJoinInfo *child_sjinfo)
{
- SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
AppendRelInfo **left_appinfos;
int left_nappinfos;
AppendRelInfo **right_appinfos;
int right_nappinfos;
- memcpy(sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
+ memcpy(child_sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
left_appinfos = find_appinfos_by_relids(root, left_relids,
&left_nappinfos);
right_appinfos = find_appinfos_by_relids(root, right_relids,
&right_nappinfos);
- sjinfo->min_lefthand = adjust_child_relids(sjinfo->min_lefthand,
+ child_sjinfo->min_lefthand = adjust_child_relids(child_sjinfo->min_lefthand,
left_nappinfos, left_appinfos);
- sjinfo->min_righthand = adjust_child_relids(sjinfo->min_righthand,
+ child_sjinfo->min_righthand = adjust_child_relids(child_sjinfo->min_righthand,
right_nappinfos,
right_appinfos);
- sjinfo->syn_lefthand = adjust_child_relids(sjinfo->syn_lefthand,
+ child_sjinfo->syn_lefthand = adjust_child_relids(child_sjinfo->syn_lefthand,
left_nappinfos, left_appinfos);
- sjinfo->syn_righthand = adjust_child_relids(sjinfo->syn_righthand,
+ child_sjinfo->syn_righthand = adjust_child_relids(child_sjinfo->syn_righthand,
right_nappinfos,
right_appinfos);
+ /*
+ * commute_above and commute_below relids contain only OJ relids which do
+ * not need any translation.
+ */
+
/* outer-join relids need no adjustment */
- sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
- (Node *) sjinfo->semi_rhs_exprs,
+ child_sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
+ (Node *) child_sjinfo->semi_rhs_exprs,
right_nappinfos,
right_appinfos);
pfree(left_appinfos);
pfree(right_appinfos);
+}
- return sjinfo;
+/*
+ * free_child_sjinfo_members
+ * Free memory consumed by members of a child SpecialJoinInfo.
+ */
+static void
+free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo)
+{
+ /*
+ * The relids are used only for comparison and their references are not
+ * stored anywhere. Free those.
+ */
+ bms_free(child_sjinfo->min_lefthand);
+ bms_free(child_sjinfo->min_righthand);
+ bms_free(child_sjinfo->syn_lefthand);
+ bms_free(child_sjinfo->syn_righthand);
+
+ /*
+ * But the list of operator OIDs and the list of expressions may be
+ * referenced somewhere else. Do not free those.
+ */
}
/*
--
2.25.1
From 670a84a4281b7e17a3a61df0ce0737daff94840f Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 12 Jul 2023 14:34:14 +0530
Subject: [PATCH 1/2] Report memory used for planning a query in EXPLAIN
ANALYZE
The memory used in the CurrentMemoryContext and its children is sampled
before and after calling pg_plan_query() from ExplainOneQuery(). The
difference in the two samples is reported as the memory consumed while
planning the query. This may not account for the memory allocated in
memory contexts which are not children of CurrentMemoryContext. These
contexts are usually other long lived contexts, e.g.
CacheMemoryContext, which are shared by all the queries run in a
session. The consumption in those can not be attributed only to a given
query and hence should not be reported any way.
The memory consumption is reported as "Planning Memory" property in
EXPLAIN ANALYZE output.
Ashutosh Bapat
---
src/backend/commands/explain.c | 12 ++++++++++--
src/backend/commands/prepare.c | 7 ++++++-
src/backend/utils/mmgr/mcxt.c | 19 +++++++++++++++++++
src/include/commands/explain.h | 3 ++-
src/include/utils/memutils.h | 1 +
src/test/regress/expected/explain.out | 15 +++++++++++----
6 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 8570b14f62..9f859949f0 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -397,16 +397,20 @@ ExplainOneQuery(Query *query, int cursorOptions,
planduration;
BufferUsage bufusage_start,
bufusage;
+ Size mem_consumed;
if (es->buffers)
bufusage_start = pgBufferUsage;
INSTR_TIME_SET_CURRENT(planstart);
+ mem_consumed = MemoryContextMemUsed(CurrentMemoryContext);
/* plan the query */
plan = pg_plan_query(query, queryString, cursorOptions, params);
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ mem_consumed = MemoryContextMemUsed(CurrentMemoryContext)
+ - mem_consumed;
/* calc differences of buffer counters. */
if (es->buffers)
@@ -417,7 +421,7 @@ ExplainOneQuery(Query *query, int cursorOptions,
/* run it (if needed) and produce output */
ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL), &mem_consumed);
}
}
@@ -527,7 +531,7 @@ void
ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
- const BufferUsage *bufusage)
+ const BufferUsage *bufusage, const Size *mem_consumed)
{
DestReceiver *dest;
QueryDesc *queryDesc;
@@ -628,6 +632,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
double plantime = INSTR_TIME_GET_DOUBLE(*planduration);
ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
+
+ if (mem_consumed)
+ ExplainPropertyUInteger("Planning Memory", "bytes",
+ (uint64) *mem_consumed, es);
}
/* Print info about runtime of triggers */
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..84544ce481 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -583,10 +583,12 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
instr_time planduration;
BufferUsage bufusage_start,
bufusage;
+ Size mem_consumed;
if (es->buffers)
bufusage_start = pgBufferUsage;
INSTR_TIME_SET_CURRENT(planstart);
+ mem_consumed = MemoryContextMemUsed(CurrentMemoryContext);
/* Look it up in the hash table */
entry = FetchPreparedStatement(execstmt->name, true);
@@ -623,6 +625,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ mem_consumed = MemoryContextMemUsed(CurrentMemoryContext)
+ - mem_consumed;
/* calc differences of buffer counters. */
if (es->buffers)
@@ -640,7 +644,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
if (pstmt->commandType != CMD_UTILITY)
ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL),
+ &mem_consumed);
else
ExplainOneUtility(pstmt->utilityStmt, into, es, query_string,
paramLI, queryEnv);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6..43af271f33 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -747,6 +747,25 @@ MemoryContextStatsDetail(MemoryContext context, int max_children,
grand_totals.totalspace - grand_totals.freespace)));
}
+/*
+ * Compute the memory used in the given context and its children.
+ *
+ * XXX: Instead of this separate function we could modify
+ * MemoryContextStatsDetail() to report used memory and disable printing the
+ * detailed stats.
+ */
+extern Size
+MemoryContextMemUsed(MemoryContext context)
+{
+ MemoryContextCounters grand_totals;
+
+ memset(&grand_totals, 0, sizeof(grand_totals));
+
+ MemoryContextStatsInternal(context, 0, false, 100, &grand_totals, false);
+
+ return grand_totals.totalspace - grand_totals.freespace;
+}
+
/*
* MemoryContextStatsInternal
* One recursion level for MemoryContextStats
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 3d3e632a0c..21e3d2f309 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -92,7 +92,8 @@ extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
- const BufferUsage *bufusage);
+ const BufferUsage *bufusage,
+ const Size *mem_consumed);
extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc);
extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 21640d62a6..d7c477f229 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -92,6 +92,7 @@ extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
bool print_to_stderr);
extern void MemoryContextAllowInCriticalSection(MemoryContext context,
bool allow);
+extern Size MemoryContextMemUsed(MemoryContext context);
#ifdef MEMORY_CONTEXT_CHECKING
extern void MemoryContextCheck(MemoryContext context);
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 1aca77491b..123ab22f5d 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -65,8 +65,9 @@ select explain_filter('explain (analyze) select * from int8_tbl i8');
-----------------------------------------------------------------------------------------------
Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
Planning Time: N.N ms
+ Planning Memory: N bytes
Execution Time: N.N ms
-(3 rows)
+(4 rows)
select explain_filter('explain (analyze, verbose) select * from int8_tbl i8');
explain_filter
@@ -74,16 +75,18 @@ select explain_filter('explain (analyze, verbose) select * from int8_tbl i8');
Seq Scan on public.int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
Output: q1, q2
Planning Time: N.N ms
+ Planning Memory: N bytes
Execution Time: N.N ms
-(4 rows)
+(5 rows)
select explain_filter('explain (analyze, buffers, format text) select * from int8_tbl i8');
explain_filter
-----------------------------------------------------------------------------------------------
Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
Planning Time: N.N ms
+ Planning Memory: N bytes
Execution Time: N.N ms
-(3 rows)
+(4 rows)
select explain_filter('explain (analyze, buffers, format xml) select * from int8_tbl i8');
explain_filter
@@ -128,6 +131,7 @@ select explain_filter('explain (analyze, buffers, format xml) select * from int8
<Temp-Written-Blocks>N</Temp-Written-Blocks> +
</Planning> +
<Planning-Time>N.N</Planning-Time> +
+ <Planning-Memory>N</Planning-Memory> +
<Triggers> +
</Triggers> +
<Execution-Time>N.N</Execution-Time> +
@@ -174,6 +178,7 @@ select explain_filter('explain (analyze, buffers, format yaml) select * from int
Temp Read Blocks: N +
Temp Written Blocks: N +
Planning Time: N.N +
+ Planning Memory: N +
Triggers: +
Execution Time: N.N
(1 row)
@@ -280,6 +285,7 @@ select explain_filter('explain (analyze, buffers, format json) select * from int
"Temp I/O Write Time": N.N +
}, +
"Planning Time": N.N, +
+ "Planning Memory": N, +
"Triggers": [ +
], +
"Execution Time": N.N +
@@ -531,7 +537,8 @@ select jsonb_pretty(
"Triggers": [ +
], +
"Planning Time": 0.0, +
- "Execution Time": 0.0 +
+ "Execution Time": 0.0, +
+ "Planning Memory": 0 +
} +
]
(1 row)
--
2.25.1
