Re: [HACKERS] Another aspect of set_ps_display () - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [HACKERS] Another aspect of set_ps_display ()
Date
Msg-id 200702132317.l1DNH3J13283@momjian.us
Whole thread Raw
Responses Re: [HACKERS] Another aspect of set_ps_display ()  (Bruce Momjian <bruce@momjian.us>)
List pgsql-patches
I did some research on this item from October, 2006.  I was struck by
the memset of 2344 for every proc title change on popular platforms like
Linux.

The attached patch reduces the memset to only span the length needed
since the last title change.  This should significantly reduce the proc
title overhead.

Now that we are using strlcpy(), we still need this code because
strlcpy() doesn't span the entire buffer if not needed, and sometimes
the clobber character is a space, rather than a zero byte.

---------------------------------------------------------------------------

Strong, David wrote:
> We were just analyzing some more OProfile and ltrace data against
> Postgres 8.2Beta1 and we noticed a number of calls as follows:
>
>
> strlen("postgres: tpc tpc 192.168.1.200("...)    = 58
> memset(0xbffff6b2, '\000', 2344)                 = 0xbffff6b2
>
>
> We have tracked this down to the following code in the set_ps_display ()
> function:
>
>
> #ifdef PS_USE_CLOBBER_ARGV
>     {
>         int         buflen;
>
>         /* pad unused memory */
>         buflen = strlen(ps_buffer);
>         MemSet(ps_buffer + buflen, PS_PADDING, ps_buffer_size - buflen);
>     }
> #endif   /* PS_USE_CLOBBER_ARGV */
>
>
> If set_ps_display () moves to use the strlcpy () function call, this
> code might be redundant. Even if the StrNCpy () call is kept, this code
> may still be redundant as StrNCpy () will zero fill the ps_buffer.
>
> A MemSet () call on the ps_buffer has to be added to the init_ps_display
> () function, if this code is removed to clear the buffer before use.
>
> David
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/misc/ps_status.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/ps_status.c,v
retrieving revision 1.34
diff -c -c -r1.34 ps_status.c
*** src/backend/utils/misc/ps_status.c    5 Jan 2007 22:19:46 -0000    1.34
--- src/backend/utils/misc/ps_status.c    13 Feb 2007 23:12:53 -0000
***************
*** 91,96 ****
--- 91,97 ----
  #else                            /* PS_USE_CLOBBER_ARGV */
  static char *ps_buffer;            /* will point to argv area */
  static size_t ps_buffer_size;    /* space determined at run time */
+ static size_t last_status_len;    /* use to minimize length of clobber */
  #endif   /* PS_USE_CLOBBER_ARGV */

  static size_t ps_buffer_fixed_size;        /* size of the constant prefix */
***************
*** 153,160 ****
          }

          ps_buffer = argv[0];
!         ps_buffer_size = end_of_area - argv[0];
!
          /*
           * move the environment out of the way
           */
--- 154,161 ----
          }

          ps_buffer = argv[0];
!         last_status_len = ps_buffer_size = end_of_area - argv[0];
!
          /*
           * move the environment out of the way
           */
***************
*** 329,335 ****

          /* pad unused memory */
          buflen = strlen(ps_buffer);
!         MemSet(ps_buffer + buflen, PS_PADDING, ps_buffer_size - buflen);
      }
  #endif   /* PS_USE_CLOBBER_ARGV */

--- 330,339 ----

          /* pad unused memory */
          buflen = strlen(ps_buffer);
!         /* clobber remainder of old status string */
!         if (last_status_len > buflen)
!             MemSet(ps_buffer + buflen, PS_PADDING, last_status_len - buflen);
!         last_status_len = buflen;
      }
  #endif   /* PS_USE_CLOBBER_ARGV */


pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: Forbid finishing a prepared transaction from another database
Next
From: Bruce Momjian
Date:
Subject: Re: array_accum aggregate