Re: Report planning memory in EXPLAIN ANALYZE

Using your patch I found out one redundant memory usage in the planner [1]. It can be interesting as an example of how
thispatch can detect problems. 

[1] Optimize planner memory consumption for huge arrays
https://www.postgresql.org/message-id/flat/em9939439a-441a-4b27-a977-ebdf5987dc49%407d14f008.com

--
Regards,
Andrei Lepikhov

On Thu, Aug 24, 2023, at 5:31 PM, Ashutosh Bapat wrote:
> Sorry for the late reply. I was working on David's suggestion.
>
> Here's a response to your questions and also a new set of patches.
>
> On Tue, Aug 22, 2023 at 1:16 PM jian he <jian.universality@gmail.com> wrote:
>> Hi. I tested it.
>> not sure if following is desired behavior. first run with explain,
>> then run with explain(summary on).
>> the second time,  Planning Memory: 0 bytes.
>>
>> regression=# PREPARE q4 AS SELECT 1 AS a;
>> explain EXECUTE q4;
>>                 QUERY PLAN
>> ------------------------------------------
>>  Result  (cost=0.00..0.01 rows=1 width=4)
>> (1 row)
>>
>> regression=# explain(summary on) EXECUTE q4;
>>                 QUERY PLAN
>> ------------------------------------------
>>  Result  (cost=0.00..0.01 rows=1 width=4)
>>  Planning Time: 0.009 ms
>>  Planning Memory: 0 bytes
>> (3 rows)
>> ---------------------------------------------
>
> Yes. This is expected since the plan is already available and no
> memory is required to fetch it from the cache. I imagine, if there
> were parameters to the prepared plan, it would consume some memory to
> evaluate those parameters and some more memory if replanning was
> required.
>
>
>> previously, if you want stats of a given memory context and its
>> children, you can only use MemoryContextStatsDetail.
>> but it will only go to stderr or LOG_SERVER_ONLY.
>> Now, MemoryContextMemUsed is being exposed. I can do something like:
>>
>> mem_consumed = MemoryContextMemUsed(CurrentMemoryContext);
>> //do stuff.
>> mem_consumed = MemoryContextMemUsed(CurrentMemoryContext) - mem_consumed;
>>
>> it will give me the NET memory consumed by doing staff in between. Is
>> my understanding correct?
>
> Yes.
>
> Here are three patches based on the latest master.
>
> 0001
> ====
> this is same as the previous patch with few things fixed. 1. Call
> MemoryContextMemUsed() before INSTR_TIME_SET_CURRENT so that the time
> taken by MemoryContextMemUsed() is not counted in planning time. 2. In
> ExplainOnePlan, use a separate code block for reporting memory.
>
> 0002
> ====
> This patch reports both memory allocated and memory used in the
> CurrentMemoryContext at the time of planning. It converts "Planning
> Memory" into a section with two values reported as "used" and
> "allocated" as below
>
> #explain (summary on) select * from pg_class c, pg_type t where
> c.reltype = t.oid;
>                                 QUERY PLAN
> --------------------------------------------------------------------------
>  Hash Join  (cost=28.84..47.08 rows=414 width=533)
>    ... snip ...
>  Planning Time: 9.274 ms
>  Planning Memory: used=80848 bytes allocated=90112 bytes
> (7 rows)
>
> In JSON format
> #explain (summary on, format json) select * from pg_class c, pg_type t
> where c.reltype = t.oid;
>                   QUERY PLAN
> -----------------------------------------------
>  [                                            +
>    {                                          +
>      "Plan": {                                +
>       ... snip ...
>      },                                       +
>      "Planning Time": 0.466,                  +
>      "Planning Memory": {                     +
>        "Used": 80608,                         +
>        "Allocated": 90112                     +
>      }                                        +
>    }                                          +
>  ]
> (1 row)
>
> PFA explain and explain analyze output in all the formats.
>
> The patch adds MemoryContextMemConsumed() which is similar to
> MemoryContextStats() or MemoryContextStatsDetails() except 1. the
> later always prints the memory statistics to either stderr or to the
> server error log and 2. it doesn't return MemoryContextCounters that
> it gathered. We should probably change MemoryContextStats or
> MemoryContextStatsDetails() according to those two points and not add
> MemoryContextMemConsumed().
>
> I have not merged this into 0001 yet. But once we agree upon whether
> this is the right thing to do, I will merge it into 0001.
>
> 0003
> ====
> When reporting memory allocated, a confusion may arise as to whether
> to report the "net" memory allocated between start and end of planner
> OR only the memory that remains allocated after end. This confusion
> can be avoided by using an exclusive memory context (child of
> CurrentMemoryContext) for planning. That's what 0003 implements as
> suggested by David. As mentioned in one of the comments in the patch,
> in order to measure memory allocated accurately the new MemoryContext
> has to be of the same type as the memory context that will be used
> otherwise by the planner i.e. CurrentMemoryContext, and should use the
> same parameters. IIUC, it will always be AllocSet. So the patch uses
> AllocSet and Asserts so. But in case this changes in future, we will
> need a way to create a new memory context with the same properties as
> the CurrentMemoryContext, a functionality missing right now. Once we
> agree upon the approach, the patch will need to be merged into 0002
> and in turn 0001.