Re: Report planning memory in EXPLAIN ANALYZE - Mailing list pgsql-hackers

On Mon, Aug 14, 2023 at 5:23 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 10 Aug 2023 at 20:33, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > My point is what's relevant here is how much net memory planner asked
> > for.
>
> But that's not what your patch is reporting. All you're reporting is
> the difference in memory that's *currently* palloc'd from before and
> after the planner ran.  If we palloc'd 600 exabytes then pfree'd it
> again, your metric won't change.

May be I didn't use the right phrase "asked for". But I expected that
to be read in the context of "net memory" - net as an adjective.
Anyway, I will make it more clear below.

>
> I'm struggling a bit to understand your goals here.  If your goal is
> to make a series of changes that reduces the amount of memory that's
> palloc'd at the end of planning, then your patch seems to suit that
> goal, but per the quote above, it seems you care about how many bytes
> are palloc'd during planning and your patch does not seem track that.

There are three metrics here. M1: The total number of bytes that the
planner "requests". M2: The total number of bytes that "remain
palloc'ed" at a given point in time. M3: Maximum number of bytes that
were palloc'ed at any point in time during planning. Following
sequence of operations will explain the difference
p1 palloc: 100 - M1 = 100, M2 = 100, M3 = 100
p2 palloc: 100, M1 = 200, M2 = 200, M3 = 200
p3 pfree: 100, M1 = 200, M2 = 100, M3 = 200
p4 palloc: 100, M1 = 300, M2 = 200, M3 = 200
p5 palloc: 100, M1 = 400, M2 = 300, M3 = 300
p6 pfree: 100, M1 = 400, M2 = 200, M3 = 300

The patch reports M2 at the end of planning.
MemoryContextData::mem_allocated is not exactly the same as M3 but
gives indication of M3.

My goal is to reduce all three M1, M2 and M3. I don't it's easy to
track M1 and also worth the complexity. M2 and M3 instead act as rough
indicators of M1.
>
> With your patch as it is, to improve the metric you're reporting we
> could go off and do things like pfree Paths once createplan.c is done,
> but really, why would we do that? Just to make the "Planning Memory"
> metric looks better doesn't seem like a worthy goal.
>

As I mentioned earlier the CurrentMemoryContext at the time of
planning is also used during query execution. There are other contexts
like per row, per operator contexts but anything which is specific to
the running query and does not fit those contexts is allocated in this
context. If we reduce memory that remains palloc'ed (M2) at the end of
the planning, it can be used during execution. So it looks like a
worthy goal.

> Instead, if we reported the context's mem_allocated, then it would
> give us the flexibility to make changes to the memory context code to
> have the metric look better. It might also alert us to planner
> inefficiencies and problems with new code that may cause a large spike
> in the amount of memory that gets allocated.

This is M3. I agree with your reasoning here. We should report M3 as
well. I will make changes to the patch.

> Now, I'm not saying we
> should add a patch that shows mem_allocated. I'm just questioning if
> your proposed patch meets the goals you're trying to achieve.  I just
> suggested that you might want to consider something else as a metric
> for your memory usage reduction work.

Ok. Thanks for the suggestions. More suggestions are welcome too.

[1] https://www.merriam-webster.com/dictionary/net

--
Best Wishes,
Ashutosh Bapat