explain format json, unit for serialize and memory are different.

Started by jian heover 1 year ago14 messages
#1jian he
jian.universality@gmail.com

hi.

explain(analyze, format json, serialize, memory, costs off, Timing
off) select * from tenk1;
QUERY PLAN
---------------------------------
[
{
"Plan": {
"Node Type": "Seq Scan",
"Parallel Aware": false,
"Async Capable": false,
"Relation Name": "tenk1",
"Alias": "tenk1",
"Actual Rows": 10000,
"Actual Loops": 1
},
"Planning": {
"Memory Used": 23432,
"Memory Allocated": 65536
},
"Planning Time": 0.290,
"Triggers": [
],
"Serialization": {
"Output Volume": 1143,
"Format": "text"
},
"Execution Time": 58.814
}
]

explain(analyze, format text, serialize, memory, costs off, Timing
off) select * from tenk1;
QUERY PLAN
---------------------------------------------------
Seq Scan on tenk1 (actual rows=10000 loops=1)
Planning:
Memory: used=23432 bytes allocated=65536 bytes
Planning Time: 0.289 ms
Serialization: output=1143kB format=text
Execution Time: 58.904 ms

under format json, "Output Volume": 1143,
1143 is kiB unit, and is not the same as "Memory Used" or "Memory
Allocated" byte unit.

Do we need to convert it to byte for the non-text format option for EXPLAIN?

#2Daniel Gustafsson
daniel@yesql.se
In reply to: jian he (#1)
Re: explain format json, unit for serialize and memory are different.

On 13 May 2024, at 11:16, jian he <jian.universality@gmail.com> wrote:

under format json, "Output Volume": 1143,
1143 is kiB unit, and is not the same as "Memory Used" or "Memory
Allocated" byte unit.

Nice catch.

Do we need to convert it to byte for the non-text format option for EXPLAIN?

Since json (and yaml/xml) is intended to be machine-readable I think we use a
single unit for all values, and document this fact.

--
Daniel Gustafsson

#3Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#2)
Re: explain format json, unit for serialize and memory are different.

On Mon, May 13, 2024 at 11:22:08AM +0200, Daniel Gustafsson wrote:

Since json (and yaml/xml) is intended to be machine-readable I think we use a
single unit for all values, and document this fact.

Agreed with the documentation gap. Another thing that could be worth
considering is to add the units aside with the numerical values, say:
"Memory Used": {"value": 23432, "units": "bytes"}

That would require changing ExplainProperty() so as the units are
showed in some shape, while still being readable when parsed. I
wouldn't recommend doing that in v17, but perhaps v18 could do better?

Units are also ignored for the XML and yaml outputs.
--
Michael

#4David Rowley
dgrowleyml@gmail.com
In reply to: Michael Paquier (#3)
Re: explain format json, unit for serialize and memory are different.

On Tue, 14 May 2024 at 17:40, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, May 13, 2024 at 11:22:08AM +0200, Daniel Gustafsson wrote:

Since json (and yaml/xml) is intended to be machine-readable I think we use a
single unit for all values, and document this fact.

Agreed with the documentation gap. Another thing that could be worth
considering is to add the units aside with the numerical values, say:
"Memory Used": {"value": 23432, "units": "bytes"}

That would require changing ExplainProperty() so as the units are
showed in some shape, while still being readable when parsed. I
wouldn't recommend doing that in v17, but perhaps v18 could do better?

I think for v17, we should consider adding a macro to explain.c to
calculate the KB from bytes. There are other inconsistencies that it
would be good to address. We normally round up to the nearest kilobyte
with (bytes + 1023) / 1024, but if you look at what 06286709e did, it
seems to be rounding to the nearest without any apparent justification
as to why. It does (metrics->bytesSent + 512) / 1024.

show_memory_counters() could be modified to use the macro and show
kilobytes rather than bytes.

David

#5David Rowley
dgrowleyml@gmail.com
In reply to: David Rowley (#4)
1 attachment(s)
Re: explain format json, unit for serialize and memory are different.

On Tue, 14 May 2024 at 18:16, David Rowley <dgrowleyml@gmail.com> wrote:

I think for v17, we should consider adding a macro to explain.c to
calculate the KB from bytes. There are other inconsistencies that it
would be good to address. We normally round up to the nearest kilobyte
with (bytes + 1023) / 1024, but if you look at what 06286709e did, it
seems to be rounding to the nearest without any apparent justification
as to why. It does (metrics->bytesSent + 512) / 1024.

show_memory_counters() could be modified to use the macro and show
kilobytes rather than bytes.

Here's a patch for that.

I checked the EXPLAIN SERIALIZE thread and didn't see any mention of
the + 512 thing. It seems Tom added it just before committing and no
patch ever made it to the mailing list with + 512. The final patch on
the list is in [1]/messages/by-id/CAEze2WhopAFRS4xdNtma6XtYCRqydPWAg83jx8HZTowpeXzOyg@mail.gmail.com.

For the EXPLAIN MEMORY part, the bytes vs kB wasn't discussed. The
closest the thread came to that was what Abhijit mentioned in [2]/messages/by-id/ZaF1fB_hMqycSq-S@toroid.org.

I also adjusted some inconsistencies around spaces between the digits
and kB. In other places in EXPLAIN we write "100kB" not "100 kB". I
see we print times with a space ("Execution Time: 1.719 ms"), so we're
not very consistent overall, but since the EXPLAIN MEMORY is new, it
makes sense to change it now to be aligned to the other kB stuff in
explain.c

The patch does change a long into an int64 in show_hash_info(). I
wondered if that part should be backpatched. It does not seem very
robust to me to divide a Size by 1024 and expect it to fit into a
long. With MSVC 64 bit, sizeof(Size) == 8 and sizeof(long) == 4. I
understand work_mem is limited to 2GB on that platform, but it does
not seem like a good reason to use a long.

David

[1]: /messages/by-id/CAEze2WhopAFRS4xdNtma6XtYCRqydPWAg83jx8HZTowpeXzOyg@mail.gmail.com
[2]: /messages/by-id/ZaF1fB_hMqycSq-S@toroid.org

Attachments:

v1-0001-Minor-fixes-to-EXPLAIN.patchapplication/octet-stream; name=v1-0001-Minor-fixes-to-EXPLAIN.patchDownload
From 4aa7527106d08027f69c1eff7c1a44530ef4db1b Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Tue, 14 May 2024 22:24:23 +1200
Subject: [PATCH v1] Minor fixes to EXPLAIN

---
 src/backend/commands/explain.c        | 34 ++++++++++++++++-----------
 src/test/regress/expected/explain.out |  6 ++---
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c0c73aa3c9..fbd2c6fa9d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -62,6 +62,12 @@ typedef struct SerializeMetrics
 #define X_CLOSE_IMMEDIATE 2
 #define X_NOWHITESPACE 4
 
+/*
+ * Various places within need to convert bytes to kilobytes.  Round these up
+ * to the next whole kilobyte.
+ */
+#define BYTES_TO_KILOBYTES(b) (((b) + 1023) / 1024)
+
 static void ExplainOneQuery(Query *query, int cursorOptions,
 							IntoClause *into, ExplainState *es,
 							const char *queryString, ParamListInfo params,
@@ -1120,11 +1126,11 @@ ExplainPrintSerialize(ExplainState *es, SerializeMetrics *metrics)
 		if (es->timing)
 			appendStringInfo(es->str, "Serialization: time=%.3f ms  output=" UINT64_FORMAT "kB  format=%s\n",
 							 1000.0 * INSTR_TIME_GET_DOUBLE(metrics->timeSpent),
-							 (metrics->bytesSent + 512) / 1024,
+							 BYTES_TO_KILOBYTES(metrics->bytesSent),
 							 format);
 		else
 			appendStringInfo(es->str, "Serialization: output=" UINT64_FORMAT "kB  format=%s\n",
-							 (metrics->bytesSent + 512) / 1024,
+							 BYTES_TO_KILOBYTES(metrics->bytesSent),
 							 format);
 
 		if (es->buffers && peek_buffer_usage(es, &metrics->bufferUsage))
@@ -1141,7 +1147,7 @@ ExplainPrintSerialize(ExplainState *es, SerializeMetrics *metrics)
 								 1000.0 * INSTR_TIME_GET_DOUBLE(metrics->timeSpent),
 								 3, es);
 		ExplainPropertyUInteger("Output Volume", "kB",
-								(metrics->bytesSent + 512) / 1024, es);
+								BYTES_TO_KILOBYTES(metrics->bytesSent), es);
 		ExplainPropertyText("Format", format, es);
 		if (es->buffers)
 			show_buffer_usage(es, &metrics->bufferUsage);
@@ -3275,7 +3281,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 
 	if (hinstrument.nbatch > 0)
 	{
-		long		spacePeakKb = (hinstrument.space_peak + 1023) / 1024;
+		int64		spacePeakKb = BYTES_TO_KILOBYTES(hinstrument.space_peak);
 
 		if (es->format != EXPLAIN_FORMAT_TEXT)
 		{
@@ -3295,7 +3301,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 		{
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: %ldkB\n",
+							 "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: " INT64_FORMAT "kB\n",
 							 hinstrument.nbuckets,
 							 hinstrument.nbuckets_original,
 							 hinstrument.nbatch,
@@ -3306,7 +3312,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 		{
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d  Batches: %d  Memory Usage: %ldkB\n",
+							 "Buckets: %d  Batches: %d  Memory Usage: " INT64_FORMAT "kB\n",
 							 hinstrument.nbuckets, hinstrument.nbatch,
 							 spacePeakKb);
 		}
@@ -3376,9 +3382,9 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
 		 * when mem_peak is 0.
 		 */
 		if (mstate->stats.mem_peak > 0)
-			memPeakKb = (mstate->stats.mem_peak + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(mstate->stats.mem_peak);
 		else
-			memPeakKb = (mstate->mem_used + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(mstate->mem_used);
 
 		if (es->format != EXPLAIN_FORMAT_TEXT)
 		{
@@ -3427,7 +3433,7 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
 		 * MemoizeInstrumentation.mem_peak field for us.  No need to do the
 		 * zero checks like we did for the serial case above.
 		 */
-		memPeakKb = (si->mem_peak + 1023) / 1024;
+		memPeakKb = BYTES_TO_KILOBYTES(si->mem_peak);
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
@@ -3464,7 +3470,7 @@ static void
 show_hashagg_info(AggState *aggstate, ExplainState *es)
 {
 	Agg		   *agg = (Agg *) aggstate->ss.ps.plan;
-	int64		memPeakKb = (aggstate->hash_mem_peak + 1023) / 1024;
+	int64		memPeakKb = BYTES_TO_KILOBYTES(aggstate->hash_mem_peak);
 
 	if (agg->aggstrategy != AGG_HASHED &&
 		agg->aggstrategy != AGG_MIXED)
@@ -3545,7 +3551,7 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
 				continue;
 			hash_disk_used = sinstrument->hash_disk_used;
 			hash_batches_used = sinstrument->hash_batches_used;
-			memPeakKb = (sinstrument->hash_mem_peak + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(sinstrument->hash_mem_peak);
 
 			if (es->workers_state)
 				ExplainOpenWorker(n, es);
@@ -3946,9 +3952,9 @@ show_memory_counters(ExplainState *es, const MemoryContextCounters *mem_counters
 	{
 		ExplainIndentText(es);
 		appendStringInfo(es->str,
-						 "Memory: used=%lld bytes  allocated=%lld bytes",
-						 (long long) (mem_counters->totalspace - mem_counters->freespace),
-						 (long long) mem_counters->totalspace);
+						 "Memory: used=%zukB  allocated=%zukB",
+						 BYTES_TO_KILOBYTES(mem_counters->totalspace - mem_counters->freespace),
+						 BYTES_TO_KILOBYTES(mem_counters->totalspace));
 		appendStringInfoChar(es->str, '\n');
 	}
 	else
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 703800d856..6585c6a69e 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -345,14 +345,14 @@ select explain_filter('explain (memory) select * from int8_tbl i8');
                      explain_filter                      
 ---------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
 (2 rows)
 
 select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
                                         explain_filter                                         
 -----------------------------------------------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
  Planning Time: N.N ms
  Execution Time: N.N ms
 (4 rows)
@@ -413,7 +413,7 @@ select explain_filter('explain (memory) execute int8_query');
                      explain_filter                      
 ---------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
 (2 rows)
 
 -- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- 
2.34.1

#6jian he
jian.universality@gmail.com
In reply to: David Rowley (#5)
Re: explain format json, unit for serialize and memory are different.

On Tue, May 14, 2024 at 6:33 PM David Rowley <dgrowleyml@gmail.com> wrote:

On Tue, 14 May 2024 at 18:16, David Rowley <dgrowleyml@gmail.com> wrote:

I think for v17, we should consider adding a macro to explain.c to
calculate the KB from bytes. There are other inconsistencies that it
would be good to address. We normally round up to the nearest kilobyte
with (bytes + 1023) / 1024, but if you look at what 06286709e did, it
seems to be rounding to the nearest without any apparent justification
as to why. It does (metrics->bytesSent + 512) / 1024.

show_memory_counters() could be modified to use the macro and show
kilobytes rather than bytes.

Here's a patch for that.

I checked the EXPLAIN SERIALIZE thread and didn't see any mention of
the + 512 thing. It seems Tom added it just before committing and no
patch ever made it to the mailing list with + 512. The final patch on
the list is in [1].

For the EXPLAIN MEMORY part, the bytes vs kB wasn't discussed. The
closest the thread came to that was what Abhijit mentioned in [2].

static void
show_memory_counters(ExplainState *es, const MemoryContextCounters
*mem_counters)
{
if (es->format == EXPLAIN_FORMAT_TEXT)
{
ExplainIndentText(es);
appendStringInfo(es->str,
"Memory: used=%zukB allocated=%zukB",
BYTES_TO_KILOBYTES(mem_counters->totalspace - mem_counters->freespace),
BYTES_TO_KILOBYTES(mem_counters->totalspace));
appendStringInfoChar(es->str, '\n');
}
else
{
ExplainPropertyInteger("Memory Used", "bytes",
mem_counters->totalspace - mem_counters->freespace,
es);
ExplainPropertyInteger("Memory Allocated", "bytes",
mem_counters->totalspace, es);
}
}

the "else" branch, also need to apply BYTES_TO_KILOBYTES marco?
otherwise, it's inconsistent?

I also adjusted some inconsistencies around spaces between the digits
and kB. In other places in EXPLAIN we write "100kB" not "100 kB". I
see we print times with a space ("Execution Time: 1.719 ms"), so we're
not very consistent overall, but since the EXPLAIN MEMORY is new, it
makes sense to change it now to be aligned to the other kB stuff in
explain.c

The patch does change a long into an int64 in show_hash_info(). I
wondered if that part should be backpatched. It does not seem very
robust to me to divide a Size by 1024 and expect it to fit into a
long. With MSVC 64 bit, sizeof(Size) == 8 and sizeof(long) == 4. I
understand work_mem is limited to 2GB on that platform, but it does
not seem like a good reason to use a long.

I also checked output
from function show_incremental_sort_group_info and show_sort_info,
the "kB" usage is consistent.

#7David Rowley
dgrowleyml@gmail.com
In reply to: jian he (#6)
1 attachment(s)
Re: explain format json, unit for serialize and memory are different.

On Wed, 15 May 2024 at 01:18, jian he <jian.universality@gmail.com> wrote:

else
{
ExplainPropertyInteger("Memory Used", "bytes",
mem_counters->totalspace - mem_counters->freespace,
es);
ExplainPropertyInteger("Memory Allocated", "bytes",
mem_counters->totalspace, es);
}
}

the "else" branch, also need to apply BYTES_TO_KILOBYTES marco?

Yeah, I missed that. Here's another patch.

Thanks for looking.

David

Attachments:

v2-0001-Minor-fixes-to-EXPLAIN.patchapplication/octet-stream; name=v2-0001-Minor-fixes-to-EXPLAIN.patchDownload
From b9cae40446f69dee43b7c66d253768959ac740ea Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Tue, 14 May 2024 22:24:23 +1200
Subject: [PATCH v2] Minor fixes to EXPLAIN

---
 src/backend/commands/explain.c        | 48 +++++++++++++++------------
 src/test/regress/expected/explain.out |  6 ++--
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c0c73aa3c9..c4b0acfa7c 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -62,6 +62,12 @@ typedef struct SerializeMetrics
 #define X_CLOSE_IMMEDIATE 2
 #define X_NOWHITESPACE 4
 
+/*
+ * Various places within need to convert bytes to kilobytes.  Round these up
+ * to the next whole kilobyte.
+ */
+#define BYTES_TO_KILOBYTES(b) (((b) + 1023) / 1024)
+
 static void ExplainOneQuery(Query *query, int cursorOptions,
 							IntoClause *into, ExplainState *es,
 							const char *queryString, ParamListInfo params,
@@ -1120,11 +1126,11 @@ ExplainPrintSerialize(ExplainState *es, SerializeMetrics *metrics)
 		if (es->timing)
 			appendStringInfo(es->str, "Serialization: time=%.3f ms  output=" UINT64_FORMAT "kB  format=%s\n",
 							 1000.0 * INSTR_TIME_GET_DOUBLE(metrics->timeSpent),
-							 (metrics->bytesSent + 512) / 1024,
+							 BYTES_TO_KILOBYTES(metrics->bytesSent),
 							 format);
 		else
 			appendStringInfo(es->str, "Serialization: output=" UINT64_FORMAT "kB  format=%s\n",
-							 (metrics->bytesSent + 512) / 1024,
+							 BYTES_TO_KILOBYTES(metrics->bytesSent),
 							 format);
 
 		if (es->buffers && peek_buffer_usage(es, &metrics->bufferUsage))
@@ -1141,7 +1147,7 @@ ExplainPrintSerialize(ExplainState *es, SerializeMetrics *metrics)
 								 1000.0 * INSTR_TIME_GET_DOUBLE(metrics->timeSpent),
 								 3, es);
 		ExplainPropertyUInteger("Output Volume", "kB",
-								(metrics->bytesSent + 512) / 1024, es);
+								BYTES_TO_KILOBYTES(metrics->bytesSent), es);
 		ExplainPropertyText("Format", format, es);
 		if (es->buffers)
 			show_buffer_usage(es, &metrics->bufferUsage);
@@ -3275,7 +3281,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 
 	if (hinstrument.nbatch > 0)
 	{
-		long		spacePeakKb = (hinstrument.space_peak + 1023) / 1024;
+		uint64		spacePeakKb = BYTES_TO_KILOBYTES(hinstrument.space_peak);
 
 		if (es->format != EXPLAIN_FORMAT_TEXT)
 		{
@@ -3287,15 +3293,15 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 								   hinstrument.nbatch, es);
 			ExplainPropertyInteger("Original Hash Batches", NULL,
 								   hinstrument.nbatch_original, es);
-			ExplainPropertyInteger("Peak Memory Usage", "kB",
-								   spacePeakKb, es);
+			ExplainPropertyUInteger("Peak Memory Usage", "kB",
+									spacePeakKb, es);
 		}
 		else if (hinstrument.nbatch_original != hinstrument.nbatch ||
 				 hinstrument.nbuckets_original != hinstrument.nbuckets)
 		{
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: %ldkB\n",
+							 "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: " UINT64_FORMAT "kB\n",
 							 hinstrument.nbuckets,
 							 hinstrument.nbuckets_original,
 							 hinstrument.nbatch,
@@ -3306,7 +3312,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 		{
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d  Batches: %d  Memory Usage: %ldkB\n",
+							 "Buckets: %d  Batches: %d  Memory Usage: " UINT64_FORMAT "kB\n",
 							 hinstrument.nbuckets, hinstrument.nbatch,
 							 spacePeakKb);
 		}
@@ -3376,9 +3382,9 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
 		 * when mem_peak is 0.
 		 */
 		if (mstate->stats.mem_peak > 0)
-			memPeakKb = (mstate->stats.mem_peak + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(mstate->stats.mem_peak);
 		else
-			memPeakKb = (mstate->mem_used + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(mstate->mem_used);
 
 		if (es->format != EXPLAIN_FORMAT_TEXT)
 		{
@@ -3427,7 +3433,7 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
 		 * MemoizeInstrumentation.mem_peak field for us.  No need to do the
 		 * zero checks like we did for the serial case above.
 		 */
-		memPeakKb = (si->mem_peak + 1023) / 1024;
+		memPeakKb = BYTES_TO_KILOBYTES(si->mem_peak);
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
@@ -3464,7 +3470,7 @@ static void
 show_hashagg_info(AggState *aggstate, ExplainState *es)
 {
 	Agg		   *agg = (Agg *) aggstate->ss.ps.plan;
-	int64		memPeakKb = (aggstate->hash_mem_peak + 1023) / 1024;
+	int64		memPeakKb = BYTES_TO_KILOBYTES(aggstate->hash_mem_peak);
 
 	if (agg->aggstrategy != AGG_HASHED &&
 		agg->aggstrategy != AGG_MIXED)
@@ -3545,7 +3551,7 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
 				continue;
 			hash_disk_used = sinstrument->hash_disk_used;
 			hash_batches_used = sinstrument->hash_batches_used;
-			memPeakKb = (sinstrument->hash_mem_peak + 1023) / 1024;
+			memPeakKb = BYTES_TO_KILOBYTES(sinstrument->hash_mem_peak);
 
 			if (es->workers_state)
 				ExplainOpenWorker(n, es);
@@ -3942,22 +3948,22 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
 static void
 show_memory_counters(ExplainState *es, const MemoryContextCounters *mem_counters)
 {
+	int64 memusedkB = BYTES_TO_KILOBYTES(mem_counters->totalspace -
+										 mem_counters->freespace);
+	int64 memallocatedkB = BYTES_TO_KILOBYTES(mem_counters->totalspace);
+
 	if (es->format == EXPLAIN_FORMAT_TEXT)
 	{
 		ExplainIndentText(es);
 		appendStringInfo(es->str,
-						 "Memory: used=%lld bytes  allocated=%lld bytes",
-						 (long long) (mem_counters->totalspace - mem_counters->freespace),
-						 (long long) mem_counters->totalspace);
+						 "Memory: used=" INT64_FORMAT "kB  allocated=" INT64_FORMAT "kB",
+						 memusedkB,  memallocatedkB);
 		appendStringInfoChar(es->str, '\n');
 	}
 	else
 	{
-		ExplainPropertyInteger("Memory Used", "bytes",
-							   mem_counters->totalspace - mem_counters->freespace,
-							   es);
-		ExplainPropertyInteger("Memory Allocated", "bytes",
-							   mem_counters->totalspace, es);
+		ExplainPropertyInteger("Memory Used", "kB", memusedkB, es);
+		ExplainPropertyInteger("Memory Allocated", "kB", memallocatedkB, es);
 	}
 }
 
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 703800d856..6585c6a69e 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -345,14 +345,14 @@ select explain_filter('explain (memory) select * from int8_tbl i8');
                      explain_filter                      
 ---------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
 (2 rows)
 
 select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
                                         explain_filter                                         
 -----------------------------------------------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
  Planning Time: N.N ms
  Execution Time: N.N ms
 (4 rows)
@@ -413,7 +413,7 @@ select explain_filter('explain (memory) execute int8_query');
                      explain_filter                      
 ---------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N)
-   Memory: used=N bytes  allocated=N bytes
+   Memory: used=NkB  allocated=NkB
 (2 rows)
 
 -- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- 
2.34.1

#8Michael Paquier
michael@paquier.xyz
In reply to: David Rowley (#7)
Re: explain format json, unit for serialize and memory are different.

On Wed, May 15, 2024 at 02:01:21AM +1200, David Rowley wrote:

Yeah, I missed that. Here's another patch.

Thanks for looking.

Thanks for bringing in a patch that makes the whole picture more
consistent across the board. When it comes to MEMORY, I can get
behind your suggestion to use kB and call it a day, while SERIALIZE
would apply the same conversion at 1023b.

It would be nice to document the units implied in the non-text
formats, rather than have the users guess what these are.

Perhaps Alvaro and Tom would like to chime in, as committers of
respectively 5de890e3610d and 06286709ee06?
--
Michael

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#8)
Re: explain format json, unit for serialize and memory are different.

Michael Paquier <michael@paquier.xyz> writes:

Perhaps Alvaro and Tom would like to chime in, as committers of
respectively 5de890e3610d and 06286709ee06?

No objection here. In a green field I might argue for
round-to-nearest instead of round-up, but it looks like we
have several precedents for round-up, so let's avoid changing
that existing behavior.

regards, tom lane

#10jian he
jian.universality@gmail.com
In reply to: Tom Lane (#9)
Re: explain format json, unit for serialize and memory are different.

explain (format json, analyze, wal, buffers, memory, serialize) insert
into tenk1 select * from tenk1 limit 1;
QUERY PLAN
-----------------------------------------------
[
{
"Plan": {
"Node Type": "ModifyTable",
"Operation": "Insert",
"Parallel Aware": false,
"Async Capable": false,
"Relation Name": "tenk1",
"Alias": "tenk1",
"Startup Cost": 0.00,
"Total Cost": 0.04,
"Plan Rows": 0,
"Plan Width": 0,
"Actual Startup Time": 0.030,
"Actual Total Time": 0.030,
"Actual Rows": 0,
"Actual Loops": 1,
"Shared Hit Blocks": 3,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0,
"WAL Records": 1,
"WAL FPI": 0,
"WAL Bytes": 299,
"Plans": [
{
"Node Type": "Limit",
"Parent Relationship": "Outer",
"Parallel Aware": false,
"Async Capable": false,
"Startup Cost": 0.00,
"Total Cost": 0.04,
"Plan Rows": 1,
"Plan Width": 244,
"Actual Startup Time": 0.011,
"Actual Total Time": 0.011,
"Actual Rows": 1,
"Actual Loops": 1,
"Shared Hit Blocks": 2,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0,
"WAL Records": 0,
"WAL FPI": 0,
"WAL Bytes": 0,
"Plans": [
{
"Node Type": "Seq Scan",
"Parent Relationship": "Outer",
"Parallel Aware": false,
"Async Capable": false,
"Relation Name": "tenk1",
"Alias": "tenk1_1",
"Startup Cost": 0.00,
"Total Cost": 445.00,
"Plan Rows": 10000,
"Plan Width": 244,
"Actual Startup Time": 0.009,
"Actual Total Time": 0.009,
"Actual Rows": 1,
"Actual Loops": 1,
"Shared Hit Blocks": 2,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0,
"WAL Records": 0,
"WAL FPI": 0,
"WAL Bytes": 0
}
]
}
]
},
"Planning": {
"Shared Hit Blocks": 0,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0,
"Memory Used": 68080,
"Memory Allocated": 131072
},
"Planning Time": 0.659,
"Triggers": [
],
"Serialization": {
"Time": 0.000,
"Output Volume": 0,
"Format": "text",
"Shared Hit Blocks": 0,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0
},
"Execution Time": 0.065
}
]

-------
"Shared Hit Blocks": 0,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0

these information duplicated for json key "Serialization" and json key
"Planning"
i am not sure this is intended?

#11David Rowley
dgrowleyml@gmail.com
In reply to: jian he (#10)
Re: explain format json, unit for serialize and memory are different.

On Wed, 15 May 2024 at 13:44, jian he <jian.universality@gmail.com> wrote:

"Shared Hit Blocks": 0,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0

these information duplicated for json key "Serialization" and json key
"Planning"
i am not sure this is intended?

Looks ok to me. Buffers used during planning are independent from the
buffers used when outputting rows to the client.

David

#12jian he
jian.universality@gmail.com
In reply to: David Rowley (#11)
Re: explain format json, unit for serialize and memory are different.

On Wed, May 15, 2024 at 10:13 AM David Rowley <dgrowleyml@gmail.com> wrote:

On Wed, 15 May 2024 at 13:44, jian he <jian.universality@gmail.com> wrote:

"Shared Hit Blocks": 0,
"Shared Read Blocks": 0,
"Shared Dirtied Blocks": 0,
"Shared Written Blocks": 0,
"Local Hit Blocks": 0,
"Local Read Blocks": 0,
"Local Dirtied Blocks": 0,
"Local Written Blocks": 0,
"Temp Read Blocks": 0,
"Temp Written Blocks": 0

these information duplicated for json key "Serialization" and json key
"Planning"
i am not sure this is intended?

Looks ok to me. Buffers used during planning are independent from the
buffers used when outputting rows to the client.

looking at serializeAnalyzeReceive.
I am not sure which part of serializeAnalyzeReceive will update pgBufferUsage.

I am looking for an example where this information under json key
"Serialization" is not zero.
So far I have tried:

create table s(a text);
insert into s select repeat('a', 1024) from generate_series(1,1024);
explain (format json, analyze, wal, buffers, memory, serialize, timing
off) select * from s;

#13David Rowley
dgrowleyml@gmail.com
In reply to: jian he (#12)
Re: explain format json, unit for serialize and memory are different.

On Wed, 15 May 2024 at 15:40, jian he <jian.universality@gmail.com> wrote:

I am looking for an example where this information under json key
"Serialization" is not zero.
So far I have tried:

Something that requires detoasting.

create table s(a text);
insert into s select repeat('a', 1024) from generate_series(1,1024);
explain (format json, analyze, wal, buffers, memory, serialize, timing
off) select * from s;

Something bigger than 1024 bytes or use SET STORAGE EXTERNAL or EXTENDED.

create table s(a text);
insert into s select repeat('a', 1024*1024) from generate_series(1,10);
explain (format text, analyze, buffers, serialize, timing off) select * from s;

Serialization: output=10241kB format=text
Buffers: shared hit=36

David

#14David Rowley
dgrowleyml@gmail.com
In reply to: Tom Lane (#9)
Re: explain format json, unit for serialize and memory are different.

On Wed, 15 May 2024 at 13:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael@paquier.xyz> writes:

Perhaps Alvaro and Tom would like to chime in, as committers of
respectively 5de890e3610d and 06286709ee06?

No objection here. In a green field I might argue for
round-to-nearest instead of round-up, but it looks like we
have several precedents for round-up, so let's avoid changing
that existing behavior.

Thanks. I've pushed the patch now.

David