Thread: pg_buffercache tidyup

pg_buffercache tidyup

From
Mark Kirkwood
Date:
This is probably terrible timing, but I noticed Tom had done some nice
tidying up on pg_freespacemap to eliminate the clumsy conversion to and
from strings. This patch does a similar thing for pg_buffercache.

I did wonder about not showing buffers that are invalid or not in use
(currently displays all attributes bar the id as NULL). Comments?

Cheers

Mark

Index: contrib/pg_buffercache/pg_buffercache_pages.c
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/pg_buffercache/pg_buffercache_pages.c,v
retrieving revision 1.10
diff -c -r1.10 pg_buffercache_pages.c
*** contrib/pg_buffercache/pg_buffercache_pages.c    19 Oct 2006 18:32:46 -0000    1.10
--- contrib/pg_buffercache/pg_buffercache_pages.c    22 Oct 2006 06:27:52 -0000
***************
*** 8,13 ****
--- 8,14 ----
   */
  #include "postgres.h"
  #include "funcapi.h"
+ #include "access/heapam.h"
  #include "catalog/pg_type.h"
  #include "storage/buf_internals.h"
  #include "storage/bufmgr.h"
***************
*** 44,52 ****
  typedef struct
  {

!     AttInMetadata *attinmeta;
      BufferCachePagesRec *record;
-     char       *values[NUM_BUFFERCACHE_PAGES_ELEM];

  }    BufferCachePagesContext;

--- 45,52 ----
  typedef struct
  {

!     TupleDesc    tupdesc;
      BufferCachePagesRec *record;

  }    BufferCachePagesContext;

***************
*** 77,83 ****
          /* Switch context when allocating stuff to be used in later calls */
          oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

!         /* Construct a tuple to return. */
          tupledesc = CreateTemplateTupleDesc(NUM_BUFFERCACHE_PAGES_ELEM, false);
          TupleDescInitEntry(tupledesc, (AttrNumber) 1, "bufferid",
                             INT4OID, -1, 0);
--- 77,87 ----
          /* Switch context when allocating stuff to be used in later calls */
          oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);

!
!         /* Create a user function context for cross-call persistence */
!         fctx = (BufferCachePagesContext *) palloc(sizeof(BufferCachePagesContext));
!
!         /* Construct a tuple to return, and save its descriptor. */
          tupledesc = CreateTemplateTupleDesc(NUM_BUFFERCACHE_PAGES_ELEM, false);
          TupleDescInitEntry(tupledesc, (AttrNumber) 1, "bufferid",
                             INT4OID, -1, 0);
***************
*** 92,118 ****
          TupleDescInitEntry(tupledesc, (AttrNumber) 6, "isdirty",
                             BOOLOID, -1, 0);

!         /* Generate attribute metadata needed later to produce tuples */
!         funcctx->attinmeta = TupleDescGetAttInMetadata(tupledesc);
!
!         /*
!          * Create a function context for cross-call persistence and initialize
!          * the buffer counters.
!          */
!         fctx = (BufferCachePagesContext *) palloc(sizeof(BufferCachePagesContext));
!         funcctx->max_calls = NBuffers;
!         funcctx->user_fctx = fctx;

          /* Allocate NBuffers worth of BufferCachePagesRec records. */
          fctx->record = (BufferCachePagesRec *) palloc(sizeof(BufferCachePagesRec) * NBuffers);

!         /* allocate the strings for tuple formation */
!         fctx->values[0] = (char *) palloc(3 * sizeof(uint32) + 1);
!         fctx->values[1] = (char *) palloc(3 * sizeof(uint32) + 1);
!         fctx->values[2] = (char *) palloc(3 * sizeof(uint32) + 1);
!         fctx->values[3] = (char *) palloc(3 * sizeof(uint32) + 1);
!         fctx->values[4] = (char *) palloc(3 * sizeof(uint32) + 1);
!         fctx->values[5] = (char *) palloc(2);

          /* Return to original context when allocating transient memory */
          MemoryContextSwitchTo(oldcontext);
--- 96,109 ----
          TupleDescInitEntry(tupledesc, (AttrNumber) 6, "isdirty",
                             BOOLOID, -1, 0);

!         fctx->tupdesc = BlessTupleDesc(tupledesc);

          /* Allocate NBuffers worth of BufferCachePagesRec records. */
          fctx->record = (BufferCachePagesRec *) palloc(sizeof(BufferCachePagesRec) * NBuffers);

!         /* Set max calls and remember the user function context. */
!         funcctx->max_calls = NBuffers;
!         funcctx->user_fctx = fctx;

          /* Return to original context when allocating transient memory */
          MemoryContextSwitchTo(oldcontext);
***************
*** 167,184 ****
      if (funcctx->call_cntr < funcctx->max_calls)
      {
          uint32        i = funcctx->call_cntr;
!         char       *values[NUM_BUFFERCACHE_PAGES_ELEM];
!         int            j;
!
!         /*
!          * Use a temporary values array, initially pointing to fctx->values,
!          * so it can be reassigned w/o losing the storage for subsequent
!          * calls.
!          */
!         for (j = 0; j < NUM_BUFFERCACHE_PAGES_ELEM; j++)
!         {
!             values[j] = fctx->values[j];
!         }


          /*
--- 158,165 ----
      if (funcctx->call_cntr < funcctx->max_calls)
      {
          uint32        i = funcctx->call_cntr;
!         Datum        values[NUM_BUFFERCACHE_PAGES_ELEM];
!         bool        nulls[NUM_BUFFERCACHE_PAGES_ELEM];


          /*
***************
*** 189,224 ****
              fctx->record[i].isvalid == false)
          {

!             sprintf(values[0], "%u", fctx->record[i].bufferid);
!             values[1] = NULL;
!             values[2] = NULL;
!             values[3] = NULL;
!             values[4] = NULL;
!             values[5] = NULL;

          }
          else
          {

!             sprintf(values[0], "%u", fctx->record[i].bufferid);
!             sprintf(values[1], "%u", fctx->record[i].relfilenode);
!             sprintf(values[2], "%u", fctx->record[i].reltablespace);
!             sprintf(values[3], "%u", fctx->record[i].reldatabase);
!             sprintf(values[4], "%u", fctx->record[i].blocknum);
              if (fctx->record[i].isdirty)
              {
!                 strcpy(values[5], "t");
              }
              else
              {
!                 strcpy(values[5], "f");
              }

          }


          /* Build and return the tuple. */
!         tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
          result = HeapTupleGetDatum(tuple);


--- 170,212 ----
              fctx->record[i].isvalid == false)
          {

!             values[0] = Int32GetDatum(fctx->record[i].bufferid);
!             nulls[0] = false;
!             nulls[1] = true;
!             nulls[2] = true;
!             nulls[3] = true;
!             nulls[4] = true;
!             nulls[5] = true;

          }
          else
          {

!             values[0] = Int32GetDatum(fctx->record[i].bufferid);
!             nulls[0] = false;
!             values[1] = ObjectIdGetDatum(fctx->record[i].relfilenode);
!             nulls[1] = false;
!             values[2] = ObjectIdGetDatum(fctx->record[i].reltablespace);
!             nulls[2] = false;
!             values[3] = ObjectIdGetDatum(fctx->record[i].reldatabase);
!             nulls[3] = false;
!             values[4] = Int64GetDatum((int64) fctx->record[i].blocknum);
!             nulls[4] = false;
              if (fctx->record[i].isdirty)
              {
!                 values[5] = BoolGetDatum(true);
              }
              else
              {
!                 values[5] = BoolGetDatum(false);
              }
+             nulls[5] = false;

          }


          /* Build and return the tuple. */
!         tuple = heap_form_tuple(fctx->tupdesc, values, nulls);
          result = HeapTupleGetDatum(tuple);



Re: pg_buffercache tidyup

From
Tom Lane
Date:
Mark Kirkwood <markir@paradise.net.nz> writes:
> This is probably terrible timing, but I noticed Tom had done some nice
> tidying up on pg_freespacemap to eliminate the clumsy conversion to and
> from strings. This patch does a similar thing for pg_buffercache.

Applied --- it is very late in the cycle, but minor code cleanup of
this sort seems pretty safe.

> I did wonder about not showing buffers that are invalid or not in use
> (currently displays all attributes bar the id as NULL). Comments?

I think we should leave it as-is.  You can easily filter the null rows
if you don't want 'em, while they might be handy for certain sorts of
analysis.  Besides, this module has already been out for one release
so we shouldn't change its API on a whim.

            regards, tom lane