Обсуждение: Re: 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. https://www.postgresql.org/message-id/20200402054120.GC14618@telsasoft.com https://www.postgresql.org/message-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: https://www.postgresql.org/message-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. > https://www.postgresql.org/message-id/20200402054120.GC14618@telsasoft.com > https://www.postgresql.org/message-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. > > https://www.postgresql.org/message-id/20200402054120.GC14618@telsasoft.com > > https://www.postgresql.org/message-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. > > > https://www.postgresql.org/message-id/20200402054120.GC14618@telsasoft.com > > > https://www.postgresql.org/message-id/20200407042521.GH2228@telsasoft.com > > > > > > "Memory: used=%lld bytes allocated=%lld bytes", > > > vs > > > "Buckets: %d (originally %d) Batches: %d (originally %d) MemoryUsage: %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