On Thu, Jan 16, 2020 at 10:27:01AM -0500, Tom Lane wrote:
Andres Freund <and...@anarazel.de> writes:
... I thought you were asking whether
any additional memory could just be avoided...
Well, I was kind of wondering that, but if it's not practical then
preallocating the space instead will do.
I don't think it's practical to rework the checks in a way that would
not require allocations. Maybe it's possible, but I think it's not worth
the extra complexity.
The attached fix should do the trick - it pre-allocates the space when
creating the context. There is a bit of complexity because we want to
allocate the space as part of the context header, but nothin too bad. We
might optimize it a bit by using a regular bitmap (instead of just an
array of bools), but I haven't done that.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index c5866d9cc3..7f9749a73f 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -70,6 +70,9 @@ typedef struct SlabContext
int chunksPerBlock; /* number of chunks per block */
int minFreeChunks; /* min number of free chunks in
any block */
int nblocks; /* number of blocks
allocated */
+#ifdef MEMORY_CONTEXT_CHECKING
+ char *freechunks; /* bitmap of free chunks on a block */
+#endif
/* blocks with free space, grouped by number of free chunks: */
dlist_head freelist[FLEXIBLE_ARRAY_MEMBER];
} SlabContext;
@@ -229,6 +232,15 @@ SlabContextCreate(MemoryContext parent,
/* Size of the memory context header */
headerSize = offsetof(SlabContext, freelist) + freelistSize;
+#ifdef MEMORY_CONTEXT_CHECKING
+ /*
+ * With memory checking, we need to allocate extra space for the bitmap
+ * of free chunks. The space is allocated at the end, and we need proper
+ * alignment (it's an array of bools, so maybe MAXALIGN is not needed).
+ */
+ headerSize = MAXALIGN(headerSize) + chunksPerBlock * sizeof(bool);
+#endif
+
slab = (SlabContext *) malloc(headerSize);
if (slab == NULL)
{
@@ -258,6 +270,12 @@ SlabContextCreate(MemoryContext parent,
for (i = 0; i < (slab->chunksPerBlock + 1); i++)
dlist_init(&slab->freelist[i]);
+#ifdef MEMORY_CONTEXT_CHECKING
+ /* set the freechunks pointer after the freelists array (aligned) */
+ slab->freechunks = (char *) slab +
+ MAXALIGN(offsetof(SlabContext, freelist) +
freelistSize);
+#endif
+
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) slab,
T_SlabContext,
@@ -701,14 +719,10 @@ SlabCheck(MemoryContext context)
int i;
SlabContext *slab = castNode(SlabContext, context);
const char *name = slab->header.name;
- char *freechunks;
Assert(slab);
Assert(slab->chunksPerBlock > 0);
- /* bitmap of free chunks on a block */
- freechunks = palloc(slab->chunksPerBlock * sizeof(bool));
-
/* walk all the freelists */
for (i = 0; i <= slab->chunksPerBlock; i++)
{
@@ -731,7 +745,7 @@ SlabCheck(MemoryContext context)
name, block->nfree, block, i);
/* reset the bitmap of free chunks for this block */
- memset(freechunks, 0, (slab->chunksPerBlock *
sizeof(bool)));
+ memset(slab->freechunks, 0, (slab->chunksPerBlock *
sizeof(bool)));
idx = block->firstFreeChunk;
/*
@@ -748,7 +762,7 @@ SlabCheck(MemoryContext context)
/* count the chunk as free, add it to the
bitmap */
nfree++;
- freechunks[idx] = true;
+ slab->freechunks[idx] = true;
/* read index of the next free chunk */
chunk = SlabBlockGetChunk(slab, block, idx);
@@ -759,7 +773,7 @@ SlabCheck(MemoryContext context)
for (j = 0; j < slab->chunksPerBlock; j++)
{
/* non-zero bit in the bitmap means chunk the
chunk is used */
- if (!freechunks[j])
+ if (!slab->freechunks[j])
{
SlabChunk *chunk =
SlabBlockGetChunk(slab, block, j);
