From 6f4cfe48ac6234e6dca165d52673777e7b4357f1 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 16 Nov 2007 21:55:59 +0000
Subject: [PATCH] Improve GIN index build's tracking of memory usage by using
 GetMemoryChunkSpace, not just the palloc request size.  This brings the
 allocatedMemory counter close enough to reality (as measured by
 MemoryContextStats printouts) that I think we can get rid of the arbitrary
 factor-of-2 adjustment that was put into the code initially.  Given the
 sensitivity of GIN build to work memory size, not using as much of work
 memory as we're allowed to seems a pretty bad idea.

---
 src/backend/access/gin/ginbulk.c   | 17 ++++++++++-------
 src/backend/access/gin/gininsert.c | 17 +++++++----------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/backend/access/gin/ginbulk.c b/src/backend/access/gin/ginbulk.c
index d0ac0b62121..09dd0094907 100644
--- a/src/backend/access/gin/ginbulk.c
+++ b/src/backend/access/gin/ginbulk.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gin/ginbulk.c,v 1.9 2007/02/01 04:16:08 neilc Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gin/ginbulk.c,v 1.10 2007/11/16 21:55:59 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
@@ -16,6 +16,7 @@
 
 #include "access/gin.h"
 #include "utils/datum.h"
+#include "utils/memutils.h"
 
 
 #define DEF_NENTRY	2048
@@ -38,7 +39,7 @@ EAAllocate(BuildAccumulator *accum)
 	if (accum->entryallocator == NULL || accum->length >= DEF_NENTRY)
 	{
 		accum->entryallocator = palloc(sizeof(EntryAccumulator) * DEF_NENTRY);
-		accum->allocatedMemory += sizeof(EntryAccumulator) * DEF_NENTRY;
+		accum->allocatedMemory += GetMemoryChunkSpace(accum->entryallocator);
 		accum->length = 0;
 	}
 
@@ -55,10 +56,11 @@ ginInsertData(BuildAccumulator *accum, EntryAccumulator *entry, ItemPointer heap
 {
 	if (entry->number >= entry->length)
 	{
-		accum->allocatedMemory += sizeof(ItemPointerData) * entry->length;
+		accum->allocatedMemory -= GetMemoryChunkSpace(entry->list);
 		entry->length *= 2;
 		entry->list = (ItemPointerData *) repalloc(entry->list,
 									sizeof(ItemPointerData) * entry->length);
+		accum->allocatedMemory += GetMemoryChunkSpace(entry->list);
 	}
 
 	if (entry->shouldSort == FALSE)
@@ -95,10 +97,10 @@ getDatumCopy(BuildAccumulator *accum, Datum value)
 		realSize = datumGetSize(value, false, att[0]->attlen);
 
 		s = (char *) palloc(realSize);
+		accum->allocatedMemory += GetMemoryChunkSpace(s);
+
 		memcpy(s, DatumGetPointer(value), realSize);
 		res = PointerGetDatum(s);
-
-		accum->allocatedMemory += realSize;
 	}
 	return res;
 }
@@ -143,8 +145,8 @@ ginInsertEntry(BuildAccumulator *accum, ItemPointer heapptr, Datum entry)
 		ea->number = 1;
 		ea->shouldSort = FALSE;
 		ea->list = (ItemPointerData *) palloc(sizeof(ItemPointerData) * DEF_NPTR);
+		accum->allocatedMemory += GetMemoryChunkSpace(ea->list);
 		ea->list[0] = *heapptr;
-		accum->allocatedMemory += sizeof(ItemPointerData) * DEF_NPTR;
 
 		if (pea == NULL)
 			accum->entries = ea;
@@ -270,11 +272,11 @@ ginGetEntry(BuildAccumulator *accum, Datum *value, uint32 *n)
 	EntryAccumulator *entry;
 	ItemPointerData *list;
 
-
 	if (accum->stack == NULL)
 	{
 		/* first call */
 		accum->stack = palloc0(sizeof(EntryAccumulator *) * (accum->maxdepth + 1));
+		accum->allocatedMemory += GetMemoryChunkSpace(accum->stack);
 		entry = accum->entries;
 
 		if (entry == NULL)
@@ -295,6 +297,7 @@ ginGetEntry(BuildAccumulator *accum, Datum *value, uint32 *n)
 	}
 	else
 	{
+		accum->allocatedMemory -= GetMemoryChunkSpace(accum->stack[accum->stackpos]->list);
 		pfree(accum->stack[accum->stackpos]->list);
 		accum->stack[accum->stackpos]->list = NULL;
 		entry = walkTree(accum);
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index db5fa68338a..8afd8afd313 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -8,17 +8,19 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gin/gininsert.c,v 1.9 2007/06/05 12:47:49 teodor Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gin/gininsert.c,v 1.10 2007/11/16 21:55:59 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
+
 #include "access/genam.h"
 #include "access/gin.h"
 #include "catalog/index.h"
 #include "miscadmin.h"
 #include "utils/memutils.h"
 
+
 typedef struct
 {
 	GinState	ginstate;
@@ -132,8 +134,7 @@ addItemPointersToTuple(Relation index, GinState *ginstate, GinBtreeStack *stack,
 }
 
 /*
- * Inserts only one entry to the index, but it can adds more that 1
- * ItemPointer.
+ * Inserts only one entry to the index, but it can add more than 1 ItemPointer.
  */
 static void
 ginEntryInsert(Relation index, GinState *ginstate, Datum value, ItemPointerData *items, uint32 nitem, bool isBuild)
@@ -198,7 +199,7 @@ ginEntryInsert(Relation index, GinState *ginstate, Datum value, ItemPointerData
 
 /*
  * Saves indexed value in memory accumulator during index creation
- * Function isn't use during normal insert
+ * Function isn't used during normal insert
  */
 static uint32
 ginHeapTupleBulkInsert(GinBuildState *buildstate, Datum value, ItemPointer heapptr)
@@ -226,7 +227,6 @@ static void
 ginBuildCallback(Relation index, HeapTuple htup, Datum *values,
 				 bool *isnull, bool tupleIsAlive, void *state)
 {
-
 	GinBuildState *buildstate = (GinBuildState *) state;
 	MemoryContext oldCtx;
 
@@ -237,11 +237,8 @@ ginBuildCallback(Relation index, HeapTuple htup, Datum *values,
 
 	buildstate->indtuples += ginHeapTupleBulkInsert(buildstate, *values, &htup->t_self);
 
-	/*
-	 * we use only half maintenance_work_mem, because there is some leaks
-	 * during insertion and extract values
-	 */
-	if (buildstate->accum.allocatedMemory >= maintenance_work_mem * 1024L / 2L)
+	/* If we've maxed out our available memory, dump everything to the index */
+	if (buildstate->accum.allocatedMemory >= maintenance_work_mem * 1024L)
 	{
 		ItemPointerData *list;
 		Datum		entry;
-- 
GitLab