Propose Optimization of using the family of macros VALGRIND_MEMPOOL_*.
How?
- add the GetMemoryChunkCapacity method, which returns the size of
usable space in the chunk
- use GetMemoryChunkCapacity on VALGRIND_MEMPOOL_* calls
- call VALGRIND_MEMPOOL_CHANGED only for really changed chunks
*) Full patch code see in attachment
001-mem-chunk-capacity-and-repalloc-valgrind.patch
Why?
Under the valgrind control, the VALGRIND_MEMPOOL_CHANGE call works
very slooowly. With a large number of allocated memory chunks (a few
thousand and more) it is almost impossible to wait for the
program/test to done. This creates problems. During debugging and
auto-tests.
For example, below code is executed 90000ms on Core i7
for (int64 i = 0; i < 16000; ++i)
chunks[i] = palloc(64);
for (int64 i = 0; i < 16000; ++i)
chunks[i] = repalloc(chunks[i], 62);
With patch above - this code is executed 150ms.
*) Full extension code to demonstrate the problem see in attachment
valgrind_demo.tar.gz
An additional example
is the rum extension (https://github.com/postgrespro/rum).
To be able to perform the tests - need reduce the generate_series size
from 100K to 16K (https://github.com/postgrespro/rum/issues/76). But
under valgrind test execution remaining unacceptably sloow. Patch
above - completely solves the problem.
valgrind_demo.tar.gz
Description: application/gzip
From 85ef5cffe892aed72980898702e31095a13fe6e0 Mon Sep 17 00:00:00 2001
From: "Ivan N. Taranov" <i.tara...@postgrespro.ru>
Date: Thu, 2 Apr 2020 12:22:38 +0300
Subject: [PATCH] +GetMemoryChunkCapacity +repalloc passes to VALGRIND only really changed chunks
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index c0623f106d2..7327f2e982e 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -272,6 +272,7 @@ static void *AllocSetRealloc(MemoryContext context, void *pointer, Size size);
static void AllocSetReset(MemoryContext context);
static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
+static Size AllocSetGetChunkCapacity(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
static void AllocSetStats(MemoryContext context,
MemoryStatsPrintFunc printfunc, void *passthru,
@@ -291,6 +292,7 @@ static const MemoryContextMethods AllocSetMethods = {
AllocSetReset,
AllocSetDelete,
AllocSetGetChunkSpace,
+ AllocSetGetChunkCapacity,
AllocSetIsEmpty,
AllocSetStats
#ifdef MEMORY_CONTEXT_CHECKING
@@ -1337,6 +1339,23 @@ AllocSetGetChunkSpace(MemoryContext context, void *pointer)
return result;
}
+/*
+ * AllocSetGetChunkCapacity
+ * Given a currently-allocated chunk, return the size of the
+ * usable space in the chunk.
+ */
+static Size
+AllocSetGetChunkCapacity(MemoryContext context, void *pointer)
+{
+ AllocChunk chunk = AllocPointerGetChunk(pointer);
+ Size result;
+
+ VALGRIND_MAKE_MEM_DEFINED(chunk, ALLOCCHUNK_PRIVATE_LEN);
+ result = chunk->size;
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOCCHUNK_PRIVATE_LEN);
+ return result;
+}
+
/*
* AllocSetIsEmpty
* Is an allocset empty of any allocated space?
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index 56651d06931..09ccd811d48 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -152,6 +152,7 @@ static void *GenerationRealloc(MemoryContext context, void *pointer, Size size);
static void GenerationReset(MemoryContext context);
static void GenerationDelete(MemoryContext context);
static Size GenerationGetChunkSpace(MemoryContext context, void *pointer);
+static Size GenerationGetChunkCapacity(MemoryContext context, void *pointer);
static bool GenerationIsEmpty(MemoryContext context);
static void GenerationStats(MemoryContext context,
MemoryStatsPrintFunc printfunc, void *passthru,
@@ -171,6 +172,7 @@ static const MemoryContextMethods GenerationMethods = {
GenerationReset,
GenerationDelete,
GenerationGetChunkSpace,
+ GenerationGetChunkCapacity,
GenerationIsEmpty,
GenerationStats
#ifdef MEMORY_CONTEXT_CHECKING
@@ -666,6 +668,23 @@ GenerationGetChunkSpace(MemoryContext context, void *pointer)
return result;
}
+/*
+ * GenerationGetChunkCapacity
+ * Given a currently-allocated chunk, return the size of the
+ * usable space in the chunk.
+ */
+static Size
+GenerationGetChunkCapacity(MemoryContext context, void *pointer)
+{
+ GenerationChunk *chunk = GenerationPointerGetChunk(pointer);
+ Size result;
+
+ VALGRIND_MAKE_MEM_DEFINED(chunk, GENERATIONCHUNK_PRIVATE_LEN);
+ result = chunk->size;
+ VALGRIND_MAKE_MEM_NOACCESS(chunk, GENERATIONCHUNK_PRIVATE_LEN);
+ return result;
+}
+
/*
* GenerationIsEmpty
* Is a GenerationContext empty of any allocated space?
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9e24fec72d6..4e5adf5c3ac 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -431,6 +431,21 @@ GetMemoryChunkSpace(void *pointer)
return context->methods->get_chunk_space(context, pointer);
}
+/*
+ * GetMemoryChunkCapacity
+ * Given a currently-allocated chunk, return the size of the
+ * usable space in the chunk.
+ *
+ * This is useful for measuring the space available for data on the chunk.
+ */
+Size
+GetMemoryChunkCapacity(void *pointer)
+{
+ MemoryContext context = GetMemoryChunkContext(pointer);
+
+ return context->methods->get_chunk_capacity(context, pointer);
+}
+
/*
* MemoryContextGetParent
* Get the parent context (if any) of the specified context
@@ -823,7 +838,7 @@ MemoryContextAlloc(MemoryContext context, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
return ret;
}
@@ -859,7 +874,7 @@ MemoryContextAllocZero(MemoryContext context, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
MemSetAligned(ret, 0, size);
@@ -897,7 +912,7 @@ MemoryContextAllocZeroAligned(MemoryContext context, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
MemSetLoop(ret, 0, size);
@@ -937,7 +952,7 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
return NULL;
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
if ((flags & MCXT_ALLOC_ZERO) != 0)
MemSetAligned(ret, 0, size);
@@ -971,7 +986,7 @@ palloc(Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
return ret;
}
@@ -1002,7 +1017,7 @@ palloc0(Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
MemSetAligned(ret, 0, size);
@@ -1040,7 +1055,7 @@ palloc_extended(Size size, int flags)
return NULL;
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
if ((flags & MCXT_ALLOC_ZERO) != 0)
MemSetAligned(ret, 0, size);
@@ -1069,7 +1084,10 @@ void *
repalloc(void *pointer, Size size)
{
MemoryContext context = GetMemoryChunkContext(pointer);
- void *ret;
+ void *ret;
+#ifdef USE_VALGRIND
+ Size prev_capacity;
+#endif
if (!AllocSizeIsValid(size))
elog(ERROR, "invalid memory alloc request size %zu", size);
@@ -1079,6 +1097,10 @@ repalloc(void *pointer, Size size)
/* isReset must be false already */
Assert(!context->isReset);
+#ifdef USE_VALGRIND
+ prev_capacity = GetMemoryChunkCapacity(pointer);
+#endif
+
ret = context->methods->realloc(context, pointer, size);
if (unlikely(ret == NULL))
{
@@ -1090,7 +1112,12 @@ repalloc(void *pointer, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
+#ifdef USE_VALGRIND
+ if ((ret != pointer) || (GetMemoryChunkCapacity(ret) != prev_capacity))
+ {
+ VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, GetMemoryChunkCapacity(ret));
+ }
+#endif
return ret;
}
@@ -1125,7 +1152,7 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
+ VALGRIND_MEMPOOL_ALLOC(context, ret, GetMemoryChunkCapacity(ret));
return ret;
}
@@ -1139,7 +1166,10 @@ void *
repalloc_huge(void *pointer, Size size)
{
MemoryContext context = GetMemoryChunkContext(pointer);
- void *ret;
+ void *ret;
+#ifdef USE_VALGRIND
+ Size prev_capacity;
+#endif
if (!AllocHugeSizeIsValid(size))
elog(ERROR, "invalid memory alloc request size %zu", size);
@@ -1149,6 +1179,10 @@ repalloc_huge(void *pointer, Size size)
/* isReset must be false already */
Assert(!context->isReset);
+#ifdef USE_VALGRIND
+ prev_capacity = GetMemoryChunkCapacity(pointer);
+#endif
+
ret = context->methods->realloc(context, pointer, size);
if (unlikely(ret == NULL))
{
@@ -1160,7 +1194,12 @@ repalloc_huge(void *pointer, Size size)
size, context->name)));
}
- VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
+#ifdef USE_VALGRIND
+ if ((ret != pointer) || (GetMemoryChunkCapacity(ret) != prev_capacity))
+ {
+ VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, GetMemoryChunkCapacity(ret));
+ }
+#endif
return ret;
}
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index c928476c479..e2c2c8ed8fd 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -132,6 +132,7 @@ static void *SlabRealloc(MemoryContext context, void *pointer, Size size);
static void SlabReset(MemoryContext context);
static void SlabDelete(MemoryContext context);
static Size SlabGetChunkSpace(MemoryContext context, void *pointer);
+static Size SlabGetChunkCapacity(MemoryContext context, void *pointer);
static bool SlabIsEmpty(MemoryContext context);
static void SlabStats(MemoryContext context,
MemoryStatsPrintFunc printfunc, void *passthru,
@@ -150,6 +151,7 @@ static const MemoryContextMethods SlabMethods = {
SlabReset,
SlabDelete,
SlabGetChunkSpace,
+ SlabGetChunkCapacity,
SlabIsEmpty,
SlabStats
#ifdef MEMORY_CONTEXT_CHECKING
@@ -630,6 +632,21 @@ SlabGetChunkSpace(MemoryContext context, void *pointer)
return slab->fullChunkSize;
}
+/*
+ * SlabGetChunkCapacity
+ * Given a currently-allocated chunk, return the size of the
+ * usable space in the chunk.
+ */
+static Size
+SlabGetChunkCapacity(MemoryContext context, void *pointer)
+{
+ SlabContext *slab = castNode(SlabContext, context);
+
+ Assert(slab);
+
+ return slab->chunkSize;
+}
+
/*
* SlabIsEmpty
* Is an Slab empty of any allocated space?
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index c9f2bbcb367..1d6d096ffe8 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -63,6 +63,7 @@ typedef struct MemoryContextMethods
void (*reset) (MemoryContext context);
void (*delete_context) (MemoryContext context);
Size (*get_chunk_space) (MemoryContext context, void *pointer);
+ Size (*get_chunk_capacity) (MemoryContext context, void *pointer);
bool (*is_empty) (MemoryContext context);
void (*stats) (MemoryContext context,
MemoryStatsPrintFunc printfunc, void *passthru,
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 909bc2e9888..7bfbfee5ba1 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -80,6 +80,7 @@ extern void MemoryContextSetIdentifier(MemoryContext context, const char *id);
extern void MemoryContextSetParent(MemoryContext context,
MemoryContext new_parent);
extern Size GetMemoryChunkSpace(void *pointer);
+extern Size GetMemoryChunkCapacity(void *pointer);
extern MemoryContext MemoryContextGetParent(MemoryContext context);
extern bool MemoryContextIsEmpty(MemoryContext context);
extern Size MemoryContextMemAllocated(MemoryContext context, bool recurse);
--
2.26.0
