pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption

Up to now, the explain " " (two space) format is not mixed with "=".

And, other places which show "Memory" do not use "=". David will
remember prior discussions.
/messages/by-id/20200402054120.GC14618@telsasoft.com
/messages/by-id/20200407042521.GH2228@telsasoft.com

"Memory: used=%lld bytes allocated=%lld bytes",
vs
"Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",

There was some discussion about "bytes" - maybe it should instead show
kB?

(Also, I first thought that "peek" should be "peak", but eventually I
realized that's it's as intended.)

On Fri, Jan 12, 2024 at 10:53:08PM +0530, Abhijit Menon-Sen wrote:

(Those "bytes" look slightly odd to me in the midst of all the x=y
pieces, but that's probably not worth thinking about.)

On Mon, Jan 29, 2024 at 04:55:27PM +0000, Alvaro Herrera wrote:

Add EXPLAIN (MEMORY) to report planner memory consumption

This adds a new "Memory:" line under the "Planning:" group (which
currently only has "Buffers:") when the MEMORY option is specified.

In order to make the reporting reasonably accurate, we create a separate
memory context for planner activities, to be used only when this option
is given. The total amount of memory allocated by that context is
reported as "allocated"; we subtract memory in the context's freelists
from that and report that result as "used". We use
MemoryContextStatsInternal() to obtain the quantities.

The code structure to show buffer usage during planning was not in
amazing shape, so I (�lvaro) modified the patch a bit to clean that up
in passing.

Author: Ashutosh Bapat
Reviewed-by: David Rowley, Andrey Lepikhov, Jian He, Andy Fan
Discussion: /messages/by-id/CAExHW5sZA=5LJ_ZPpRO-w09ck8z9p7eaYAqq3Ks9GDfhrxeWBw@mail.gmail.com

On Tue, Feb 6, 2024 at 3:51 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

Up to now, the explain " " (two space) format is not mixed with "=".

And, other places which show "Memory" do not use "=". David will
remember prior discussions.
/messages/by-id/20200402054120.GC14618@telsasoft.com
/messages/by-id/20200407042521.GH2228@telsasoft.com

"Memory: used=%lld bytes allocated=%lld bytes",
vs
"Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",

I have used = to be consistent with Buffers usage in the same Planning section.

Are you suggesting that
"Memory: used=%lld bytes allocated=%lld bytes",
should be used instead of
"Memory: used=%lld bytes allocated=%lld bytes",
Please notice the single vs double space.

I am fine with this.

There was some discussion about "bytes" - maybe it should instead show
kB?

So EXPLAIN (memory) on a prepared statement may report memory less
than 1kB in which case bytes is a better unit. Planner may consume as
less as few kBs of memory, reporting which in kBs would be lossy.

(Also, I first thought that "peek" should be "peak", but eventually I
realized that's it's as intended.)

Don't understand the context. But probably it doesn't matter.

--
Best Wishes,
Ashutosh Bapat

Many thanks, Justin, for the post-commit review.

On 2024-Feb-06, Ashutosh Bapat wrote:

On Tue, Feb 6, 2024 at 3:51 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

Up to now, the explain " " (two space) format is not mixed with "=".

And, other places which show "Memory" do not use "=". David will
remember prior discussions.
/messages/by-id/20200402054120.GC14618@telsasoft.com
/messages/by-id/20200407042521.GH2228@telsasoft.com

"Memory: used=%lld bytes allocated=%lld bytes",
vs
"Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",

I have used = to be consistent with Buffers usage in the same Planning section.

Are you suggesting that
"Memory: used=%lld bytes allocated=%lld bytes",
should be used instead of
"Memory: used=%lld bytes allocated=%lld bytes",
Please notice the single vs double space.

I think using a single space here would be confusing. It's not a
problem for show_wal_usage because that one doesn't print units.
I don't know it was you (Ashutosh) or I that put the double space, but I
considered the matter and determined that two were better.

In the new line we have two different separators (: and =) because there
are two levels of info being presented; in the show_hash_info one we
have only one type of separator.

I'm not saying this is final and definite. I'm just saying I did
consider this whole format issue and what you see is the conclusion I
reached. It may or may not be what Ashutosh submitted -- I don't
remember. As committer, I almost always tweak submitted patches, and I
won't apologize for that. Further patches to correct my mistakes and
bad decisions always welcome.

(Also, I first thought that "peek" should be "peak", but eventually I
realized that's it's as intended.)

Don't understand the context. But probably it doesn't matter.

Source code always matters. Why would people spend so much time fixing
typos otherwise?

src/backend/commands/explain.c:
static bool peek_buffer_usage(ExplainState *es, const BufferUsage *usage);

We don't want to know what the "peak" buffer usage is, but we want to
"peek" whether buffer usage would print anything. I did have to spent a
minute thinking what the correct spelling was, here (but my English is
almost exclusively read/written, not spoken. Condolencies if you've had
to suffer my spoken English at some conference or whatever). I didn't
look at the dictionary back then, but now I do:
https://www.merriam-webster.com/dictionary/peek
As Justin says, it's the right word.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"After a quick R of TFM, all I can say is HOLY CR** THAT IS COOL! PostgreSQL was
amazing when I first started using it at 7.2, and I'm continually astounded by
learning new features and techniques made available by the continuing work of
the development team."
Berend Tober, http://archives.postgresql.org/pgsql-hackers/2007-08/msg01009.php

On Wed, Feb 7, 2024 at 3:43 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

Many thanks, Justin, for the post-commit review.

On 2024-Feb-06, Ashutosh Bapat wrote:

On Tue, Feb 6, 2024 at 3:51 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

Up to now, the explain " " (two space) format is not mixed with "=".

And, other places which show "Memory" do not use "=". David will
remember prior discussions.
/messages/by-id/20200402054120.GC14618@telsasoft.com
/messages/by-id/20200407042521.GH2228@telsasoft.com

"Memory: used=%lld bytes allocated=%lld bytes",
vs
"Buckets: %d (originally %d) Batches: %d (originally %d) Memory Usage: %ldkB\n",

I have used = to be consistent with Buffers usage in the same Planning section.

Are you suggesting that
"Memory: used=%lld bytes allocated=%lld bytes",
should be used instead of
"Memory: used=%lld bytes allocated=%lld bytes",
Please notice the single vs double space.

I think using a single space here would be confusing. It's not a
problem for show_wal_usage because that one doesn't print units.
I don't know it was you (Ashutosh) or I that put the double space, but I
considered the matter and determined that two were better.

In the new line we have two different separators (: and =) because there
are two levels of info being presented; in the show_hash_info one we
have only one type of separator.

I'm not saying this is final and definite. I'm just saying I did
consider this whole format issue and what you see is the conclusion I
reached. It may or may not be what Ashutosh submitted -- I don't
remember. As committer, I almost always tweak submitted patches, and I
won't apologize for that. Further patches to correct my mistakes and
bad decisions always welcome.

I don't have a preference myself. But now that you explain it, two
spaces between unit and next entity does seem a better choice. I had
used ",", which faced a minor objection. Thanks for that modification.
I failed to notice it in the beginning. Sorry.

(Also, I first thought that "peek" should be "peak", but eventually I
realized that's it's as intended.)

Don't understand the context. But probably it doesn't matter.

Source code always matters. Why would people spend so much time fixing
typos otherwise?

src/backend/commands/explain.c:
static bool peek_buffer_usage(ExplainState *es, const BufferUsage *usage);

Ah! Thanks for sharing the context. Without that context, I didn't
understand Justin's comment. I had reviewed this change post-commit
and knew very much that its peek and not peak. I also note that it's
better than show_planning :).

--
Best Wishes,
Ashutosh Bapat