On Tue, 16 Apr 2024 at 10:57, Andres Freund <and...@anarazel.de> wrote:
> I guess was thinking more about BumpIsEmpty() and BumpStats() then the "bogus"
> cases. But BumpIsEmpty() likely is unreachable as well.
The only call to MemoryContextIsEmpty() I see is AtSubCommit_Memory()
and it's on an aset.c context type. I see generation.c has the same
issue per [1].
> BumpStats() is
> reachable, but perhaps it's not worth it?
>
> BEGIN;
> DECLARE foo CURSOR FOR SELECT LEFT(a,10),b FROM (VALUES(REPEAT('a', 512 *
> 1024),1),(REPEAT('b', 512 * 1024),2)) v(a,b) ORDER BY v.a DESC;
> FETCH 1 FROM foo;
> SELECT * FROM pg_backend_memory_contexts WHERE name = 'Caller tuples';
I think primarily it's good to exercise that code just to make sure it
does not crash. Looking at the output of the above on my machine:
name | ident | parent | level | total_bytes |
total_nblocks | free_bytes | free_chunks | used_bytes
---------------+-------+----------------+-------+-------------+---------------+------------+-------------+------------
Caller tuples | | TupleSort sort | 6 | 1056848 |
3 | 8040 | 0 | 1048808
(1 row)
I'd say:
Name: stable
ident: stable
parent: stable
level: could change from a refactor of code
total_bytes: could be different on other platforms or dependent on
MEMORY_CONTEXT_CHECKING
total_nblocks: stable enough
free_bytes: could be different on other platforms or dependent on
MEMORY_CONTEXT_CHECKING
free_chunks: always 0
used_bytes: could be different on other platforms or dependent on
MEMORY_CONTEXT_CHECKING
I've attached a patch which includes your test with unstable columns
stripped out.
I cut the 2nd row down to just 512 bytes as I didn't see the need to
add two large datums. Annoyingly it still uses 3 blocks as I've opted
to do dlist_push_head(&set->blocks, &block->node); in BumpAllocLarge()
which is the block that's picked up again in BumpAlloc() per block =
dlist_container(BumpBlock, node, dlist_head_node(&set->blocks));
wonder if the large blocks should push tail instead.
> Hm, independent of this, seems a bit odd that we don't include the memory
> context type in pg_backend_memory_contexts?
That seems like useful information to include. It sure would be
useful to have in there to verify that I'm testing BumpStats(). I've
written a patch [2].
David
[1]
https://coverage.postgresql.org/src/backend/utils/mmgr/generation.c.gcov.html#997
[2]
https://postgr.es/m/CAApHDvrXX1OR09Zjb5TnB0AwCKze9exZN=9nxxg1zcvv8w-...@mail.gmail.com
diff --git a/src/test/regress/expected/sysviews.out
b/src/test/regress/expected/sysviews.out
index 9be7aca2b8..f537a1aeee 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -28,6 +28,26 @@ select name, ident, parent, level, total_bytes >= free_bytes
TopMemoryContext | | | 0 | t
(1 row)
+-- We can exercise some MemoryContext type stats functions. Most of the
+-- column values are too platform-dependant to display.
+begin;
+declare cur cursor for select left(a,10), b
+from (values(repeat('a', 512 * 1024),1),(repeat('b', 512),2)) v(a,b)
+order by v.a desc;
+fetch 1 from cur;
+ left | b
+------------+---
+ bbbbbbbbbb | 2
+(1 row)
+
+select name, parent, total_nblocks, free_chunks
+from pg_backend_memory_contexts where name = 'Caller tuples';
+ name | parent | total_nblocks | free_chunks
+---------------+----------------+---------------+-------------
+ Caller tuples | TupleSort sort | 3 | 0
+(1 row)
+
+rollback;
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/expected/tuplesort.out
b/src/test/regress/expected/tuplesort.out
index 0e8b5bf4a3..6dd97e7427 100644
--- a/src/test/regress/expected/tuplesort.out
+++ b/src/test/regress/expected/tuplesort.out
@@ -343,6 +343,19 @@ ORDER BY ctid DESC LIMIT 5;
(5 rows)
ROLLBACK;
+----
+-- test sorting of large datums VALUES
+----
+-- Ensure the order is correct and values look intact
+SELECT LEFT(a,10),b FROM
+ (VALUES(REPEAT('a', 512 * 1024),1),(REPEAT('b', 512 * 1024),2)) v(a,b)
+ORDER BY v.a DESC;
+ left | b
+------------+---
+ bbbbbbbbbb | 2
+ aaaaaaaaaa | 1
+(2 rows)
+
----
-- test forward and backward scans for in-memory and disk based tuplesort
----
diff --git a/src/test/regress/sql/sysviews.sql
b/src/test/regress/sql/sysviews.sql
index 6b4e24601d..000caaea21 100644
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -17,6 +17,17 @@ select count(*) >= 0 as ok from pg_available_extensions;
select name, ident, parent, level, total_bytes >= free_bytes
from pg_backend_memory_contexts where level = 0;
+-- We can exercise some MemoryContext type stats functions. Most of the
+-- column values are too platform-dependant to display.
+begin;
+declare cur cursor for select left(a,10), b
+from (values(repeat('a', 512 * 1024),1),(repeat('b', 512),2)) v(a,b)
+order by v.a desc;
+fetch 1 from cur;
+select name, parent, total_nblocks, free_chunks
+from pg_backend_memory_contexts where name = 'Caller tuples';
+rollback;
+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
diff --git a/src/test/regress/sql/tuplesort.sql
b/src/test/regress/sql/tuplesort.sql
index 658fe98dc5..8476e594e6 100644
--- a/src/test/regress/sql/tuplesort.sql
+++ b/src/test/regress/sql/tuplesort.sql
@@ -146,6 +146,15 @@ FROM abbrev_abort_uuids
ORDER BY ctid DESC LIMIT 5;
ROLLBACK;
+----
+-- test sorting of large datums VALUES
+----
+
+-- Ensure the order is correct and values look intact
+SELECT LEFT(a,10),b FROM
+ (VALUES(REPEAT('a', 512 * 1024),1),(REPEAT('b', 512 * 1024),2)) v(a,b)
+ORDER BY v.a DESC;
+
----
-- test forward and backward scans for in-memory and disk based tuplesort
----
