Re: (full) Memory context dump considered harmful - Mailing list pgsql-hackers

From Tom Lane
Subject Re: (full) Memory context dump considered harmful
Date
Msg-id 8021.1440194441@sss.pgh.pa.us
Whole thread Raw
In response to Re: (full) Memory context dump considered harmful  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> I thought a bit more about this.  Generally, what you want to know about
> a given situation is which contexts have a whole lot of allocations
> and/or a whole lot of child contexts.  What you suggest above won't work
> very well if the problem is buried more than about two levels down in
> the context tree.  But suppose we add a parameter to memory context stats
> collection that is the maximum number of child contexts to print *per
> parent context*.  If there are more than that, summarize the rest as per
> your suggestion.

Here's a draft patch along this line.  After playing with it a bit,
I think that a wired-in limit of 100 children per level would be fine.
We could imagine adding a GUC for that, but I think it's overkill.

A couple of notes:

Since the summarization mechanism requires passing totals back up anyway,
I took the opportunity to add a "grand total" line to the end of the
printout.  I'm half tempted to modify that to convert the numbers in the
grand total line to kilobytes or even MB, but it could be argued either way.

This assumes that the total memory allocation couldn't exceed the range of
size_t, a thing which is not promised by the C standard --- but AFAIK it
would be true on any modern machine (and really we were already making
such an assumption in AllocSetStats).  Also, if we ever add a memory
context type that works fundamentally differently from AllocSet, we might
need to rethink what we compute/print as summary stats.  I figure we can
cross that bridge when we come to it.

Comments?

            regards, tom lane
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index febeb6e..e86c7cb 100644
*** a/src/backend/utils/mmgr/aset.c
--- b/src/backend/utils/mmgr/aset.c
*************** static void AllocSetReset(MemoryContext
*** 253,259 ****
  static void AllocSetDelete(MemoryContext context);
  static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
  static bool AllocSetIsEmpty(MemoryContext context);
! static void AllocSetStats(MemoryContext context, int level);

  #ifdef MEMORY_CONTEXT_CHECKING
  static void AllocSetCheck(MemoryContext context);
--- 253,260 ----
  static void AllocSetDelete(MemoryContext context);
  static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
  static bool AllocSetIsEmpty(MemoryContext context);
! static void AllocSetStats(MemoryContext context, int level, bool print,
!               MemoryContextCounters *totals);

  #ifdef MEMORY_CONTEXT_CHECKING
  static void AllocSetCheck(MemoryContext context);
*************** AllocSetIsEmpty(MemoryContext context)
*** 1228,1237 ****

  /*
   * AllocSetStats
!  *        Displays stats about memory consumption of an allocset.
   */
  static void
! AllocSetStats(MemoryContext context, int level)
  {
      AllocSet    set = (AllocSet) context;
      Size        nblocks = 0;
--- 1229,1243 ----

  /*
   * AllocSetStats
!  *        Compute stats about memory consumption of an allocset.
!  *
!  * level: recursion level (0 at top level); used for print indentation.
!  * print: true to print stats to stderr.
!  * totals: if not NULL, add stats about this allocset into *totals.
   */
  static void
! AllocSetStats(MemoryContext context, int level, bool print,
!               MemoryContextCounters *totals)
  {
      AllocSet    set = (AllocSet) context;
      Size        nblocks = 0;
*************** AllocSetStats(MemoryContext context, int
*** 1239,1247 ****
      Size        totalspace = 0;
      Size        freespace = 0;
      AllocBlock    block;
-     AllocChunk    chunk;
      int            fidx;
-     int            i;

      for (block = set->blocks; block != NULL; block = block->next)
      {
--- 1245,1251 ----
*************** AllocSetStats(MemoryContext context, int
*** 1251,1256 ****
--- 1255,1262 ----
      }
      for (fidx = 0; fidx < ALLOCSET_NUM_FREELISTS; fidx++)
      {
+         AllocChunk    chunk;
+
          for (chunk = set->freelist[fidx]; chunk != NULL;
               chunk = (AllocChunk) chunk->aset)
          {
*************** AllocSetStats(MemoryContext context, int
*** 1259,1271 ****
          }
      }

!     for (i = 0; i < level; i++)
!         fprintf(stderr, "  ");

!     fprintf(stderr,
              "%s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
!             set->header.name, totalspace, nblocks, freespace, nchunks,
!             totalspace - freespace);
  }


--- 1265,1289 ----
          }
      }

!     if (print)
!     {
!         int            i;

!         for (i = 0; i < level; i++)
!             fprintf(stderr, "  ");
!         fprintf(stderr,
              "%s: %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
!                 set->header.name, totalspace, nblocks, freespace, nchunks,
!                 totalspace - freespace);
!     }
!
!     if (totals)
!     {
!         totals->nblocks += nblocks;
!         totals->nchunks += nchunks;
!         totals->totalspace += totalspace;
!         totals->freespace += freespace;
!     }
  }


diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 12d29f7..ad1421c 100644
*** a/src/backend/utils/mmgr/mcxt.c
--- b/src/backend/utils/mmgr/mcxt.c
*************** MemoryContext CurTransactionContext = NU
*** 52,58 ****
  MemoryContext PortalContext = NULL;

  static void MemoryContextCallResetCallbacks(MemoryContext context);
! static void MemoryContextStatsInternal(MemoryContext context, int level);

  /*
   * You should not do memory allocations within a critical section, because
--- 52,60 ----
  MemoryContext PortalContext = NULL;

  static void MemoryContextCallResetCallbacks(MemoryContext context);
! static void MemoryContextStatsInternal(MemoryContext context, int level,
!                            bool print, int max_children,
!                            MemoryContextCounters *totals);

  /*
   * You should not do memory allocations within a critical section, because
*************** MemoryContextIsEmpty(MemoryContext conte
*** 477,501 ****
   * MemoryContextStats
   *        Print statistics about the named context and all its descendants.
   *
!  * This is just a debugging utility, so it's not fancy.  The statistics
!  * are merely sent to stderr.
   */
  void
  MemoryContextStats(MemoryContext context)
  {
!     MemoryContextStatsInternal(context, 0);
  }

  static void
! MemoryContextStatsInternal(MemoryContext context, int level)
  {
      MemoryContext child;

      AssertArg(MemoryContextIsValid(context));

!     (*context->methods->stats) (context, level);
!     for (child = context->firstchild; child != NULL; child = child->nextchild)
!         MemoryContextStatsInternal(child, level + 1);
  }

  /*
--- 479,584 ----
   * MemoryContextStats
   *        Print statistics about the named context and all its descendants.
   *
!  * This is just a debugging utility, so it's not very fancy.  However, we do
!  * make some effort to summarize when the output would otherwise be very long.
!  * The statistics are sent to stderr.
   */
  void
  MemoryContextStats(MemoryContext context)
  {
!     /* A hard-wired limit on the number of children is usually good enough */
!     MemoryContextStatsDetail(context, 100);
! }
!
! /*
!  * MemoryContextStatsDetail
!  *
!  * Entry point for use if you want to vary the number of child contexts shown.
!  */
! void
! MemoryContextStatsDetail(MemoryContext context, int max_children)
! {
!     MemoryContextCounters grand_totals;
!
!     memset(&grand_totals, 0, sizeof(grand_totals));
!
!     MemoryContextStatsInternal(context, 0, true, max_children, &grand_totals);
!
!     fprintf(stderr,
!     "Grand total: %zu bytes in %zd blocks; %zu free (%zd chunks); %zu used\n",
!             grand_totals.totalspace, grand_totals.nblocks,
!             grand_totals.freespace, grand_totals.nchunks,
!             grand_totals.totalspace - grand_totals.freespace);
  }

+ /*
+  * MemoryContextStatsInternal
+  *        One recursion level for MemoryContextStats
+  *
+  * Print this context if print is true, but in any case accumulate counts into
+  * *totals (if given).
+  */
  static void
! MemoryContextStatsInternal(MemoryContext context, int level,
!                            bool print, int max_children,
!                            MemoryContextCounters *totals)
  {
+     MemoryContextCounters local_totals;
      MemoryContext child;
+     int            ichild;

      AssertArg(MemoryContextIsValid(context));

!     /* Examine the context itself */
!     (*context->methods->stats) (context, level, print, totals);
!
!     /*
!      * Examine children.  If there are more than max_children of them, we do
!      * not print the rest explicitly, but just summarize them.
!      */
!     memset(&local_totals, 0, sizeof(local_totals));
!
!     for (child = context->firstchild, ichild = 0;
!          child != NULL;
!          child = child->nextchild, ichild++)
!     {
!         if (ichild < max_children)
!             MemoryContextStatsInternal(child, level + 1,
!                                        print, max_children,
!                                        totals);
!         else
!             MemoryContextStatsInternal(child, level + 1,
!                                        false, max_children,
!                                        &local_totals);
!     }
!
!     /* Deal with excess children */
!     if (ichild > max_children)
!     {
!         if (print)
!         {
!             int            i;
!
!             for (i = 0; i <= level; i++)
!                 fprintf(stderr, "  ");
!             fprintf(stderr,
!                     "%d more child contexts containing %zu total in %zd blocks; %zu free (%zd chunks); %zu used\n",
!                     ichild - max_children,
!                     local_totals.totalspace,
!                     local_totals.nblocks,
!                     local_totals.freespace,
!                     local_totals.nchunks,
!                     local_totals.totalspace - local_totals.freespace);
!         }
!
!         if (totals)
!         {
!             totals->nblocks += local_totals.nblocks;
!             totals->nchunks += local_totals.nchunks;
!             totals->totalspace += local_totals.totalspace;
!             totals->freespace += local_totals.freespace;
!         }
!     }
  }

  /*
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index 5e036b9..5e6bb60 100644
*** a/src/include/nodes/memnodes.h
--- b/src/include/nodes/memnodes.h
***************
*** 17,22 ****
--- 17,40 ----
  #include "nodes/nodes.h"

  /*
+  * MemoryContextCounters
+  *        Summarization state for MemoryContextStats collection.
+  *
+  * The set of counters in this struct is biased towards AllocSet; if we ever
+  * add any context types that are based on fundamentally different approaches,
+  * we might need more or different counters here.  A possible API spec then
+  * would be to print only nonzero counters, but for now we just summarize in
+  * the format historically used by AllocSet.
+  */
+ typedef struct MemoryContextCounters
+ {
+     Size        nblocks;        /* Total number of malloc blocks */
+     Size        nchunks;        /* Total number of free chunks */
+     Size        totalspace;        /* Total bytes requested from malloc */
+     Size        freespace;        /* The unused portion of totalspace */
+ } MemoryContextCounters;
+
+ /*
   * MemoryContext
   *        A logical context in which memory allocations occur.
   *
*************** typedef struct MemoryContextMethods
*** 44,50 ****
      void        (*delete_context) (MemoryContext context);
      Size        (*get_chunk_space) (MemoryContext context, void *pointer);
      bool        (*is_empty) (MemoryContext context);
!     void        (*stats) (MemoryContext context, int level);
  #ifdef MEMORY_CONTEXT_CHECKING
      void        (*check) (MemoryContext context);
  #endif
--- 62,69 ----
      void        (*delete_context) (MemoryContext context);
      Size        (*get_chunk_space) (MemoryContext context, void *pointer);
      bool        (*is_empty) (MemoryContext context);
!     void        (*stats) (MemoryContext context, int level, bool print,
!                                       MemoryContextCounters *totals);
  #ifdef MEMORY_CONTEXT_CHECKING
      void        (*check) (MemoryContext context);
  #endif
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index f0fe0f4..38106d7 100644
*** a/src/include/utils/memutils.h
--- b/src/include/utils/memutils.h
*************** extern MemoryContext GetMemoryChunkConte
*** 104,109 ****
--- 104,110 ----
  extern MemoryContext MemoryContextGetParent(MemoryContext context);
  extern bool MemoryContextIsEmpty(MemoryContext context);
  extern void MemoryContextStats(MemoryContext context);
+ extern void MemoryContextStatsDetail(MemoryContext context, int max_children);
  extern void MemoryContextAllowInCriticalSection(MemoryContext context,
                                      bool allow);


pgsql-hackers by date:

Previous
From: Jeff Janes
Date:
Subject: Re: Patch for ginCombineData
Next
From: Jim Nasby
Date:
Subject: Re: pg_dump quietly ignore missing tables - is it bug?