Re: [HACKERS] Postgres Speed or lack thereof - Mailing list pgsql-hackers

Tom Lane wrote:

>
> jwieck@debis.com (Jan Wieck) writes:
> > Tom Lane wrote:
> >> However, we probably don't really want to touch each individual palloc()
> >> call in the system ...
>
> >     There are about 900 calls of palloc() in the backend code. It
> >     is much less than I expected (we have over 200,000  lines  of
> >     code).
>
> Much less than I thought, also.  And (grep|wc...) over 300 of these
> calls are in the utils/adt code, ie, they are for passing back the
> results of type conversion functions.  We'll only need to think through
> how that should work once, and then it's just a mechanical edit to all
> the ADT files.

    Exactly  these  300  ones  (except  for 20 or 30 of them) are
    those that the caller needs :-)

> What about my prior point that the bottom-level function may not know
> how long the caller needs the storage?  Will we have to add a "memory
> context to put result in" parameter to a lot of routines?  Ugh, but
> maybe it's the only way.

    I've browsed a little throug the code (well, the candidates I
    thought are good choices for optimization). The problem there
    is, that the order of allocation hasn't anything to  do  with
    the  lifecycle  of  the  objects.  So  if  we  want to handle
    allocations different depending on the object lifecycle,  the
    allocators  (mainly their callers) would have to switch often
    between different contexts.

    It wouldn't make the code much  better  readable  if  between
    most   of   the  statements  some  MemoryContextStumbleOver()
    appears. And many memory contexts means  on  the  other  hand
    many  concurrently read/written pages. Thus I would expect it
    to affect the caching of the CPU.

>
> I've noticed that everyone else contributing to this thread has been
> thinking in terms of inventing multiple allocation functions with
> different names, ie a routine might have to call "palloc" or
> "fast_palloc" or "tmp_palloc" or whatever depending on what behavior it
> wants.

    So I.

>        I really think we are better off to stick to the structure that
> we already have (cf. include/nodes/memnodes.h and include/utils/mcxt.h),
> in which there's one set of interface routines that take a "context"
> parameter, and the context determines the semantics of allocation.
> (The interface routines probably ought to be macros, not out-of-line
> code, but that's a minor optimization.)  This will make it easier to
> handle cases where one routine has to tell another one how to allocate
> its result space: you pass a MemoryContext parameter.

    I came to the  same  conclusion.  So  I  continued  with  the
    approach  of  bigger  chunks handled in palloc(). What I have
    now is  something,  that  gains  about  10%  speedup  at  the
    regression  test,  while  memory consumption (visibly watched
    with top(1)) seems not to raise compared against old version.

    Since  the  bigger  blocks  are  built on top of the existing
    memory context model, it would not conflict with any enhanced
    usage we could make with it.

    I include a patch at the end - please comment.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #


diff -cr src.orig/backend/utils/mmgr/mcxt.c src/backend/utils/mmgr/mcxt.c
*** src.orig/backend/utils/mmgr/mcxt.c    Thu Jan 28 16:47:39 1999
--- src/backend/utils/mmgr/mcxt.c    Fri Jan 29 20:15:41 1999
***************
*** 106,111 ****
--- 106,112 ----
  static struct GlobalMemory TopGlobalMemoryData = {
      T_GlobalMemory,                /* NodeTag                tag          */
      &GlobalContextMethodsData,    /* ContextMethods        method      */
+     NULL,                        /* char* smallchunk_block    */
      {{0}},                        /* uninitialized OrderedSetData allocSetD */
      "TopGlobal",                /* char* name       */
      {0}                            /* uninitialized OrderedElemData elemD */
diff -cr src.orig/backend/utils/mmgr/palloc.c src/backend/utils/mmgr/palloc.c
*** src.orig/backend/utils/mmgr/palloc.c    Thu Jan 28 16:47:39 1999
--- src/backend/utils/mmgr/palloc.c    Fri Jan 29 21:57:06 1999
***************
*** 24,29 ****
--- 24,57 ----

  #include "utils/palloc.h"

+
+ typedef struct PallocBlock {
+     MemoryContext    mcxt;
+     int                refcount;
+     char           *unused_chunk;
+     char           *freeptr;
+     char           *endptr;
+ } PallocBlock;
+
+ typedef struct PallocChunk {
+     PallocBlock        *blk;
+     Size            size;
+ } PallocChunk;
+
+
+ #define PALLOC_BLOCKSIZE        \
+             (8192 - sizeof(OrderedElemData) - sizeof(Size))
+ #define PALLOC_CHUNK_LIMIT        512
+ #define PALLOC_SIZE_ALIGN(s)    ((s + 15) & ~15)
+
+ #define PALLOC_CHUNK_BLKPTR(c)    (((PallocChunk *)(c))[-1].blk)
+ #define PALLOC_CHUNK_SIZE(c)    (((PallocChunk *)(c))[-1].size)
+
+ #define PALLOC_CHUNK_HDRSZ        MAXALIGN(sizeof(PallocChunk))
+ #define PALLOC_BLOCK_HDRSZ        MAXALIGN(sizeof(PallocBlock))
+
+ #define PALLOC_FREESPACE(b)        ((b)->endptr - (b)->freeptr)
+
  /* ----------------------------------------------------------------
   *        User library functions
   * ----------------------------------------------------------------
***************
*** 66,72 ****
  #ifdef PALLOC_IS_MALLOC
      return malloc(size);
  #else
!     return MemoryContextAlloc(CurrentMemoryContext, size);
  #endif     /* PALLOC_IS_MALLOC */
  }

--- 94,168 ----
  #ifdef PALLOC_IS_MALLOC
      return malloc(size);
  #else
!     PallocBlock        *block;
!     char            *chunk;
!     Size            asize = PALLOC_SIZE_ALIGN(size);
!
!     if (asize >= PALLOC_CHUNK_LIMIT)
!     {
!         chunk = (char *)MemoryContextAlloc(CurrentMemoryContext, size +
!                                                         PALLOC_CHUNK_HDRSZ);
!         chunk += PALLOC_CHUNK_HDRSZ;
!         PALLOC_CHUNK_BLKPTR(chunk) = NULL;
!         return (void *)chunk;
!     }
!
!     block = (PallocBlock *)(CurrentMemoryContext->smallchunk_block);
!
!     if (block != NULL && PALLOC_FREESPACE(block) < asize)
!     {
!         char    *prev = NULL;
!         Size    chunk_size;
!
!         chunk = block->unused_chunk;
!         while (chunk)
!         {
!             chunk_size = PALLOC_CHUNK_SIZE(chunk);
!             if (asize == chunk_size)
!             {
!                 if (prev == NULL)
!                     block->unused_chunk = (char *)PALLOC_CHUNK_BLKPTR(chunk);
!                 else
!                     PALLOC_CHUNK_BLKPTR(prev) = PALLOC_CHUNK_BLKPTR(chunk);
!
!                 block->refcount++;
!                 PALLOC_CHUNK_BLKPTR(chunk) = block;
!                 /*
!                 PALLOC_CHUNK_SIZE(chunk) = size;
!                 */
!                 return (void *)chunk;
!             }
!             prev = chunk;
!             chunk = (char *)PALLOC_CHUNK_BLKPTR(chunk);
!         }
!
!         block = NULL;
!     }
!
!     if (block == NULL)
!     {
!         block = (PallocBlock *)MemoryContextAlloc(CurrentMemoryContext,
!                                     PALLOC_BLOCKSIZE);
!         block->mcxt = CurrentMemoryContext;
!         block->unused_chunk = NULL;
!         block->refcount = 0;
!         block->freeptr = ((char *)block) + PALLOC_BLOCK_HDRSZ +
!                                             PALLOC_CHUNK_HDRSZ;
!         block->endptr = ((char *)block) + PALLOC_BLOCKSIZE;
!
!         CurrentMemoryContext->smallchunk_block = (void *)block;
!     }
!
!     chunk = block->freeptr;
!     block->freeptr += PALLOC_CHUNK_HDRSZ + asize;
!     block->refcount++;
!     PALLOC_CHUNK_BLKPTR(chunk) = block;
!     PALLOC_CHUNK_SIZE(chunk) = asize;
!
!     if (block->freeptr >= block->endptr)
!         block->mcxt->smallchunk_block = NULL;
!
!     return (void *)chunk;
  #endif     /* PALLOC_IS_MALLOC */
  }

***************
*** 76,82 ****
  #ifdef PALLOC_IS_MALLOC
      free(pointer);
  #else
!     MemoryContextFree(CurrentMemoryContext, pointer);
  #endif     /* PALLOC_IS_MALLOC */
  }

--- 172,202 ----
  #ifdef PALLOC_IS_MALLOC
      free(pointer);
  #else
!     PallocBlock        *block = PALLOC_CHUNK_BLKPTR(pointer);
!
!     if (block == NULL)
!     {
!         MemoryContextFree(CurrentMemoryContext, (char *)pointer - PALLOC_CHUNK_HDRSZ);
!         return;
!     }
!
!     PALLOC_CHUNK_BLKPTR(pointer) = (PallocBlock *)(block->unused_chunk);
!     block->unused_chunk = (char *)pointer;
!
!     block->refcount--;
!     if (block->refcount == 0)
!     {
!         if (block == (PallocBlock *)(block->mcxt->smallchunk_block))
!         {
!             block->freeptr = ((char *)block) + PALLOC_BLOCK_HDRSZ +
!                                                 PALLOC_CHUNK_HDRSZ;
!             block->unused_chunk = NULL;
!         }
!         else
!         {
!             MemoryContextFree(block->mcxt, (void *)block);
!         }
!     }
  #endif     /* PALLOC_IS_MALLOC */
  }

***************
*** 100,106 ****
  #ifdef PALLOC_IS_MALLOC
      return realloc(pointer, size);
  #else
!     return MemoryContextRealloc(CurrentMemoryContext, pointer, size);
  #endif
  }

--- 220,248 ----
  #ifdef PALLOC_IS_MALLOC
      return realloc(pointer, size);
  #else
!     PallocBlock        *block = PALLOC_CHUNK_BLKPTR(pointer);
!     char             *new;
!     Size            tocopy;
!
!     if (block == NULL)
!     {
!         new = (char *)MemoryContextRealloc(CurrentMemoryContext,
!                                 (char *)pointer - PALLOC_CHUNK_HDRSZ,
!                                 size + PALLOC_CHUNK_HDRSZ);
!         new += PALLOC_CHUNK_HDRSZ;
!         PALLOC_CHUNK_BLKPTR(new) = NULL;
!         return (void *)new;
!     }
!     else
!     {
!         new = palloc(size);
!
!         tocopy = PALLOC_CHUNK_SIZE(pointer) > size ?
!                             size : PALLOC_CHUNK_SIZE(pointer);
!         memcpy(new, pointer, tocopy);
!         pfree(pointer);
!         return (void *)new;
!     }
  #endif
  }

***************
*** 117,119 ****
--- 259,263 ----

      return nstr;
  }
+
+
diff -cr src.orig/backend/utils/mmgr/portalmem.c src/backend/utils/mmgr/portalmem.c
*** src.orig/backend/utils/mmgr/portalmem.c    Thu Jan 28 16:47:39 1999
--- src/backend/utils/mmgr/portalmem.c    Fri Jan 29 20:15:41 1999
***************
*** 390,395 ****
--- 390,396 ----
      NodeSetTag((Node *) &portal->variable, T_PortalVariableMemory);
      AllocSetInit(&portal->variable.setData, DynamicAllocMode, (Size) 0);
      portal->variable.method = &PortalVariableContextMethodsData;
+     portal->variable.smallchunk_block = NULL;

      /*
       * initialize portal heap context
***************
*** 399,404 ****
--- 400,406 ----
      FixedStackInit(&portal->heap.stackData,
                     offsetof(HeapMemoryBlockData, itemData));
      portal->heap.method = &PortalHeapContextMethodsData;
+     portal->heap.smallchunk_block = NULL;

      /*
       * set bogus portal name
***************
*** 756,761 ****
--- 758,764 ----
      NodeSetTag((Node *) &portal->variable, T_PortalVariableMemory);
      AllocSetInit(&portal->variable.setData, DynamicAllocMode, (Size) 0);
      portal->variable.method = &PortalVariableContextMethodsData;
+     portal->variable.smallchunk_block = NULL;

      /* initialize portal heap context */
      NodeSetTag((Node *) &portal->heap, T_PortalHeapMemory);
***************
*** 763,768 ****
--- 766,772 ----
      FixedStackInit(&portal->heap.stackData,
                     offsetof(HeapMemoryBlockData, itemData));
      portal->heap.method = &PortalHeapContextMethodsData;
+     portal->heap.smallchunk_block = NULL;

      /* initialize portal name */
      length = 1 + strlen(name);
***************
*** 918,923 ****
--- 922,928 ----

      /* free current mode */
      AllocSetReset(&HEAPMEMBLOCK(context)->setData);
+     context->smallchunk_block = NULL;
      MemoryContextFree((MemoryContext) PortalHeapMemoryGetVariableMemory(context),
                        context->block);

diff -cr src.orig/include/nodes/memnodes.h src/include/nodes/memnodes.h
*** src.orig/include/nodes/memnodes.h    Thu Jan 28 16:47:39 1999
--- src/include/nodes/memnodes.h    Fri Jan 29 20:15:41 1999
***************
*** 60,65 ****
--- 60,66 ----
  {
      NodeTag        type;
      MemoryContextMethods method;
+     void        *smallchunk_block;
  }           *MemoryContext;

  /* think about doing this right some time but we'll have explicit fields
***************
*** 68,73 ****
--- 69,75 ----
  {
      NodeTag        type;
      MemoryContextMethods method;
+     void        *smallchunk_block;
      AllocSetData setData;
      char       *name;
      OrderedElemData elemData;
***************
*** 79,84 ****
--- 81,87 ----
  {
      NodeTag        type;
      MemoryContextMethods method;
+     void        *smallchunk_block;
      AllocSetData setData;
  }           *PortalVariableMemory;

***************
*** 86,91 ****
--- 89,95 ----
  {
      NodeTag        type;
      MemoryContextMethods method;
+     void        *smallchunk_block;
      Pointer        block;
      FixedStackData stackData;
  }           *PortalHeapMemory;