public inbox for [email protected]
help / color / mirror / Atom feedFrom: David Rowley <[email protected]>
To: Tom Lane <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
Date: Sun, 29 Mar 2026 21:43:16 +1300
Message-ID: <CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
On Sat, 28 Mar 2026 at 06:41, Tom Lane <[email protected]> wrote:
> In MEMORY_CONTEXT_CHECKING builds, we can cheaply detect double frees
> by using the existing behavior that requested_size is set to
> InvalidAllocSize during AllocSetFree. Another plausible idea is to
> change a freed chunk's MemoryContextMethodID to something invalid,
> which'd permit detection of double frees even in
> non-MEMORY_CONTEXT_CHECKING builds.
>
> I made draft patches showing how to do it both ways. (Both patches
> pass check-world and are able to detect the bug in v17.) The
> methodid-change way seems like the better alternative to me,
> but it is more invasive and does add a cycle or two when freeing or
> reusing a chunk.
I do think it's quite nice that we can detect the double free in
production builds by switching the MemoryContextMethodID to an unused
one. However, I did spend quite a bit of time making all that code as
fast as possible. For example, storing the freelist index in the chunk
header rather than the size, just to save the (pretty cheap)
AllocSetFreeIndex() call during pfree to get the freelist index from
the chunk size. That sort of thing was done because I could measure a
speedup from doing it.
For the switching MemoryContextMethodID patch, I applied the memory
context benchmarking patch I used when writing that code to test out
the overhead in a tight palloc/pfree loop (attached). I can see an
overhead of a little over 6.5%.
select run,pg_allocate_memory_test(8,512,1024::bigint*1024*1024,'aset')
as seconds from generate_Series(1,3) run;
master
run | seconds
-----+----------
1 | 0.823345
2 | 0.834834
3 | 0.835506
patched
run | seconds
-----+----------
1 | 0.887794
2 | 0.884866
3 | 0.88592
I would rather see us using the requested_size method in
MEMORY_CONTEXT_CHECKING enabled builds.
Thanks for working on the patches.
David
From 6b0a75fe8ecab6d0d248e789cb7e0b02d27b3833 Mon Sep 17 00:00:00 2001
From: David Rowley <[email protected]>
Date: Wed, 25 May 2022 14:24:37 +1200
Subject: [PATCH] Function to test palloc/pfree performance
---
src/backend/utils/adt/mcxtfuncs.c | 213 ++++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 12 ++
2 files changed, 225 insertions(+)
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 1a4dbbeb8db..89ba91d6de4 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -13,6 +13,8 @@
*-------------------------------------------------------------------------
*/
+#include <time.h>
+
#include "postgres.h"
#include "catalog/pg_type_d.h"
@@ -308,3 +310,214 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+typedef struct AllocateTestNext
+{
+ struct AllocateTestNext *next; /* ptr to the next allocation */
+} AllocateTestNext;
+
+/* #define ALLOCATE_TEST_DEBUG */
+/*
+ * pg_allocate_memory_test
+ * Used to test the performance of a memory context types
+ */
+Datum
+pg_allocate_memory_test(PG_FUNCTION_ARGS)
+{
+ int32 chunk_size = PG_GETARG_INT32(0);
+ int64 keep_memory = PG_GETARG_INT64(1);
+ int64 total_alloc = PG_GETARG_INT64(2);
+ text *context_type_text = PG_GETARG_TEXT_PP(3);
+ char *context_type;
+ int64 curr_memory_use = 0;
+ int64 remaining_alloc_bytes = total_alloc;
+ MemoryContext context;
+ MemoryContext oldContext;
+ AllocateTestNext *next_free_ptr = NULL;
+ AllocateTestNext *last_alloc = NULL;
+ clock_t start, end;
+
+ if (chunk_size < sizeof(AllocateTestNext))
+ elog(ERROR, "chunk_size (%d) must be at least %ld bytes", chunk_size,
+ sizeof(AllocateTestNext));
+ if (keep_memory > total_alloc)
+ elog(ERROR, "keep_memory (" INT64_FORMAT ") must be less than total_alloc (" INT64_FORMAT ")",
+ keep_memory, total_alloc);
+
+ context_type = text_to_cstring(context_type_text);
+
+ start = clock();
+
+ if (strcmp(context_type, "generation") == 0)
+ context = GenerationContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "aset") == 0)
+ context = AllocSetContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "slab") == 0)
+ context = SlabContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_MAXSIZE,
+ chunk_size);
+ else
+ elog(ERROR, "context_type must be \"generation\", \"aset\" or \"slab\"");
+
+ oldContext = MemoryContextSwitchTo(context);
+
+ while (remaining_alloc_bytes > 0)
+ {
+ AllocateTestNext *curr_alloc;
+
+ CHECK_FOR_INTERRUPTS();
+
+ /* Allocate the memory and update the counters */
+ curr_alloc = (AllocateTestNext *) palloc(chunk_size);
+ remaining_alloc_bytes -= chunk_size;
+ curr_memory_use += chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "alloc %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", curr_alloc, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ /*
+ * Point the last allocate to this one so that we can free allocations
+ * starting with the oldest first.
+ */
+ curr_alloc->next = NULL;
+ if (last_alloc != NULL)
+ last_alloc->next = curr_alloc;
+
+ if (next_free_ptr == NULL)
+ {
+ /*
+ * Remember the first chunk to free. We will follow the ->next
+ * pointers to find the next chunk to free when freeing memory
+ */
+ next_free_ptr = curr_alloc;
+ }
+
+ /*
+ * If the currently allocated memory has reached or exceeded the amount
+ * of memory we want to keep allocated at once then we'd better free
+ * some. Since all allocations are the same size we only need to free
+ * one allocation per loop.
+ */
+ if (curr_memory_use >= keep_memory)
+ {
+ AllocateTestNext *next = next_free_ptr->next;
+
+ /* free the memory and update the current memory usage */
+ pfree(next_free_ptr);
+ curr_memory_use -= chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "free %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", next_free_ptr, curr_memory_use, remaining_alloc_bytes);
+#endif
+ /* get the next chunk to free */
+ next_free_ptr = next;
+ }
+
+ if (curr_memory_use > 0)
+ last_alloc = curr_alloc;
+ else
+ last_alloc = NULL;
+ }
+
+ /* cleanup loop -- pfree remaining memory */
+ while (next_free_ptr != NULL)
+ {
+ AllocateTestNext *next = next_free_ptr->next;
+
+ /* free the memory and update the current memory usage */
+ pfree(next_free_ptr);
+ curr_memory_use -= chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "free %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", next_free_ptr, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ next_free_ptr = next;
+ }
+
+ MemoryContextSwitchTo(oldContext);
+
+ end = clock();
+
+ PG_RETURN_FLOAT8((double) (end - start) / CLOCKS_PER_SEC);
+}
+
+Datum
+pg_allocate_memory_test_reset(PG_FUNCTION_ARGS)
+{
+ int32 chunk_size = PG_GETARG_INT32(0);
+ int64 keep_memory = PG_GETARG_INT64(1);
+ int64 total_alloc = PG_GETARG_INT64(2);
+ text *context_type_text = PG_GETARG_TEXT_PP(3);
+ char *context_type;
+ int64 curr_memory_use = 0;
+ int64 remaining_alloc_bytes = total_alloc;
+ MemoryContext context;
+ MemoryContext oldContext;
+ clock_t start, end;
+
+ if (chunk_size < 1)
+ elog(ERROR, "size of chunk must be above 0");
+ if (keep_memory > total_alloc)
+ elog(ERROR, "keep_memory (" INT64_FORMAT ") must be less than total_alloc (" INT64_FORMAT ")",
+ keep_memory, total_alloc);
+
+ context_type = text_to_cstring(context_type_text);
+
+ start = clock();
+
+ if (strcmp(context_type, "generation") == 0)
+ context = GenerationContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "aset") == 0)
+ context = AllocSetContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "slab") == 0)
+ context = SlabContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_MAXSIZE,
+ chunk_size);
+ else
+ elog(ERROR, "context_type must be \"generation\", \"aset\" or \"slab\"");
+
+ oldContext = MemoryContextSwitchTo(context);
+
+ while (remaining_alloc_bytes > 0)
+ {
+ CHECK_FOR_INTERRUPTS();
+
+ /* Allocate the memory and update the counters */
+ (void) palloc(chunk_size);
+ remaining_alloc_bytes -= chunk_size;
+ curr_memory_use += chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "alloc %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", curr_alloc, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ /*
+ * If the currently allocated memory has reached or exceeded the amount
+ * of memory we want to keep allocated at once then reset the context.
+ */
+ if (curr_memory_use >= keep_memory)
+ {
+ curr_memory_use = 0;
+ MemoryContextReset(context);
+ }
+ }
+
+ MemoryContextSwitchTo(oldContext);
+ MemoryContextDelete(context);
+
+ end = clock();
+
+ PG_RETURN_FLOAT8((double) (end - start) / CLOCKS_PER_SEC);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0118e970dda..c43468da28b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8710,6 +8710,18 @@
prosrc => 'pg_log_backend_memory_contexts',
proacl => '{POSTGRES=X}' },
+# just for testing memory context allocation speed
+{ oid => '9319', descr => 'for testing performance of allocation and freeing',
+ proname => 'pg_allocate_memory_test', provolatile => 'v',
+ prorettype => 'float8', proargtypes => 'int4 int8 int8 text',
+ prosrc => 'pg_allocate_memory_test' },
+
+# just for testing memory context allocation speed
+{ oid => '9320', descr => 'for testing performance of allocation and resetting',
+ proname => 'pg_allocate_memory_test_reset', provolatile => 'v',
+ prorettype => 'float8', proargtypes => 'int4 int8 int8 text',
+ prosrc => 'pg_allocate_memory_test_reset' },
+
# non-persistent series generator
{ oid => '1066', descr => 'non-persistent series generator',
proname => 'generate_series', prorows => '1000',
--
2.51.0
Attachments:
[text/plain] 0001-Function-to-test-palloc-pfree-performance.patch.txt (8.2K, 2-0001-Function-to-test-palloc-pfree-performance.patch.txt)
download | inline diff:
From 6b0a75fe8ecab6d0d248e789cb7e0b02d27b3833 Mon Sep 17 00:00:00 2001
From: David Rowley <[email protected]>
Date: Wed, 25 May 2022 14:24:37 +1200
Subject: [PATCH] Function to test palloc/pfree performance
---
src/backend/utils/adt/mcxtfuncs.c | 213 ++++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 12 ++
2 files changed, 225 insertions(+)
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 1a4dbbeb8db..89ba91d6de4 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -13,6 +13,8 @@
*-------------------------------------------------------------------------
*/
+#include <time.h>
+
#include "postgres.h"
#include "catalog/pg_type_d.h"
@@ -308,3 +310,214 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(true);
}
+
+typedef struct AllocateTestNext
+{
+ struct AllocateTestNext *next; /* ptr to the next allocation */
+} AllocateTestNext;
+
+/* #define ALLOCATE_TEST_DEBUG */
+/*
+ * pg_allocate_memory_test
+ * Used to test the performance of a memory context types
+ */
+Datum
+pg_allocate_memory_test(PG_FUNCTION_ARGS)
+{
+ int32 chunk_size = PG_GETARG_INT32(0);
+ int64 keep_memory = PG_GETARG_INT64(1);
+ int64 total_alloc = PG_GETARG_INT64(2);
+ text *context_type_text = PG_GETARG_TEXT_PP(3);
+ char *context_type;
+ int64 curr_memory_use = 0;
+ int64 remaining_alloc_bytes = total_alloc;
+ MemoryContext context;
+ MemoryContext oldContext;
+ AllocateTestNext *next_free_ptr = NULL;
+ AllocateTestNext *last_alloc = NULL;
+ clock_t start, end;
+
+ if (chunk_size < sizeof(AllocateTestNext))
+ elog(ERROR, "chunk_size (%d) must be at least %ld bytes", chunk_size,
+ sizeof(AllocateTestNext));
+ if (keep_memory > total_alloc)
+ elog(ERROR, "keep_memory (" INT64_FORMAT ") must be less than total_alloc (" INT64_FORMAT ")",
+ keep_memory, total_alloc);
+
+ context_type = text_to_cstring(context_type_text);
+
+ start = clock();
+
+ if (strcmp(context_type, "generation") == 0)
+ context = GenerationContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "aset") == 0)
+ context = AllocSetContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "slab") == 0)
+ context = SlabContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_MAXSIZE,
+ chunk_size);
+ else
+ elog(ERROR, "context_type must be \"generation\", \"aset\" or \"slab\"");
+
+ oldContext = MemoryContextSwitchTo(context);
+
+ while (remaining_alloc_bytes > 0)
+ {
+ AllocateTestNext *curr_alloc;
+
+ CHECK_FOR_INTERRUPTS();
+
+ /* Allocate the memory and update the counters */
+ curr_alloc = (AllocateTestNext *) palloc(chunk_size);
+ remaining_alloc_bytes -= chunk_size;
+ curr_memory_use += chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "alloc %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", curr_alloc, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ /*
+ * Point the last allocate to this one so that we can free allocations
+ * starting with the oldest first.
+ */
+ curr_alloc->next = NULL;
+ if (last_alloc != NULL)
+ last_alloc->next = curr_alloc;
+
+ if (next_free_ptr == NULL)
+ {
+ /*
+ * Remember the first chunk to free. We will follow the ->next
+ * pointers to find the next chunk to free when freeing memory
+ */
+ next_free_ptr = curr_alloc;
+ }
+
+ /*
+ * If the currently allocated memory has reached or exceeded the amount
+ * of memory we want to keep allocated at once then we'd better free
+ * some. Since all allocations are the same size we only need to free
+ * one allocation per loop.
+ */
+ if (curr_memory_use >= keep_memory)
+ {
+ AllocateTestNext *next = next_free_ptr->next;
+
+ /* free the memory and update the current memory usage */
+ pfree(next_free_ptr);
+ curr_memory_use -= chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "free %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", next_free_ptr, curr_memory_use, remaining_alloc_bytes);
+#endif
+ /* get the next chunk to free */
+ next_free_ptr = next;
+ }
+
+ if (curr_memory_use > 0)
+ last_alloc = curr_alloc;
+ else
+ last_alloc = NULL;
+ }
+
+ /* cleanup loop -- pfree remaining memory */
+ while (next_free_ptr != NULL)
+ {
+ AllocateTestNext *next = next_free_ptr->next;
+
+ /* free the memory and update the current memory usage */
+ pfree(next_free_ptr);
+ curr_memory_use -= chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "free %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", next_free_ptr, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ next_free_ptr = next;
+ }
+
+ MemoryContextSwitchTo(oldContext);
+
+ end = clock();
+
+ PG_RETURN_FLOAT8((double) (end - start) / CLOCKS_PER_SEC);
+}
+
+Datum
+pg_allocate_memory_test_reset(PG_FUNCTION_ARGS)
+{
+ int32 chunk_size = PG_GETARG_INT32(0);
+ int64 keep_memory = PG_GETARG_INT64(1);
+ int64 total_alloc = PG_GETARG_INT64(2);
+ text *context_type_text = PG_GETARG_TEXT_PP(3);
+ char *context_type;
+ int64 curr_memory_use = 0;
+ int64 remaining_alloc_bytes = total_alloc;
+ MemoryContext context;
+ MemoryContext oldContext;
+ clock_t start, end;
+
+ if (chunk_size < 1)
+ elog(ERROR, "size of chunk must be above 0");
+ if (keep_memory > total_alloc)
+ elog(ERROR, "keep_memory (" INT64_FORMAT ") must be less than total_alloc (" INT64_FORMAT ")",
+ keep_memory, total_alloc);
+
+ context_type = text_to_cstring(context_type_text);
+
+ start = clock();
+
+ if (strcmp(context_type, "generation") == 0)
+ context = GenerationContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "aset") == 0)
+ context = AllocSetContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_SIZES);
+ else if (strcmp(context_type, "slab") == 0)
+ context = SlabContextCreate(CurrentMemoryContext,
+ "pg_allocate_memory_test",
+ ALLOCSET_DEFAULT_MAXSIZE,
+ chunk_size);
+ else
+ elog(ERROR, "context_type must be \"generation\", \"aset\" or \"slab\"");
+
+ oldContext = MemoryContextSwitchTo(context);
+
+ while (remaining_alloc_bytes > 0)
+ {
+ CHECK_FOR_INTERRUPTS();
+
+ /* Allocate the memory and update the counters */
+ (void) palloc(chunk_size);
+ remaining_alloc_bytes -= chunk_size;
+ curr_memory_use += chunk_size;
+
+#ifdef ALLOCATE_TEST_DEBUG
+ elog(NOTICE, "alloc %p (curr_memory_use " INT64_FORMAT " bytes, remaining_alloc_bytes " INT64_FORMAT ")", curr_alloc, curr_memory_use, remaining_alloc_bytes);
+#endif
+
+ /*
+ * If the currently allocated memory has reached or exceeded the amount
+ * of memory we want to keep allocated at once then reset the context.
+ */
+ if (curr_memory_use >= keep_memory)
+ {
+ curr_memory_use = 0;
+ MemoryContextReset(context);
+ }
+ }
+
+ MemoryContextSwitchTo(oldContext);
+ MemoryContextDelete(context);
+
+ end = clock();
+
+ PG_RETURN_FLOAT8((double) (end - start) / CLOCKS_PER_SEC);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0118e970dda..c43468da28b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8710,6 +8710,18 @@
prosrc => 'pg_log_backend_memory_contexts',
proacl => '{POSTGRES=X}' },
+# just for testing memory context allocation speed
+{ oid => '9319', descr => 'for testing performance of allocation and freeing',
+ proname => 'pg_allocate_memory_test', provolatile => 'v',
+ prorettype => 'float8', proargtypes => 'int4 int8 int8 text',
+ prosrc => 'pg_allocate_memory_test' },
+
+# just for testing memory context allocation speed
+{ oid => '9320', descr => 'for testing performance of allocation and resetting',
+ proname => 'pg_allocate_memory_test_reset', provolatile => 'v',
+ prorettype => 'float8', proargtypes => 'int4 int8 int8 text',
+ prosrc => 'pg_allocate_memory_test_reset' },
+
# non-persistent series generator
{ oid => '1066', descr => 'non-persistent series generator',
proname => 'generate_series', prorows => '1000',
--
2.51.0
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
In-Reply-To: <CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox