public inbox for [email protected]
help / color / mirror / Atom feedpgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
5+ messages / 3 participants
[nested] [flat]
* pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
@ 2024-01-29 16:55 Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Alvaro Herrera @ 2024-01-29 16:55 UTC (permalink / raw)
To: [email protected]
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: https://www.postgresql.org/message-id/[email protected]...
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/5de890e3610d5a12cdaea36413d967cf5c544e20
Modified Files
--------------
contrib/auto_explain/auto_explain.c | 2 +
doc/src/sgml/ref/explain.sgml | 14 ++++
src/backend/commands/explain.c | 152 ++++++++++++++++++++++++++++------
src/backend/commands/prepare.c | 22 ++++-
src/backend/utils/mmgr/mcxt.c | 13 +++
src/include/commands/explain.h | 4 +-
src/include/utils/memutils.h | 2 +
src/test/regress/expected/explain.out | 76 +++++++++++++++++
src/test/regress/sql/explain.sql | 8 ++
9 files changed, 265 insertions(+), 28 deletions(-)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
@ 2024-02-05 22:21 Justin Pryzby <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Justin Pryzby @ 2024-02-05 22:21 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: [email protected]; David Rowley <[email protected]>; Ashutosh Bapat <[email protected]>
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.
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/[email protected]
"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: https://www.postgresql.org/message-id/[email protected]...
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
@ 2024-02-06 09:26 Ashutosh Bapat <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Ashutosh Bapat @ 2024-02-06 09:26 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: [email protected]; David Rowley <[email protected]>; Alvaro Herrera <[email protected]>
On Tue, Feb 6, 2024 at 3:51 AM Justin Pryzby <[email protected]> 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.
> https://www.postgresql.org/message-id/[email protected]
> https://www.postgresql.org/message-id/[email protected]
>
> "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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
@ 2024-02-07 10:13 Alvaro Herrera <[email protected]>
parent: Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Alvaro Herrera @ 2024-02-07 10:13 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: Justin Pryzby <[email protected]>; [email protected]; David Rowley <[email protected]>
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 <[email protected]> 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.
> > https://www.postgresql.org/message-id/[email protected]
> > https://www.postgresql.org/message-id/[email protected]
> >
> > "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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
@ 2024-02-07 10:39 Ashutosh Bapat <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Ashutosh Bapat @ 2024-02-07 10:39 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Justin Pryzby <[email protected]>; [email protected]; David Rowley <[email protected]>
On Wed, Feb 7, 2024 at 3:43 PM Alvaro Herrera <[email protected]> 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 <[email protected]> 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.
> > > https://www.postgresql.org/message-id/[email protected]
> > > https://www.postgresql.org/message-id/[email protected]
> > >
> > > "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
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-02-07 10:39 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-29 16:55 pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption Alvaro Herrera <[email protected]>
2024-02-05 22:21 ` Justin Pryzby <[email protected]>
2024-02-06 09:26 ` Ashutosh Bapat <[email protected]>
2024-02-07 10:13 ` Alvaro Herrera <[email protected]>
2024-02-07 10:39 ` Ashutosh Bapat <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox